Remove editor/ dependencies from scene/ files

[akien-mga]

Godot version:
3.2 master (f410e7a)

Issue description:
As discussed with @reduz on IRC today (logs from 19:37), core files in scene/ should not have any dependency on editor code (and thus headers from editor/).

Over the years we merged a few changes which add such dependencies, guarded by #ifdef TOOLS_ENABLED and runtime checks to make sure they only impact the editor, but they still break the design of each folder/category being independent with a single dependency order: editor -> scene -> servers -> core.

List of "core" files with dependencies on editor headers:

$ rg -g'!editor' -g'!modules' -g'!main' -g'!platform' include.*editor | sort              
core/os/input.cpp:#include "editor/editor_settings.h"
scene/2d/camera_2d.cpp:#include <editor/editor_node.h>
scene/2d/path_2d.cpp:#include "editor/editor_scale.h"
scene/3d/physics_body.cpp:#include "editor/plugins/spatial_editor_plugin.h"
scene/animation/animation_player.cpp:#include "editor/editor_settings.h"
scene/gui/color_picker.cpp:#include "editor_scale.h"
scene/gui/color_picker.cpp:#include "editor_settings.h"
scene/gui/control.cpp:#include "editor/editor_settings.h"
scene/gui/control.cpp:#include "editor/plugins/canvas_item_editor_plugin.h"
scene/gui/dialogs.cpp:#include "editor/editor_node.h"
scene/gui/gradient_edit.cpp:#include "editor/editor_scale.h"
scene/gui/graph_edit.cpp:#include "editor/editor_scale.h"
scene/gui/line_edit.cpp:#include "editor/editor_scale.h"
scene/gui/line_edit.cpp:#include "editor/editor_settings.h"
scene/gui/rich_text_label.cpp:#include "editor/editor_scale.h"
scene/gui/text_edit.cpp:#include "editor/editor_scale.h"
scene/gui/tree.cpp:#include "editor/editor_node.h"
scene/main/node.cpp:#include "editor/editor_settings.h"
scene/main/scene_tree.cpp:#include "editor/editor_node.h"
scene/resources/material.cpp:#include "editor/editor_settings.h"
servers/audio/effects/audio_effect_record.h:#include "editor/import/resource_importer_wav.h"

(modules/, platform/ and main/ are further up the dependency chain, so they can depend on editor/).

We should discuss how to remove those depending on the different use cases, and clean this up.

[akien-mga]

Among the files listed above, several of them have an editor dependency for the EDSCALE factor used for HiDPI support in the editor. In scene/:

2d/path_2d.cpp
gui/color_picker.cpp
gui/dialogs.cpp
gui/gradient_edit.cpp
gui/gradient_edit.h
gui/graph_edit.cpp
gui/line_edit.cpp
gui/rich_text_label.cpp
gui/text_edit.cpp
gui/tree.cpp

Instead of using EDSCALE, these Controls should register new constants in their theme, and the relevant EDSCALE-based values can be set in the editor theme.
That's a first step for solving this issue which can be done independently of the rest.

[akien-mga]

Another chunk is all the classes which access EditorSettings to customize their behavior when used in the editor. In scene/:

$ rg EditorSettings\|EDITOR_DEF | sort
animation/animation_player.cpp: const String quote_style = EDITOR_DEF("text_editor/completion/use_single_quotes", 0) ? "'" : "\"";
gui/color_picker.cpp:                   EditorSettings::get_singleton()->set_project_metadata("color_picker", "presets", arr_to_save);
gui/color_picker.cpp:           EditorSettings::get_singleton()->set_project_metadata("color_picker", "presets", arr_to_save);
gui/color_picker.cpp:                           PoolColorArray saved_presets = EditorSettings::get_singleton()->get_project_metadata("color_picker", "presets", PoolColorArray());
gui/control.cpp:        const String quote_style = EDITOR_DEF("text_editor/completion/use_single_quotes", 0) ? "'" : "\"";
gui/line_edit.cpp:                              cursor_set_blink_enabled(EDITOR_DEF("text_editor/cursor/caret_blink", false));
gui/line_edit.cpp:      cursor_set_blink_enabled(EDITOR_DEF("text_editor/cursor/caret_blink", false));
gui/line_edit.cpp:                              cursor_set_blink_speed(EDITOR_DEF("text_editor/cursor/caret_blink_speed", 0.65));
gui/line_edit.cpp:      cursor_set_blink_speed(EDITOR_DEF("text_editor/cursor/caret_blink_speed", 0.65));
gui/line_edit.cpp:                                      EditorSettings::get_singleton()->connect("settings_changed", this, "_editor_settings_changed");
gui/line_edit.cpp:                              if (!EditorSettings::get_singleton()->is_connected("settings_changed", this, "_editor_settings_changed")) {
main/node.cpp:  const String quote_style = EDITOR_DEF("text_editor/completion/use_single_quotes", 0) ? "'" : "\"";
resources/material.cpp: const String quote_style = EDITOR_DEF("text_editor/completion/use_single_quotes", 0) ? "'" : "\"";

And in core/:

os/input.cpp:   const String quote_style = EDITOR_DEF("text_editor/completion/use_single_quotes", 0) ? "'" : "\"";

[akien-mga]

The last chunk is access to EditorNode or icons from the editor theme (and some false positives in that list, like ADD_GROUP("Editor", "") which is legit). In scene/:

 rg Editor[^S] | sort
2d/camera_2d.cpp:       ADD_GROUP("Editor", "editor_");
3d/light.cpp:   ADD_GROUP("Editor", "");
3d/physics_body.cpp:    SpatialEditor::get_singleton()->update_transform_gizmo();
gui/color_picker.cpp:           text_type->set_icon(get_icon("Script", "EditorIcons"));
gui/control.cpp:        set_position((get_position() + get_transform().basis_xform(p_edit_rect.position)).snapped(Vector2(1, 1)), CanvasItemEditor::get_singleton()->is_anchors_mode_enabled());
gui/control.cpp:        set_position(p_position, CanvasItemEditor::get_singleton()->is_anchors_mode_enabled());
gui/control.cpp:        set_size(p_edit_rect.size.snapped(Vector2(1, 1)), CanvasItemEditor::get_singleton()->is_anchors_mode_enabled());
gui/dialogs.cpp:                                EditorNode::get_singleton()->dim_editor(false);
gui/dialogs.cpp:                                EditorNode::get_singleton()->dim_editor(true);
gui/dialogs.cpp:                        if (get_tree() && Engine::get_singleton()->is_editor_hint() && EditorNode::get_singleton())
gui/dialogs.cpp:                        if (get_tree() && Engine::get_singleton()->is_editor_hint() && EditorNode::get_singleton())
gui/graph_edit.cpp:                             get_color("accent_color", "Editor") * Color(1, 1, 1, 0.375));
gui/text_edit.cpp:      cache.executing_icon = get_icon("MainPlay", "EditorIcons");
gui/text_edit.cpp:      cache.folded_eol_icon = get_icon("GuiEllipsis", "EditorIcons");
main/node.h:    friend class SceneTreeEditor;
main/scene_tree.cpp:                            EditorNode::get_singleton()->show_about();
main/scene_tree.cpp:                    if (EditorNode::get_singleton()) {

[akien-mga]

Some more comments on some of the cases listed above:

• scene/3d/physics_body.cpp has gizmo-related code for PhysicalBone calling a SpatialEditor method. I guess it should be ported to the new gizmo system in editor/spatial_editor_gizmos.cpp. @AndreaCatania @JFonS
• core/os/input.cpp, scene/animation/animation_player.cpp, scene/gui/control.cpp, scene/main/node.cpp and scene/resources/material.cpp have code calling const String quote_style = EDITOR_DEF("text_editor/completion/use_single_quotes", 0) ? "'" : "\"";, introduced in Add settings for single-quotes on completion #24437. Instead, this should be moved to a TextEdit property, which can then be defined in editor/code_editor.cpp in CodeTextEditor::update_editor_settings(), where many properties like that are defined based on editor properties. @mateusfccp @Paulb23
• scene/gui/line_edit.cpp also refers to EditorSettings for its caret blink feature. Not sure how to solve that one. @Paulb23
• gui/control.cpp has code referring to CanvasItemEditor in its _edit_* methods, this should be done differently. @groud
• gui/dialogs.cpp has code referring to EditorNode::dim_editor() added back in 2017 with Editor: Dim UI when a WindowDialog is shown. #7970. I assume this could be replaced by a signal emitted by WindowDialog and caught by EditorNode? @Hinsbart
• servers/audio/effects/audio_effect_record.h calls ResourceImporterWAV::_compress_ima_adpcm() directly, which is from editor/import/resource_importer_wav.h.

[pouleyKetchoupp]

Extra cases to add to the list after merging #47872 (see #40347 for more detailed discussions):

• This will probably require to add proper support for 2D gizmo plugins.

godot/scene/2d/skeleton_2d.cpp

Lines 192 to 195 in 94c31ba

	// Bone2D Editor gizmo drawing:
	#ifndef _MSC_VER
	#warning TODO Bone2D gizmo drawing needs to be moved to an editor plugin
	#endif

• And in the same file, usage of NOTIFICATION_EDITOR_PRE_SAVE & NOTIFICATION_EDITOR_POST_SAVE notifications used to reset the skeleton while saving the scene should be reviewed to see if there could be a better solution:

godot/scene/2d/skeleton_2d.cpp

Lines 187 to 191 in 94c31ba

	else if (p_what == NOTIFICATION_EDITOR_PRE_SAVE || p_what == NOTIFICATION_EDITOR_POST_SAVE) {
	Transform2D tmp_trans = get_transform();
	set_transform(cache_transform);
	cache_transform = tmp_trans;
	}

[akien-mga]

Superseded by #53295 (comment)