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

8719a81 breaks player-reanimating mods #15692

Open
appgurueu opened this issue Jan 19, 2025 · 5 comments
Open

8719a81 breaks player-reanimating mods #15692

appgurueu opened this issue Jan 19, 2025 · 5 comments
Labels
@ Client rendering Discussion Issues meant for discussion of one or more proposals Not a bug The behaviour is working as intended and won't be changed Regression Something that used to work no longer does
Milestone

Comments

@appgurueu
Copy link
Contributor

appgurueu commented Jan 19, 2025

8719a81 breaks playeranim and character_anim.

Both these mods "re-animate" the player (originally for the reason that 5.2 and lower did not support overriding single bones), that is, they calculate bone transforms in pure Lua for all bones and apply them via bone overrides.

The problem boils down to the fact that both these mods override the Body bone rotation to identity, when they should override it to a 180° rotation around the Y axis. It's easy to see that these mods are in the wrong by inspecting character.b3d and observing that this is what the body rotation should be (as a quaternion, that's 0, 1, 0, 0 in XYZW order).

Problem is, older Luanti would falsely decompose a perfect 180° Y axis rotation (which is represented exactly this way in the b3d model!) into a -1, 1, -1 scaling and an identity rotation. In other words: These mods are working around an older Luanti bug. I now fixed this bug, and now these mods are setting wrong rotations for all bones that have such perfect rotations:

Image

The bugs are relatively easy to pinpoint:

Furthermore the manual head rotation needs to be fixed.

For a mod to be affected, it needs to apply a bone override to a bone which is rotated by exactly 180° around one of the cardinal axes, thereby inverting the other two axes. This means there will likely be more affected mods than these two.

Impact:

  • playeranim has ~1.5k downloads on CDB. character_anim has ~43k downloads.
  • playeranim serves a total of ~50 players on only 5 servers at the time of this writing, the most notable being Your Land (@Bastrabun) and Blocky Survival with 30 and 15 players respectively. character_anim serves ~10 players on a few more servers.
  • Even if we fix the mods, it's still bad:
    • New clients joining servers running older versions of these mods will see players incorrectly.
    • Older clients will see the models incorrectly, because they are sent the same overrides.
Affected servers

Image
Image

Possible solutions:

  • headanim is a decent alternative that is unaffected by these changes, but only works on 5.3+. This seems a good compromise to me so far.
  • Put the bug back in. Not a decent option long term. May be a decent option for 5.11.
  • Put the bug back in, but behind a flag. May be a bit tricky, but could work. I don't like having to enable "correct" behavior though.
@appgurueu appgurueu added @ Client rendering Discussion Issues meant for discussion of one or more proposals Not a bug The behaviour is working as intended and won't be changed Regression Something that used to work no longer does labels Jan 19, 2025
@Zughy
Copy link
Contributor

Zughy commented Jan 19, 2025

These mods are working around an older Luanti bug. I now fixed this bug

Problem solved then, I don't see how it's an issue of ours when we did our part. The rest is just more noise in the code

@nininik0
Copy link

There is a similar issue after 5.8 with 3d_armor_flyswim where the animation is jittery and loops, so it dosent show the full animation because it resets, which looks ugly and not good, and applies to all but the walking animation.

@appgurueu
Copy link
Contributor Author

There is a similar issue after 5.8 with 3d_armor_flyswim where the animation is jittery and loops, so it dosent show the full animation because it resets, which looks ugly and not good, and applies to all but the walking animation.

That sounds like a different issue, possibly not even an engine issue. This issue is specifically about a change in behavior in 5.11-dev.

@sfan5
Copy link
Collaborator

sfan5 commented Jan 20, 2025

Someone reported a Mineclone-related breakage here: https://irc.minetest.net/minetest/2025-01-20#i_6236567

@Desour Desour added this to the 5.11.0 milestone Jan 20, 2025
@appgurueu
Copy link
Contributor Author

An additional workaround I think I can offer:

Using modlib's b3d tools, I could produce slightly modified versions of models (at load time even) which avoid triggering the bug by very slightly rotating bones (not that this rotation should matter for playeranim or character_anim, because it is entirely overwritten by the bone override anyways). This would enable consistency between older and newer Luanti client versions. Users would still have to upgrade playeranim or character_anim (or their Mineclon* version).

Alternatively, I could turn these perfect rotations into no rotation and negative scale in the b3d and then make some slightly tricky changes to preserve the TRS representation with negative scaling inside Irrlicht, but I don't see much of an advantage of this (except that it doesn't require updating the affected mods, but since there's only a handful and character_anim and player_anim should be pretty easy to fix, it doesn't matter much; making the engine changes would probably be trickier).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Client rendering Discussion Issues meant for discussion of one or more proposals Not a bug The behaviour is working as intended and won't be changed Regression Something that used to work no longer does
Projects
None yet
Development

No branches or pull requests

5 participants