-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Avoid double editing when clicking AnimatedSprite #90815
Conversation
@@ -2325,7 +2329,7 @@ bool SpriteFramesEditorPlugin::handles(Object *p_object) const { | |||
if (animated_sprite_3d && *animated_sprite_3d->get_sprite_frames()) { | |||
return true; | |||
} | |||
return p_object->is_class("SpriteFrames"); | |||
return !frames_editor->is_editing() && Object::cast_to<SpriteFrames>(p_object); |
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.
That pattern is a bit unconventional, what makes SpriteFrames special compared to other editor plugins which handle resources?
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.
It handles both AnimatedSprite nodes and SpriteFrames resource. (handling nodes allows opening the editor when the resource is folded)
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 new logic is incorrect, e.g. if you have selected/edited an AnimatedSprite2D with a valid SpriteFrames and then try to edit another SpriteFrames (e.g. by double clicking in the FileSystem dock a different SpriteFrames resource file) then the SpriteFrames edited by the plugin wouldn't be changed, this new logic would refuse to edit another SpriteFrames object.
Shouldn't it be simply checking whether it's a SpriteFrames different then the currently edited one? 🤔
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 tested that and the editor is clearing the frame before editing a new one.
Or at least does something that makes it work.
But yes, if it turns out broken, it should also check if the new SpriteFrames are different.
Thanks! |
Hello, it seems it still hasn't been fixed... #97577 |
Fixes #83256
Caused by sprite frames plugin handling AnimatedSprite and then the attached SpriteFrames.