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

Add hint for oneshot property & warning when it will be updated continuously by Force Continuous in AnimationMixer #95711

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

TokageItLab
Copy link
Member

These properties should not be continually updated, so that warnings are issued when they may occur unintentionally.

Ideally, FLAG_USAGE should be used, but I fear the int value limit as flag, so I append that to HINT;

As I remember only audio and partcle bool value properties should have these, but if there are non-bool value properties in other classes that should have this, they should be in USAGE. If there is such a thing, please let me know.

@TokageItLab TokageItLab added this to the 4.x milestone Aug 17, 2024
@TokageItLab TokageItLab requested review from a team as code owners August 17, 2024 19:29
@TokageItLab TokageItLab changed the title Add hint for oneshot & warning when it will be updated continuously Add hint for oneshot property & warning when it will be updated continuously unintentionally Aug 17, 2024
@TokageItLab TokageItLab changed the title Add hint for oneshot property & warning when it will be updated continuously unintentionally Add hint for oneshot property & warning when it will be updated continuously by Force Contunuous in AnimationMixer Aug 17, 2024
@TokageItLab TokageItLab modified the milestones: 4.x, 4.4 Aug 25, 2024
@TokageItLab TokageItLab added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Aug 25, 2024
@fire
Copy link
Member

fire commented Sep 2, 2024

The title and the commit has a typo in English grammar. Force Contunuous -> ???

@TokageItLab TokageItLab changed the title Add hint for oneshot property & warning when it will be updated continuously by Force Contunuous in AnimationMixer Add hint for oneshot property & warning when it will be updated continuously by Force Continuous in AnimationMixer Sep 2, 2024
@TokageItLab TokageItLab force-pushed the warn-oneshot-prop branch 2 times, most recently from 328e4f1 to ce2fb11 Compare October 5, 2024 17:52
@TokageItLab TokageItLab requested a review from fire October 5, 2024 17:52
@KoBeWi
Copy link
Member

KoBeWi commented Nov 7, 2024

This should really be a property usage, but I think we can add just only one more usage value🤔
Then again, none of the current ONE_SHOT properties have problem with this being a hint, so we could treat it as temporary solution and think of a fix once an actual problem arises (i.e. conflict with another hint, as properties can't have more than 1 hint). To avoid breaking compatibility in the future, you can unexpose the new hint, with a comment that it shouldn't be exposed, because it's improper solution or something.

doc/classes/@GlobalScope.xml Outdated Show resolved Hide resolved
scene/animation/animation_mixer.cpp Outdated Show resolved Hide resolved
doc/classes/@GlobalScope.xml Outdated Show resolved Hide resolved
@akien-mga akien-mga requested review from reduz and removed request for a team November 11, 2024 11:57
@TokageItLab
Copy link
Member Author

For now, note that there is comment that reduz's lgtm on contributors chat.
https://chat.godotengine.org/channel/animation?msg=eFmBbXF992AHvW5DM

@Repiteo Repiteo merged commit 37305e4 into godotengine:master Nov 22, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 22, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release enhancement topic:animation topic:audio topic:particles
Projects
Status: Very Bad
Status: Done
Development

Successfully merging this pull request may close these issues.

AnimationTree state machine causes glitchy sound when triggering playing of AudioStreamPlayer3D
5 participants