goostengine / goost

A general-purpose, extensible and customizable C++ extension for Godot Engine.
https://goostengine.github.io/
MIT License
481 stars 18 forks source link

Mixinscript editor workflow tweaks #147

Closed ghost closed 3 years ago

ghost commented 3 years ago

As discussed previously, I have made some small usability enhancements to the Mixin Script Editor.

  1. Opening a .ms MixinScript from Script Editor, File System dock or Node tree always loads the Mixin Script Editor panel.
  2. Double clicking on a Mixin item from the panel automatically loads the script.

I have tried adding a setting to the editor to open the last edited script but my approach felt too hacky so I discarded that part and only pulling the workflow tweaks.

Long version: esentially what is happening is that the MixinScriptEditor loads only when a .ms file is opened. So it never sets that setting for first time users. I was forced to put it in MixinScriptEditorPlugin which loads right away and adds that setting. You would need some new approach either load MixinScriptEditor earlier or have a whole new class that handles MixinScript settings. Settings cannot be done at class registration phase either probably because EditorSettings is not loaded yet or I didn't do it properly.

a

There is a signal "open_script" emitted by the Node Tree dock but I could not reach it. I think it may be internal. I think it may be possible to bring that open last script behavior only when opening the script from the Node Tree dock. This might actually be a better approach I think.

Xrayez commented 3 years ago

I understand the desire for the MixinScriptEditor to always load the panel as it makes the behavior consistent with other script editors, but ideally we should have both approaches in place, it's just that reusing Godot's UI is what leads to this confusion.

To reiterate what I said in the chat, the idea is that MixinScript is simply a collection of scripts that will be rarely edited, the main focus would fall onto one of the scripts that MixinScript holds during development. In fact, while I worked on this feature, I've even come up with the notion of the "main script" and even added it as a property (see https://github.com/goostengine/goost/commit/4a2c986d065ca8feb5ba8cc868c68a62b0a5c55d), but I then removed it in https://github.com/goostengine/goost/commit/613889b1245c002c4f397c77f85f2e79abe25ea6 (read commit messages for rationale).

Having said that, I really think an editor setting should be added in order to retain current functionality. Simply adding a boolean option would be enough. I think the best place to add this would be in text_editor/files setting:

image

I would name the new option as text_editor/files/open_first_script_on_editing_mixin_script, or something like this.

Having it disabled by default is also alright for now, unless other users prefer to enable it by default.

ghost commented 3 years ago

I added the setting property which changes the behavior to the way it was before (with the option turned on) or the new behavior (with the option turned off). Reintroduces the editor meta setting to maintain correct behavior when opening from Inspector. By default is set to false. Let me know :)

image

ghost commented 3 years ago

I addressed the last 2 issues in the last push :)

Xrayez commented 3 years ago

Thanks and congrats for your first contribution!