Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SkeletonIK (3D) Regression in Godot 4.0 #74753

Closed
DillonSteyl opened this issue Mar 11, 2023 · 12 comments · Fixed by #77194
Closed

SkeletonIK (3D) Regression in Godot 4.0 #74753

DillonSteyl opened this issue Mar 11, 2023 · 12 comments · Fixed by #77194

Comments

@DillonSteyl
Copy link

Godot version

4.0

System information

Windows 11

Issue description

SkeletonIK behaves completely differently in Godot 4.0 than it did in Godot 3.5 when using the same setting (magnet, etc).
In addition, it is difficult to test this because the IK stops whenever you select a node other than the SkeletonIK3D node - this also doesn't occur in Godot 3.5.

It easiest to demonstrate this with a video:

skeleton_ik_regression.mov

Steps to reproduce

  • Import a model with an armature.
  • Create an inherited scene for the model.
  • Create a SkeletonIK3D as a child of the Skeleton3D node. Assign the "root bone" and "tip bone" properties to some bones in the model.
  • Create a Node3D to act as the target, and assign the "target node" property of the SkeletonIK3D to this new node.
  • Write a @tool script which starts the SkeletonIK3D on _ready.
  • Re-open the scene, and then wiggle the target node. Observe the behaviour.

Minimal reproduction project

SkeletonIKRegression.zip

@Michael-Sjogren
Copy link

Michael-Sjogren commented Mar 11, 2023

it is difficult to test this because the IK stops whenever you select a node other than the SkeletonIK3D node - this also doesn't occur in Godot 3.5.

yes i have experienced this also in godot v4.0.stable.official [92bee43] unfortunately.

@yosoyfreeman
Copy link
Contributor

As far as i know, everything on the ik systems in broken right now. Neither me or other people couldn't made nothing work after trying and trying.

@DillonSteyl
Copy link
Author

DillonSteyl commented Mar 11, 2023

As far as i know, everything on the ik systems in broken right now. Neither me or other people couldn't made nothing work after trying and trying.

Thats a shame, as it is a core feature - I've also taken a stab at fixing this particular issue and I wasn't able to get it working. I believe I have isolated where the problem is occurring, but even after reverting back to old code the behaviour was even worse. Perhaps someone who was actively involved in changes to the Skeleton system will have to figure out what's wrong.

It's also a shame that I only discovered the IK system was broken after investing a number of hours into porting my game across to Godot 4.0 - perhaps this should at least be documented somewhere?

Specifically, I think the problem might be here:

if (!ci->children.is_empty()) {
	Vector3 forward_vector = get_bone_axis_forward_vector(p_task->skeleton, ci->bone);
	// Rotate the bone towards the next bone in the chain:
	new_bone_pose.basis.rotate_to_align(forward_vector, new_bone_pose.origin.direction_to(ci->children[0].current_pos));
}

as if you comment the basis rotation out, the actual positions of the bones seem to behave a lot nicer.

@yosoyfreeman
Copy link
Contributor

As you said there was lot of changes to skeletons, physics... At some point a IK stack was made, but none of it's components worked and i think was deprecated.

As i see it, is not only that IK not works, is that the ik implementations never made a lot of sense, for example, you are limited to work with bones, while lots of the time you would want to work with objects/nodes. This, in godot, which is node centered, is especially estrange.

I think that if this is going to be approached in some point the workflow should be more general, node centered and works like any other part of the engine, with nodes / bones and keeping different kinds of IK encapsulated in a single node per IK.

@DillonSteyl
Copy link
Author

While I agree with you on this, I still think its a good idea to have a barebones IK implementation that is at the very least consistent with Godot 3.5 - in my case, it served its purpose well enough and this is the first thing I've come across that has halted my progress in porting my project over to Godot 4.0.

I would hope someone smarter than myself might be able to fix this issue so that we can at least replicate the 3.5 behaviour until a more fully-fledged system is implemented (e.g. the IK stack you mentioned)

@AThousandShips
Copy link
Member

AThousandShips commented Mar 11, 2023

As far as I recall the decision was made to not leave the IK system in a non-functional state for 4.0, and making it functional in time for the release was not feasible, but I believe fixing it is one of the things intended for a future release

@AThousandShips
Copy link
Member

AThousandShips commented Mar 11, 2023

See #71137 and #69752

@DillonSteyl
Copy link
Author

@AThousandShips

Thanks for the context - the reason I raised this separately is because those issues refer to the removal of the newly-added modification stack, and they both contain comments like this:
"To be clear, I intend to get the classic SkeletonIK3D node (same as Godot 3.x) up and running in time for 4.0"
and
"I do intend to fix the known issues with SkeletonIK3D (the Node) in time for 4.0 to get it working as well as it was in 3.5"

My goal with this issue was to make it known that the IK solution is not working as well as it was in 3.5, and hopefully get some help with debugging it so we can at least have something comparable to 3.5 up and running soon

@AThousandShips
Copy link
Member

Very valid, wanted to provide context for that change, not up to date on the process of fixing those problems unfortunately

@DillonSteyl
Copy link
Author

I'm fairly sure this issue is related to the bone rest/pose change here.

As far as I can tell, the current IK code was not updated to reflect those changes. There are a number of places in the IK code that use functions like get_bone_global_pose and get_bone_global_pose_override - one thing I tried was changing these to use get_bone_global_rest whenever the initial transform of the chain item is set, and the behaviour improved significantly:

bone_rests.mov

Now it's just a matter of figuring out when to use pose and when to use rest - the above is better but it still doesn't work perfectly (especially when the magnet is introduced). I'm not very confident making these changes myself but I feel confident this is where the issue lies

@Riteo
Copy link
Contributor

Riteo commented May 16, 2023

@DillonSteyl nice catch!

I'm not very confident making these changes myself but I feel confident this is where the issue lies

While #70887 will probably be the best option in the long term, I think that we should look into fixing the problem completely, as it may allow old games that rely heavily on the old stack to be ported here while also giving people something to work with as we can't know when the new system will be ready, if ever.

@fire
Copy link
Member

fire commented May 18, 2023

We think #77194 solves this, would like testing on the usecases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants