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

Key rotation rather than rotation-degrees #21

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

doctor-g
Copy link
Contributor

@doctor-g doctor-g commented Jul 1, 2023

Playing back animations that have been keyed to 'rotation-degrees' will work, but these keys cannot be edited in the animation editor--at least not in Godot 4. One can see the time of the key but not its value. This prevents tweaking animations. Worse, new rotation keys will be made on the 'rotation' property, not the 'rotation-degrees' property, and interpolating both values in an animation doesn't work at all.

Playing back animations that have been keyed to 'rotation-degrees' will work, but these keys cannot be edited in the animation editor--at least not in Godot 4. One can see the time of the key but not its value. This prevents tweaking animations. Worse, new rotation keys will be made on the 'rotation' property, not the 'rotation-degrees' property, and interpolating both values in an animation doesn't work at all.
@doctor-g
Copy link
Contributor Author

doctor-g commented Jul 1, 2023

I did have one case with my sample data where a rotation that was supposed to be about two degrees to the left was instead 358 degrees to the right. I imagine this is related to the spinning or easing handling, maybe that code where I replaced 360 by TAU to work in radians. Since it was just one case, and I could now edit the values in Godot 4.0's animation editor, it was easy to fix. Still, I wanted to mention it here in case someone else has a better sense of the actual data structures and parameters being used here.

@wojtossfm
Copy link
Owner

@doctor-g You wouldn't potentially be able to submit a reproduction case e.g. you scml file with placeholder images instead of the actual art? Asking just in case it doesn't come up with the cases that I have currently.

Copy link
Owner

@wojtossfm wojtossfm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll merge this in as-is and investigate further before releasing a new version to the asset library. Thank you for the pull request.

@doctor-g
Copy link
Contributor Author

Here's a way to demonstrate the issue I mentioned. I've been tinkering with the animations from CraftPix, and you can reproduce the situation using one of their free downloads, such as the goblin from this set. In the download, you'll find Goblin/PNG/VectorParts, which contains the figure parts as well as Animations.scml.

The animations look fine in isolation; the problem only arises when crossfading between them, as the rotations will sometimes go "the long way around." There are different ways to reproduce the problem, but here's a particular workflow that I was commonly using.

  1. With the SCML importer enabled (and this patch in place), import the goblin parts and animations as described above.
  2. Open the Animations.scml file and create a New Inherited scene when prompted. Save that as a new scene, then remove the inheritance by right-clicking on the root of tree and selecting "Clear Inheritance." This is required to be able to edit the animations.
  3. In the AnimationPlayer node, disable looping on the Idle and Walking animations.
  4. Create an AnimationTreePlayer node. Connect the AnimationPlayer to it. Make its root node into an AnimationNodeStateMachine.
  5. In the state machine, add the Idle and Walking animations, then add automatic transitions to go between them that change at the end of the animation. (That's why the looping was removed above.) Set the animation tree player to Active.
  6. Add a crossfade (e.g. 0.2 seconds) between the animations. Now you will be able to see that the change between animations has the head spinning almost a complete circle.

I hope that helps!

@wojtossfm
Copy link
Owner

I've raised it separately as #24 and will see if I can find a fix. I've also adjusted some things further around the rotation logic (see 9bde4dc).

Also worth seeing if:

  • does disabling the "optimise for blends" option help in any way?

As for disabling looping - there is also an import option to disable looping on the animations that are created (just in case you missed it and it simplifies some of the process).

@doctor-g
Copy link
Contributor Author

FWIW, I disabled "optimise for blends" and that made it behave very strangely. The AnimationPlayer still worked fine, but as soon as I played the animation through my AnimationTree, the textures got removed from the sprites.

@wojtossfm
Copy link
Owner

It is worth knowing - I haven't really used the animations much with blends and as such will see if it will help.

  • Your patch left some parts of the rotation_degrees logic around. I'm not certain how big of an impact the changes might have for your case but worth seeing. Do let me know if you tried the current master (haven't made a tag release yet) which contains 9bde4dc
  • Likewise at least for one of my cases the (though with looping) the loop interpolation accounting for wrapping was giving unexpected results which is why I added an option for it and made it default to disabled (at least my expectation was people would expect it to be disabled). Since you reproduced it without looping I don't expect it to impact your case but figured worth knowing if you are actively using the plugin.

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

Successfully merging this pull request may close these issues.

None yet

2 participants