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

Convert usages of EditorNode to EditorPlugin #58506

Closed

Conversation

trollodel
Copy link
Contributor

@trollodel trollodel commented Feb 24, 2022

Warning: This can't be merged as-is because there are some heap-use-after-free caused by deleted EditorPlugins and EditorInterface. I need to figure out how to fix them.

Follow up of #57306

This PR remove direct usages of EditorNode in favor of EditorPlugin and EditorInterface methods.
Where necessary, the EditorPlugin pointer is stored in the class where necessary (pretty much how it was done for EditorNode before #57306).
In some cases, I create local variables because the new syntax is too long (and it was too long).

Also, it reduces the EditorPlugin includes to 3 by forward-declaring everything else. EDIT: Now done in #60684


Note for reviewers:
There are some inconsistencies, unrelated changes, missing and wrong changes in the PR. Here's a list:

  • The plugin pointer should be typed EditorPlugin, not SomethingEditorPlugin. Using the real type is useless and requires a forward declaration.
  • In several cases I renamed the local variables, but It's not done consistently.
  • The EditorNode method names differs from EditorInterface ones, so it's possible that the picked method is wrong.
    To check that, compare the old name with the one in the EditorInterface's method body. If they differ, the used EditorInterface method is wrong.
    For example, EditorInterface::get_base_control calls EditorNode::get_gui_base

Many methods in EditorNode don't have their counterparts in EditorInterface, which I want to add in a follow-up PR to further reduce the (direct) dependencies to EditorNode.
Here's a list of the methods:

EditorNode::show_warning
EditorNode::show_accept
EditorNode::load_resource
EditorNode::save_resource
EditorNode::save_resource_as
EditorNode::set_visible_editor
EditorNode::get_editor_plugin_screen
EditorNode::push_item
EditorNode::get_scene_root
EditorNode::load_scene
EditorNode::get_object_icon
EditorNode::get_editor_plugins_over
EditorNode::get_editor_plugins_force_over
EditorNode::get_editor_history
EditorNode::drag_resource
EditorNode::is_scene_open
EditorNode::show_accept
EditorNode::is_changing_scene
EditorNode::save_scene_list
EditorNode::get_editor_data (can't be exposed as-is)
EditorNode::get_theme_base
EditorNode::get_log

This PR decreases the EditorNode::get_singleton usages.

EditorNode::get_singleton usages in master (4dc8214):

rg "EditorNode::get_singleton()" | wc -l
1003

EditorNode::get_singleton usages in this PR:

rg "EditorNode::get_singleton()" | wc -l
713

@Chaosus Chaosus added this to the 4.0 milestone Feb 24, 2022
@trollodel trollodel force-pushed the editor_node_to_editor_plugin branch 3 times, most recently from f2fc40a to dad9e13 Compare March 20, 2022 13:41
@trollodel
Copy link
Contributor Author

This has too many conflicts, closing.

Some changes will be redone in smaller PRs.

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