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

Fix errors and warnings when loading Skeleton2D Modifications #84474

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

johnsonbaugh
Copy link
Contributor

Fixes #73247

  • Don't print cache errors about not being "properly" setup when not setup, because that is normal
  • Don't print warnings about verification when not setup
  • Avoid cache warnings and other tree-reference-related errors calling setup_modifications on NOTIFICATION_POST_ENTER_TREE

Copy link

@0xcafeb33f 0xcafeb33f left a comment

Choose a reason for hiding this comment

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

I tested this patch locally on my own project, and it resolved all of the incorrect error messages for my Skeleton2Ds.

Can this be cherrypicked to 4.2?

Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

I don't have much experience with using SkeletonModifier2D in production, but I am aware of the logspam issue it has, so this would be good to get in.

The change looks good. Almost all changes are making log messages less noisy which is a good thing.

Other than a few extra calls to fabrik_joint_update_bone2d_cache() and inside_tree checks for setup (and a NOTIFICATION_POST_ENTER_TREE to trigger that setup at the correct time) there are no functional changes so this seems pretty safe.

@akien-mga
Copy link
Member

@johnsonbaugh Would you be able to rebase to solve the merge conflicts?

@akien-mga akien-mga merged commit 1c3a307 into godotengine:master Apr 24, 2024
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

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

Successfully merging this pull request may close these issues.

Skeleton2D Modifications Throw Error and Warnings stating that bone index needs to be updated
5 participants