-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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 AnimationMixer bindings only bound in the editor #83440
Remove AnimationMixer bindings only bound in the editor #83440
Conversation
ClassDB::bind_method(D_METHOD("_reset"), &AnimationMixer::reset); | ||
ClassDB::bind_method(D_METHOD("_restore", "backup"), &AnimationMixer::restore); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't seem to need to be bound, so I just removed them.
scene/animation/animation_mixer.cpp
Outdated
ClassDB::bind_method(D_METHOD("set_reset_on_save_enabled", "enabled"), &AnimationMixer::set_reset_on_save_enabled); | ||
ClassDB::bind_method(D_METHOD("is_reset_on_save_enabled"), &AnimationMixer::is_reset_on_save_enabled); | ||
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "reset_on_save", PROPERTY_HINT_NONE, ""), "set_reset_on_save_enabled", "is_reset_on_save_enabled"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it was only bound to add a checkbox to the inspector, so I moved it to _get_property_list
, _get
and _set
.
Unfortunately, it seems we can't document the property if it's added in _get_property_list
, so CI complains about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should simply be exposed for both tools and non-tools builds. It can be documented that it's only functional in the editor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, actually we have to expose it because it was exposed before so it would break compat not to. I thought it was a new property. By the way, the documentation already says that it's used by the editor.
ClassDB::bind_method(D_METHOD("_restore", "backup"), &AnimationMixer::restore); | ||
ClassDB::bind_method(D_METHOD("set_reset_on_save_enabled", "enabled"), &AnimationMixer::set_reset_on_save_enabled); | ||
ClassDB::bind_method(D_METHOD("is_reset_on_save_enabled"), &AnimationMixer::is_reset_on_save_enabled); | ||
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "reset_on_save", PROPERTY_HINT_NONE, ""), "set_reset_on_save_enabled", "is_reset_on_save_enabled"); | ||
ADD_SIGNAL(MethodInfo("mixer_updated")); // For updating dummy player. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signal is now bound in non-editor builds even though it's only emitted in editor builds. I'm not sure if that's OK, but the signal is used by AnimationPlayerEditorPlugin
so I think it's still needed.
4e67935
to
537075a
Compare
537075a
to
ae9ac5c
Compare
The documentation seems to already properly flag the property and signal as editor-only. |
Thanks! |
Removes bindings inside
#ifdef TOOL_ENABLED
that would only be bound in the editor and break the C# bindings in exported games.