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

Remove unused warnings for bones and skeleton modifications and change some cache reload warning to error in case of failure #89387

Conversation

Ughuuu
Copy link
Contributor

@Ughuuu Ughuuu commented Mar 11, 2024

Based on: #73247 (comment)

My findings are on that page, but will put them here also:

The warning doesn't need to be a warning as the bone_cache will be updated anyway during the setup call.
Also some warnings don't need to be warnings at all as they are not actually warnings, but just the value is set initially(eg. bone index) and the cache not computed.

@Ughuuu Ughuuu marked this pull request as ready for review March 11, 2024 15:47
@Ughuuu Ughuuu requested a review from a team as a code owner March 11, 2024 15:47
@Ughuuu Ughuuu force-pushed the Skeleton2D-Modifications-Throw-Error-and-Warnings-that-are-not-needed branch 2 times, most recently from e2b179f to f67dd13 Compare March 11, 2024 15:55
@AThousandShips
Copy link
Member

Needs a more descriptive title, it is very vague on what errors and warnings are talked about

@Ughuuu Ughuuu changed the title Remove unused warnings and change some warning to error. Remove unused warnings for bones and skeleton modifications and change some cache reload warning to error in case of failure Mar 11, 2024
@Ughuuu Ughuuu force-pushed the Skeleton2D-Modifications-Throw-Error-and-Warnings-that-are-not-needed branch from f67dd13 to 63daffe Compare March 11, 2024 20:16
update

lint

code feedback

also don't throw error in case of regular setup when is setup isn't set and we just set a var.
@Ughuuu Ughuuu force-pushed the Skeleton2D-Modifications-Throw-Error-and-Warnings-that-are-not-needed branch from 98eef42 to e68634f Compare March 11, 2024 20:24
@Ughuuu
Copy link
Contributor Author

Ughuuu commented Mar 11, 2024

Seems to fix all warnings/errors from popping up. And fixes some use cases for cache rebuilding.

@0xcafeb33f
Copy link

It looks like this solves the skeleton errors in a slightly different way than #84474. This PR guards against calling the _cache functions before the skeleton is setup, while the previous PR changed the _cache functions internally to allow them to be called under all conditions without error. I'm uncertain which of these two approaches is better, but only one should probably be merged, as they duplicate fixing the issue in different ways.
Note that this is meant to fix #73247, but I have not yet tested this patch locally.

@Ughuuu
Copy link
Contributor Author

Ughuuu commented Mar 30, 2024

Last time someone on that issue said there are still some errors/warnings printed with this PR, so it might miss some cases right now.

@Ughuuu Ughuuu closed this Apr 24, 2024
@Ughuuu Ughuuu deleted the Skeleton2D-Modifications-Throw-Error-and-Warnings-that-are-not-needed branch April 24, 2024 08:13
@AThousandShips AThousandShips removed this from the 4.x milestone Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants