godot
godot copied to clipboard
Editor: Restructure editor code
As discussed with @akien-mga the editor code is very cluttered, which makes it both difficult to assign code owners to, but also makes it hard to navigate
Opening as a draft to discuss the specifics of how to organize things, where I still have some areas open for discussion:
- [x] The structuring of sub folders of
/editor/, currently these are organized mainly by type, not area, this could be changed to be more area specific (i.e. all dialogs are in/editor/dialogs/rather than specifically/editor/animation/etc.) - [x] If some files should be moved between sub folders, for example some files that are in
/editor/gui/that might be better in/editor/dialogs/ - [ ] If some files that are under
/editor/plugins/should be moved out of there and into the main sub folders, like the dialogs under/editor/plugins/tiles/ - [X] How and if CODEOWNERS should be restructured, currently it uses relatively rough glob patterns to handle files, this could now be made more specific (this will be fixed in a follow up PR beyond the basic changes in this, which can be done without any noise)
- [ ] If we should use more relative paths, as you can see several paths were made more specific (including cases where the file references was moved to the same folder, making the specification unnecessary)
Generally looking for feedback on these specifics, but also generally, in my opinion this is badly needed to ensure the editor code remains manageable as it grows more complicated, currently the majority of files in /editor/ lack owners, making review more complicated
- Supersedes: https://github.com/godotengine/godot/pull/83161
I was about to comment that we should merge #83161 before this PR, but then I saw you linking to it. Though, I still want to say that we should merge #83161 before this PR :)
I think this is a step in the right direction, the only thing I dislike the idea of grouping things by "type" of GUI element. I don't think dialogs should be grouped into the same folder, and same for docks.
Instead, I think we simply should have per-topic sorting. For example the inspector dock, and all inspector-related dialogs should go in the inspector folder I believe. Similarly, all filesystem-related files can go in a filesystem folder I believe.
I think we shouldn't be afraid of adding a lot more folders though (including subfolders). I think we can add, for example, a settings folder, maybe with an input subfolder, things like that.
Agreed, picked this approach at first because it was the most straightforward, and I'm leaning towards switching that out, will hold off slightly but will look at resturcturing next week probably if we agree on that switch
It would be nice to have some visualization of before and after the changes. GitHub isn't very friendly for previewing the changes.
Can do a summary
Will push some changes soon and add some tree views of the differences
What effect does it have on scu_build? docs
If there are more subfolders, this increases the build times?
https://github.com/godotengine/godot/blob/master/scu_builders.py#L292-L303
process_folder(["editor"], ["file_system_dock", "editor_resource_preview"], 32)
process_folder(["editor/debugger"])
process_folder(["editor/debugger/debug_adapter"])
process_folder(["editor/export"])
process_folder(["editor/gui"])
process_folder(["editor/themes"])
process_folder(["editor/project_manager"])
process_folder(["editor/import"])
process_folder(["editor/import/3d"])
process_folder(["editor/plugins"])
process_folder(["editor/plugins/gizmos"])
process_folder(["editor/plugins/tiles"])
Will try and compare, good point!
I made a before and after using this script:
@tool
extends Tree
@export_global_dir var base_folder: String
func _ready() -> void:
add_folder(base_folder, null)
func add_folder(path: String, parent: TreeItem):
var folder_item := create_item(parent)
folder_item.set_text(0, path.get_file())
folder_item.set_custom_color(0, Color.YELLOW)
for dir in DirAccess.get_directories_at(path):
add_folder(path.path_join(dir), folder_item)
for file in DirAccess.get_files_at(path):
if file.get_extension() != "h":
continue
var file_item := create_item(folder_item)
file_item.set_text(0, file.get_basename())
if folder_item.get_child_count() == 0:
folder_item.free()
Set base_folder to editor in the source code and it will make a huge folder tree 🙃
Comparison, but it's really big.
| Before | After |
|---|---|
I took a look at the new structure.
- ~~There is some inconsistency between
dialogsandgui. E.g.editor_file_dialogis ingui, whilecreate_dialog, which is also technically a reusable GUI control, is insidedialogs.~~- ~~I think
dialogsfolder might be too general. There are some dialogs that belong to docks (connections_dialog), to script editor (find_in_files), to shader editor (editor_native_shader_source_visualiser, I think) etc. They should be together with their base components. I'd put only "unique" and "top-level" dialogs intodialogsfolder (editor_about,editor_feature_profileetc.)~~ Addressed, butfind_in_filesis in the wrong place. It's part of script editor.
- ~~I think
- ~~Should
physicsandnavigationgo under2d/3d, or should they be standalone folders? I think making them standalone makes more sense, although the current structure makes it easier to disable 2D and 3D separately.~~ - ~~Texture plugins could get their own folder.~~
- ~~EditorHelp stuff and DocData could be put into their own folder (together I mean).~~
- ~~
project_managercould be inproject_managerfolder.~~ - ~~
project_upgrade_tooland3_to_4stuff can be in aproject_upgradefolder maybe~~ - ~~
extractable_translations.genlooks like it belongs totranslations~~ - ~~
file_infois part ofeditor_file_dialogandfile_system_dock. There might be better place for it than top level~~ - ~~Might be worth it to add some folder in
editor_pluginsfor files that aren't specific plugins? Likeeditor_plugin,editor_plugin_settings, but alsoeditor_preview_pluginsoreditor_resource_tooltip_plugins. They look like they can be grouped together.~~ - ~~
shader_globals_editorcould be inshaderfolder, except it's not part of the plugin., so not sure.~~ - ~~
engine_update_labelcan be insideproject manager, it's not used anywhere else.~~ - ~~
code_editorshould be inscript, although technically it's also used by shader editor. Maybegui` then?~~ - ~~
event_listener_line_editshould be ingui.~~ - ~~
register_exporterscould be inexport, unless there is a reason why it's where it is.~~
I could see more potential groups in the top-level folder, but it's arguable.
Note that the dialogs folder has been reverted
EditorHelp and stuff is in a new folder, event_listener_line_edit is currently under a new settings folder, but can move it to gui
Will add the other suggestions and push in the next few days, possibly tomorrow
There, made a major restructure, dropped the old "dialogs" division, and moved into a few new categories
Still to do:
- [ ] Decide on moving
editor/plugins/editor_plugin.hand similar, this is used everywhere and is also used in custom modules, so it's risky, and the returns are limited, but it would improve organization - [ ] Decide on any missing categories, and some division between them
- [x] ~Move some code from
editor/SCsubto other parts, the script parts generating documentation etc.~ Tested this and wasn't convenient - [ ] Figure out some further specifics on
CODEOWNERS
Otherwise this is nearly in what I'd prefer as the final state
CC @KoBeWi @akien-mga @Repiteo
The new structure:
Unfold
- Not sure about
textures/, it sounds like a folder with textures.texture/could be better, if it's fine. color_channel_selectorshould be intextures/.canvas_item_editor_pluginshould be inplugins/2d.- Some
.genfiles weren't moved to proper subfolders and are still ineditor/ editor_build_profilecould go tosettings/
Also some of my previous comments still apply, but overall this new structure looks much better.
Some .gen files weren't moved to proper subfolders and are still in
editor/
Have you made sure to clean before building? They have been moved
canvas_item_editor_pluginshould be inplugins/2d.
It belongs in the base as it isn't 2D or GUI, same reasoning why canvas_item was moved to scene/main for 4.0
Also some of my previous comments still apply
Can you please point to which, I've resolved most or responded reasons why they weren't
For the prioritizing physics vs 2D/3D I prefer keeping it as is, or possibly adding a sub-folder, I prefer having things closer to the way scene is organized, it helps navigation
Have you made sure to clean before building? They have been moved
I didn't. I actually didn't even build, so this is probably why 🙃
Can you please point to which, I've resolved most or responded reasons why they weren't
I crossed out resolved stuff.
There are some areas to still figure out the details of (especially CODEOWNERS), but this is ready for deeper review
Latest structure (this time properly includes all changes):
Unfold
- [x] Why is
doc_data_compressed.genintranslations/? - [x]
dependency_editorbelongs tofile_system/. - [x]
editor_foldingcan be insettings/. - [x]
window_wrappercould be ingui/I think?
Aside from that it's probably fine. Maybe there could be folder for audio and assetlib stuff, but these files are in different folders, so not sure.
Will update! doc_data_compressed.gen must have been a mistake don't remember moving it there!
Audio makes sense, two files should be enough to move, in both plugins and the root, will update and rebase soon
Moved the suggested, as well as audio both in editor/ and editor/plugins/, as well as credits_roll into editor/gui/
~Added scu for editor/plugins/script/ which was excluded previously, for these changes my condition is three or more files to justify it,~ adding SCU for these cases as it saves a bit of binary size, will compare build times later to validate
This restructure adds about 0.45% to the size of a Windows dev build of the editor, ~comparing production builds as well~ production builds are unchanged
That's a good point, will merge those
The gui/code_editor, script/find_in_files, and the plugins/text_editor can all be in plugins/script, since they are all part of the script editor plugin. The code_editor is also used by the shader editor, but I think its better to have it in the script folder than having it in a general folder.
I'll leave files from editor/ outside editor/plugins/, they're not part of the plugins, and I'd prefer to keep editor/plugins/text_editor where it is to minimize confusion
The editor/plugins/text_editor is just like the editor/plugins/script/script_text_editor, and it is only used by the script editor. It also isn't a plugin by itself, so I'm not sure why it should be outside of the script plugin folder.
The editor/script/find_in_files is currently created and managed by the script editor as well, it cannot work alone, so I'd say it is part of the plugin.
Can move the text editor, given its scope, but I'll leave the rest
While discussing this the suggestion was again brought up to simply break up the editor/plugins/ folder entirely, keeping only "plugins" things (like editor_plugin and editor_plugin_settings for example) in that folder, and putting the rest in the relevant base folders, like audio, and adding new categories for the stuff still in editor/plugins/, this won't be a lot of new work but would be good to see if this is desired
My suggestion for this would be:
editor/plugins/shader_baker/->editor/shader/shader_baker/editor/plugins/shader_baker_export_plugin->editor/export/editor/plugins/gdextension_export_plugin->editor/export/editor/plugins/2d->editor/scene/2d/editor/plugins/3d->editor/scene/3d/editor/plugins/gui->editor/scene/gui/editor/plugins/texture->editor/scene/texture/editor/plugins/tiles->editor/scene/2d/tiles/- Debugger plugins into
editor/debugger/ editor/plugins/game_view_plugin->editor/run/editor/plugins/packed_scene_translation_parser_plugin->editor/translations/editor/plugins/dedicated_server_export_plugin->editor/export/editor/plugins/sub_viewport_preview_editor_plugin->editor/inspector/editor/plugins/editor_preview_plugins->editor/inspector/editor/plugins/editor_resource_conversion_plugin->editor/inspector/editor/plugins/editor_context_menu_plugin->editor/inspector/editor/plugins/tool_button_editor_plugin->editor/inspector/editor/plugins/editor_resource_tooltip_plugins->editor/inspector/editor/editor_asset_installer->editor/asset_library/editor/plugins/asset_library_editor_plugin->editor/asset_library/- Remaining scene related plugins into
editor/scene/ editor/editor_vcs_interface->editor/version_control/editor/plugins/version_control_editor_plugin->editor/version_control/editor/plugins/gizmos/->editor/scene/3d/gizmos/editor/plugins/embedded_process->editor/editor/plugins/input_event_editor_plugin->editor/inspector/
This description will probably no longer up-to-date after the PR merge. https://docs.godotengine.org/en/stable/contributing/development/editor/introduction_to_editor_development.html
(Last updated 2 years ago)
If there's no objections to the restructure suggested above I'll see about trying it out this weekend, it shouldn't be too hard to rework
Thank you for reminding me about that documentation, will look at updating that following this!
I incorporated the new changes, plugins is now essentially clean, and the structure is far more organized, and things are more or less in one place, i.e. there's no separation between plugins and other folders for the same area like animation
Some things could still be restructured but this should be a pretty good start, some content in editor/gui/ could still be divided out, but I think this is a good point
In a windows dev build the size increase is now just 0.4%, due to some SCU files being smaller
Will try to make a comparison of compile time with SCU from scratch
Latest structure for reference:
Unfold
Will rebase and fix the final details! Will be great to have this done, but it's been an interesting experience and I now know the editor even better!