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 read-only mode to AnimationTreeEditor plugins #63249

Merged

Conversation

SaracenOne
Copy link
Member

Adds a read-only mode to the AnimationTreeEditor plugin when operating on resources embedded in foreign scenes. Currently only covers state machines, blend 1D, and blend 2D editors. Blend trees do not currently support a read-only mode due to their reliance on the built-in GraphEdit control which does not yet have the required read-only state needed to prevent edits.

Complementary PR to #63245

@SaracenOne SaracenOne requested a review from a team as a code owner July 20, 2022 14:22
@SaracenOne SaracenOne force-pushed the animation_tree_editor_read_only branch 3 times, most recently from cea81a9 to bec7eca Compare July 21, 2022 14:53
@Chaosus Chaosus added this to the 4.0 milestone Aug 13, 2022
reduz
reduz previously requested changes Aug 19, 2022
editor/plugins/animation_blend_space_1d_editor.cpp Outdated Show resolved Hide resolved
@reduz
Copy link
Member

reduz commented Aug 19, 2022

Looks good to me save for minor nitpick, I think @TokageItLab should review too.

@TokageItLab
Copy link
Member

I have confirmed that the change to BlendSpace is prevented. But I concerned about the behavior of the BlendSpace point being canceled after it is grabbed. It would be better if it could be prevented from grabbing the point.

I am more concerned that #63245 is not working very well. I think that instead of preventing overwrites, #63245 should also cancel the user's operation, just like this PR.

@SaracenOne SaracenOne force-pushed the animation_tree_editor_read_only branch from bec7eca to 25bd697 Compare August 25, 2022 03:08
@SaracenOne SaracenOne requested review from a team as code owners August 25, 2022 03:08
@SaracenOne
Copy link
Member Author

SaracenOne commented Aug 25, 2022

Okay, @reduz, this has now been updated to address the issues @TokageItLab pointed out; the dragging of points in the blend spaces is no longer allowed, amongst other tweaks such as disabling a few buttons which I missed. I apologize that the read-only modes for blend spaces was so sloppy, I think that part of the code was a bit rushed on my behalf. I also went and replaced some of the duplicate code required for detecting if a resource is read_only with the new 'is_resource_read_only' function introduced in #63282

Aside from that, I've actually gone and ostensibly finished this PR by going ahead and adding read-only mode to the blend trees too, which did requires some minor tweaks the GraphEdit and GraphNode APIs, such as including a new 'draggable' and 'selectable' property for GraphNodes, and an option to hide the Arrange Nodes button in GraphEdit. I also introduced a new behaviour to GraphNode slots which does have accompanying documentation explaining it. The rule introduced is that slot types with negative numbers will not allow the user to create connections via the GraphEdit, which taking advantage of now allows the GraphEdit node in the BlendTreeEditor to be exist in a fully read-only state. My gut instincts tells me this feels more reasonable than shoe-horning a whole other variable for controlling this.

It also fixes a bug in the EditorSpinSplider which allowed the slider to still be moved in read-only mode.

@SaracenOne SaracenOne force-pushed the animation_tree_editor_read_only branch from 25bd697 to 435bf73 Compare August 25, 2022 03:26
@TokageItLab
Copy link
Member

TokageItLab commented Aug 25, 2022

Looks good except that the StateMachine's Node name is still editable.

@SaracenOne SaracenOne force-pushed the animation_tree_editor_read_only branch from 435bf73 to 75f1357 Compare August 25, 2022 15:29
@SaracenOne
Copy link
Member Author

Thanks @TokageItLab, I missed that 👀
Should be fixed now. Also cleaned up some minor UI jank with the text edit bar cursor showing up for the hardcoded uneditable 'start' and 'end' nodes.

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

Seems good.

@akien-mga akien-mga dismissed reduz’s stale review August 27, 2022 06:14

Changes done.

@akien-mga akien-mga merged commit f999845 into godotengine:master Aug 27, 2022
@akien-mga
Copy link
Member

Thanks!

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.

5 participants