-
Notifications
You must be signed in to change notification settings - Fork 159
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
Editor: share base menu functionality between ScriptEditor and DialogEditor #1743
Editor: share base menu functionality between ScriptEditor and DialogEditor #1743
Conversation
I wonder if the autocomplete issues would be fixed after a rebase, since #1726 was made and merged shortly after this. |
I had plans to rebase this for 3.6.1, but autocomplete problems mentioned in this ticket will not be fixed, because #1726 did not address them in particular:
|
The ScriptEditorBase parent class implements menu commands shared between standard ScriptEditor and DialogEditor. Copied code mostly from the ScriptEditor, as its list of commands is more full. Few commands that are not applicable in DialogEditor (at least not yet) are added in override methods of ScriptEditor. Moved common operations (search, goto line, etc) to ScriptEditorBase.
8848ea2
to
0b3d4b9
Compare
Recreated for 3.6.1, using old pr as a reference, because some ScriptEditor code could have changed since. Requires testing. |
Testing with simple scripts, go-to definition looks like it's working. The text above of the PR mentions the following
Yes, this is true, but I had to read the issue above to figure this was a thing because ultimately I rarely declare variables locally in a dialog script, but instead mostly call functions that are either global or in objects defined elsewhere (like a character). Overall I really like this PR because it adds a lot of new functionality to the dialog editor without increasing code lines as it's just picking features from elsewhere and sharing it smartly. |
Yes, it's a quite straightforward change, should have added this to 3.6.0, but i did not expect it to be released about 6 months later. |
Resolves #1480
The problem with DialogEditor is that it's a stripped copy-paste version of the ScriptEditor (when it comes to working with script itself). So many menu commands that work in the ScriptEditor control are not available in DialogEditor.
The solution seem to be to create a parent class that shares a chosen functionality among these two.
In this PR, the parent class ScriptEditorBase is created, which has following functionality picked out from ScriptEditor and DialogEditor and merged:
The menu commands appear to work per se, but some of them have exceptions where they don't work. This is not related to command itself, but to internal functionality of the editor class. For instance, DialogEditor does not support AutoCompleteData for some reason (maybe because it's more complicated to parse dialog scripts). Because of that neither autocomplete nor "go to definition" will work on variables declared locally in dialog script. I tried to investigate this, but it seems like implementing this for dialogs would require picking out much more code from the ScriptEditor, and it's not clear how well that will work. So I'd rather leave that for future consideration.