Introduce EditorLanguage and move ScriptLanguage::complete_code into it.#117946
Introduce EditorLanguage and move ScriptLanguage::complete_code into it.#117946HolonProduction wants to merge 1 commit intogodotengine:masterfrom
EditorLanguage and move ScriptLanguage::complete_code into it.#117946Conversation
| * Note: This class is unstable and work-in-progress, stability is only ensured for the things currently exposed through `ScriptLanguageExtension`. | ||
| * The goal is to move all editor functionality out of `ScriptLanguage` over time. | ||
| */ | ||
| class EditorLanguage { |
There was a problem hiding this comment.
This name is a little generic and could be confused for localization.
What do you think about DocumentEditorLanguage? This way it is more connected to the built in editor too (after we rename the classes to use document #28607 (comment))
There was a problem hiding this comment.
My issue with a longer name is that it will be pretty annoying for accessing nested types and enums. Maybe we could put it in a namespace to disambiguate, but I guess that would be bad for exposing it.
There was a problem hiding this comment.
I don't think DocumentEditorLanguage is too long, its even a character shorter than ScriptLanguageExtension. There's also always autocomplete.
There was a problem hiding this comment.
That comparison isn't really fair. DocumentEditorLanguageExtension is longer than ScriptLanguageExtension. Although we might be able to just make the basetype hold all the GDVIRTUAL methods instead of having an extension wrapper 🤔 In that case DocumentEditorLanguage would be fine I guess.
HolonProduction
force-pushed
the
editor-language-kickoff
branch
from
a77f235 to
ac50500
Compare
kitbdev
left a comment
kitbdev
left a comment
There was a problem hiding this comment.
Looks good.
Can be continued in follow ups.
| @@ -36,6 +36,10 @@ | |||
| #include "core/variant/native_ptr.h" | |||
| #include "core/variant/typed_array.h" | |||
|
|
|||
| #ifdef TOOLS_ENABLED | |||
| #include "editor/script/editor_language.h" | |||
There was a problem hiding this comment.
You can't include editor files in core/.
There was a problem hiding this comment.
What kind of can't are we talking about? Is there a technical reason for it, or are we just talking about bad architecture?
There was a problem hiding this comment.
So architecture. Sure it's not measurable in includes, but the fact that editor functionality is part of ScriptLanguage is breaking encapsulation in a worse way than this IMO. As described in godotengine/godot-proposals#14566 the idea is to remove this association once all editor functionality has been removed (at this point the adapter code could be moved into the editor as well). So for me this is a worthwhile temporary tradeoff.
HolonProduction
force-pushed
the
editor-language-kickoff
branch
from
ac50500 to
1a2b1e5
Compare
4 participants
Initial boilerplate for godotengine/godot-proposals#14566
This PR introduces
EditorLanguageand movescomplete_codeinto it.Data structures related to completion remain in
ScriptLanguagefor the time being and will be moved in a followup.AI Disclosure
GH Copilot was used to do some of the refactoring and boilerplate. This was done in an interactive VSCode Chat session with automatic model selection. I did go into this with a clear idea and instructed the AI with individual steps for which I verified and adjusted the results. None of the comments are written by AI.