godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.94k stars 20.17k forks source link

SkeletonIK node causing bones to become detached #48772

Closed iwilliams closed 3 years ago

iwilliams commented 3 years ago

Godot version: 3.3.1-rc2 & 3.3-stable

OS/device including version: Linux

Issue description: I have multiple SkeletonIK nodes working together to pose my character model. In 3.2.3, the SkeletonIK nodes apply together as expected, but in 3.3-stable and later there are a few different issues. See the video below for problems as well as my summary for each engine version I tested:

https://user-images.githubusercontent.com/5215799/118409805-18cab780-b641-11eb-81b9-df0f5ddf7876.mp4

Steps to reproduce:

  1. Import a model with a humanoid skeleton.
  2. Set up multiple SkeletonIK nodes for different limbs.
  3. Use a tool script to activate each SkeletonIK node simultaneously while in editor.
  4. Move an IK target that would displace a bone in another IK chain.
  5. Observe differences between engine versions.

Minimal reproduction project: ik-bug.zip

akien-mga commented 3 years ago

CC @TwistedTwigleg

TwistedTwigleg commented 3 years ago

I think this is a side effect of fixing the bone attachment bug. The problem is that to fix the BoneAttachment issue, instead of using the global pose override position it instead uses the global pose of the bone without any overrides (so just animation stuff). Previously it overrode the global pose override, which would mean the first joint would be reset but future joints would calculate correctly, allowing multiple SkeletonIK nodes to be used at once.

However because it uses the global pose without overrides, if something like the spine moves with an override, other SkeletonIK nodes are unaware as they use the non-overrides pose, leading to the issue seen here.

Edit: This is just my assumption based on what is going on, I have not looked at the code or anything.

Unfortunately, I’m not really sure what the best way to fix this would be right off, other than rewriting how the algorithm works. I suppose as a fix we could have a check box that will set it to use the global pose with overrides, so it can be used with multiple SkeletonIK nodes at the cost of BoneAttachment nodes not working when this checkbox is enabled. That way, those who need BoneAttachment nodes can have the checkbox disabled but not have multiple SkeletonIK support, but those who want to use multiple SkeletonIK nodes still can but without BoneAttachment’s working.

iwilliams commented 3 years ago

Unfortunately, I’m not really sure what the best way to fix this would be right off, other than rewriting how the algorithm works. I suppose as a fix we could have a check box that will set it to use the global pose with overrides, so it can be used with multiple SkeletonIK nodes at the cost of BoneAttachment nodes not working when this checkbox is enabled. That way, those who need BoneAttachment nodes can have the checkbox disabled but not have multiple SkeletonIK support, but those who want to use multiple SkeletonIK nodes still can but without BoneAttachment’s working.

For what it's worth, I'm using this IK setup in 3.2.3 with BoneAttachments and it's working mostly correct besides some inconsistent behavior when trying to move children of the BoneAttachment.

TwistedTwigleg commented 3 years ago

For what it's worth, I'm using this IK setup in 3.2.3 with BoneAttachments and it's working mostly correct besides some inconsistent behavior when trying to move children of the BoneAttachment.

Are you using BoneAttachments on the first bone in the chain? The issue with BoneAttachment nodes not working with SkeletonIK only seems to affect the very first bone in the chain, as described in #39893. Other bones should work fine with BoneAttachment nodes.

iwilliams commented 3 years ago

For what it's worth, I'm using this IK setup in 3.2.3 with BoneAttachments and it's working mostly correct besides some inconsistent behavior when trying to move children of the BoneAttachment.

Are you using BoneAttachments on the first bone in the chain? The issue with BoneAttachment nodes not working with SkeletonIK only seems to affect the very first bone in the chain, as described in #39893. Other bones should work fine with BoneAttachment nodes.

By "first bone in the chain" do you mean bones set as the root bone of a SkeletonIK Node? I have attachments on tip and root bones and they both work fine for me.

TwistedTwigleg commented 3 years ago

For what it's worth, I'm using this IK setup in 3.2.3 with BoneAttachments and it's working mostly correct besides some inconsistent behavior when trying to move children of the BoneAttachment.

Are you using BoneAttachments on the first bone in the chain? The issue with BoneAttachment nodes not working with SkeletonIK only seems to affect the very first bone in the chain, as described in #39893. Other bones should work fine with BoneAttachment nodes.

By "first bone in the chain" do you mean bones set as the root bone of a SkeletonIK Node? I have attachments on tip and root bones and they both work fine for me.

Correct! I mean the first bone in the SkeletonIK node. They should be working now, but prior to 3.2.4, BoneAttachment's on root bones were not working, or at least not in the sample project in #39893.

I fixed the bone attachment issue in Godot 3.2.4 but then the twisting bug occurred. Fixing the twisting bug probably caused this issue, as now the code uses the global pose without overrides when calculating, making it not work with multiple SkeletonIK nodes.

I've been thinking about this all day and I'm wondering if its possible to use the global pose override positions directly, without resetting them at all. In theory, FABRIK should work just fine with bones in any starting position, as long as the root bone can be snapped back into place. I'll have to tinker with the code, probably this weekend, and see if I can find a solution.

iwilliams commented 3 years ago

For what it's worth, I'm using this IK setup in 3.2.3 with BoneAttachments and it's working mostly correct besides some inconsistent behavior when trying to move children of the BoneAttachment.

Are you using BoneAttachments on the first bone in the chain? The issue with BoneAttachment nodes not working with SkeletonIK only seems to affect the very first bone in the chain, as described in #39893. Other bones should work fine with BoneAttachment nodes.

By "first bone in the chain" do you mean bones set as the root bone of a SkeletonIK Node? I have attachments on tip and root bones and they both work fine for me.

Correct! I mean the first bone in the SkeletonIK node. They should be working now, but prior to 3.2.4, BoneAttachment's on root bones were not working, or at least not in the sample project in #39893.

I fixed the bone attachment issue in Godot 3.2.4 but then the twisting bug occurred. Fixing the twisting bug probably caused this issue, as now the code uses the global pose without overrides when calculating, making it not work with multiple SkeletonIK nodes.

I've been thinking about this all day and I'm wondering if its possible to use the global pose override positions directly, without resetting them at all. In theory, FABRIK should work just fine with bones in any starting position, as long as the root bone can be snapped back into place. I'll have to tinker with the code, probably this weekend, and see if I can find a solution.

Thank's for all your hard work on this! If you think it would help, I'd be happy to add you on my repo where I have a a decently complex setup with multiple SkeletonIK nodes and BoneAttachments. Just let me know!

TwistedTwigleg commented 3 years ago

Thank's for all your hard work on this! If you think it would help, I'd be happy to add you on my repo where I have a a decently complex setup with multiple SkeletonIK nodes and BoneAttachments. Just let me know!

Thanks! I’ll probably try creating and/or using a minimal project for testing/fixing initially, but once I have a potential fix, I certainly think a more complex setup would be helpful for making sure it all fully works.

(Also, sorry about the delay in replying! Other stuff has taken more time than I expected)

akien-mga commented 3 years ago

@iwilliams Could you test #49031 which aims at fixing this issue? You can download a CI build from Linux here: https://github.com/godotengine/godot/suites/2816593439/artifacts/62689994

iwilliams commented 3 years ago

@akien-mga Just tested and my bones seem to be attached as expected! Thanks for looking into it @TwistedTwigleg :)

akien-mga commented 3 years ago

Fixed by #49031.

For the reference, that fix is not included yet in Godot 3.3.2-stable which I'm about to release (already built), but it should be in 3.4 and 3.3.3.