Fix SkeletonModification2DTwoBoneIK with negative scales.

[thiagola92]

trafficstars

Issues: #80252

This is an attempt to fix SkeletonModification2DTwoBoneIK when only one scale is negative.

• Identifies when only one scale axis is negative and change the logic
• Take in count scales when drawing gizmo
• Fix typo

Note: Last bone will be inverted due to issue #80643 (PR to fix this: #81048)

• Bugsquad edit, fixes: #80252

[thecodacus]

can this be pushed as patch update.. the ik bug is making the ik chain unusable

[thiagola92]

I'm not actually happy with this solution (using if to check scales and decide the action).
And I hope someone knows a better way :rofl:.

[GopherDevGames]

is there anything blocking this PR as of now?

[thiagola92]

is there anything blocking this PR as of now?

Is better to wait someone with knowledge like @TokageItLab to review
Someone that could tell if there is or not a better way to solve this (or tell if this solutions brings any problem)

[TokageItLab]

Since TwoBoneIK is separated from any other node in the first place, so the change should be safe, at least from a code standpoint. Also, if this change is mainly dedicated to flips due to scaling, then there seems to be fine.

I am a little concerned about whether it will work well when combined with the https://github.com/godotengine/godot/pull/91731 fix. If we can confirm that much, I think it is safe to merge.

BTW, since the ModificationStack2D is experimental to begin with, so I think in the future it will be necessary to migrate into SkeletonModifier2D in the same way as the SkeletonModifier3D.

[CsloudX]

Is this plan for 4.3 or 4.4?

[fire]

@TokageItLab We can review this for 4.4

[fire]

The pull request became stale and requires a rebase.

@SaracenOne, @TokageItLab, @fire and Cthulhu discussed this briefly at the animation meeting.

We discussed the problems with using the scale to flip 2d and 3d animation in general.

We wanted a proposal for modifying the godot human skeleton profile to add a hard-defined mirror boolean for regular and mirrored left hand to right hand.

[thiagola92]

We discussed the problems with using the scale to flip 2d and 3d animation in general.

There is a list?

All I know is that is not possible to differ 2D matrices that had X scaled by -1 from those that had Y scaled by -1 and rotate by 180º. At least not without a variable to store this information.

We wanted a proposal for modifying the godot human skeleton profile to add a hard-defined mirror boolean for regular and mirrored left hand to right hand.

I don't understand how this would work under the hood. There is a change in "modification" logic? Or would be something like:

if (!mirrored) {
  joint_one_bone->set_global_rotation(angle_atan - angle_0 - joint_one_bone->get_bone_angle());
} else {
  joint_one_bone->set_global_rotation(angle_atan + angle_0 + joint_one_bone->get_bone_angle());
}

Instead of my propose:

if (same_scale_sign) {
  joint_one_bone->set_global_rotation(angle_atan - angle_0 - joint_one_bone->get_bone_angle());
} else {
  joint_one_bone->set_global_rotation(angle_atan + angle_0 + joint_one_bone->get_bone_angle());
}

[F3der1co]

The pull request became stale and requires a rebase.

@SaracenOne, @TokageItLab, @fire and Cthulhu discussed this briefly at the animation meeting.

We discussed the problems with using the scale to flip 2d and 3d animation in general.

We wanted a proposal for modifying the godot human skeleton profile to add a hard-defined mirror boolean for regular and mirrored left hand to right hand.

This doesn't make sense to me, or are you talking about skeleton2d mirror boolean, not human skeleton profile? Mirroring is required at runtime in almost any 2d skeleton. For left and right facing variants of each animation. Maybe I am not getting what this proposal would do/be about and how it would solve this problem.

Sad to see this being stale

[0xcafeb33f]

Can this be merged in 4.5? If not, what specifically is blocking it?

If the concern is only that it needs a rebase, I have already rebased this PR and #83330 locally in my custom build of Godot 4.5, and would be happy to contribute it back if desired.

I don't understand the suggestion for a proposal. This PR does not add a new mirroring feature; it fixes a longstanding bug when Node2D transforms interact with Skeleton2Ds. I have verified that this fixes the issue in my project. Why would a proposal be needed for this bugfix?

[rjunkfolder-del]

We really need this to be fixed...

[GopherDevGames]

We really do. This PR used to basically fix the issue 99% of the time, but the original code changed too much and I'm too unfamiliar with the engine to rebase it now. At least there's a bunch of GDScript solutions for 2DIK

[0xcafeb33f]

I just saw @thiagola92's comment that they no longer plan to work on this PR. I have created a separate PR that rebases this one (#110298) per my previous comment -- please review/test and let me know if there are any issues.

[Repiteo]

Superseded by #110298