Fix Skeleton3D editor crash regression after #76592

[spanzeri]

Control notifies a theme changed before the editor has entered the tree,

Fixes: #77046

However, I am not sure why Control notifies THEME_CHANGED before propagating ENTER_TREE to subclasses.
Note that moving the editor creating back to where it was before my change cause theme resources to be accessed before they are ready.
I could use someone with more expertise on theme changes and UI to double check if the fix makes sense or if there is a better way to handle this :)

[lyuma]

is_inside_tree() does not solve it.
It appears that is_inside_tree() is returning true, even before NOTIFICATION_ENTER_TREE has been delivered. Something really confusing changed between RC2 and RC3 in terms of the ordering of notifications.

See the debugger here:

Here you can see how the events are being propagated:

The ones marked with v are _notificationv which first propagates to parent classes before calling the Skeleton3DEditor::_notification function we worked on in this PR.

You can see that it attempted to call NOTIFICATION_ENTER_TREE, but in the middle of executing that notification, it decided to deliver a NOTIFICATION_THEME_CHANGED which arrives in Skeleton3DEditor::_notification before the NOTIFICATION_ENTER_TREE does, but is_inside_tree() is already true.

[spanzeri]

@lyuma thanks for pitching in. I did test with is_inside_tree and that worked for me in latest master, so I am not sure why it is behaving differently.
Regarding the notifications and their order you are 100% right and that is what I was trying to explain in the original comment.
Unfortunately, I find myself in a chicken and egg situation with this one where either I create the editor on construction, which cause theme resources to be accessed before they are ready, or on enter tree but that means I get a theme change notification before the editor is created.
Note that this is very specific of control nodes, because it's the control base class that fires the theme change notification from enter tree.
But as you mentioned, this code pre-dates my changes, so something else must have changed in RC3 and I am not exactly sure what that is :)

[lyuma]

Looks good. This change effectively reverts the Skeleton3DEditor portion of #76592 and replaces it with using a temporary theme color, and updating the editor's color in NOTIFICATION_THEME_CHANGED

Here is the actual diff relative to 4.0.2

git diff 4.0.2-stable..spanzeri/fix-theme-crash-skeleton-editor editor/plugins/skeleton_3d_editor_plugin.cpp

diff --git a/editor/plugins/skeleton_3d_editor_plugin.cpp b/editor/plugins/skeleton_3d_editor_plugin.cpp
index 120cfbdefb..90f054275a 100644
--- a/editor/plugins/skeleton_3d_editor_plugin.cpp
+++ b/editor/plugins/skeleton_3d_editor_plugin.cpp
@@ -43,6 +43,7 @@
 #include "scene/3d/mesh_instance_3d.h"
 #include "scene/3d/physics_body_3d.h"
 #include "scene/gui/separator.h"
+#include "scene/gui/texture_rect.h"
 #include "scene/resources/capsule_shape_3d.h"
 #include "scene/resources/skeleton_profile.h"
 #include "scene/resources/sphere_shape_3d.h"
@@ -695,9 +696,6 @@ void Skeleton3DEditor::update_joint_tree() {
 	}
 }
 
-void Skeleton3DEditor::update_editors() {
-}
-
 void Skeleton3DEditor::create_editors() {
 	set_h_size_flags(SIZE_EXPAND_FILL);
 	set_focus_mode(FOCUS_ALL);
@@ -797,10 +795,8 @@ void Skeleton3DEditor::create_editors() {
 	animation_hb->add_child(key_insert_all_button);
 
 	// Bone tree.
-	const Color section_color = get_theme_color(SNAME("prop_subsection"), SNAME("Editor"));
-
-	EditorInspectorSection *bones_section = memnew(EditorInspectorSection);
-	bones_section->setup("bones", "Bones", skeleton, section_color, true);
+	bones_section = memnew(EditorInspectorSection);
+	bones_section->setup("bones", "Bones", skeleton, Color(0.0f, 0.0, 0.0f), true);
 	add_child(bones_section);
 	bones_section->unfold();
 
@@ -832,7 +828,6 @@ void Skeleton3DEditor::_notification(int p_what) {
 	switch (p_what) {
 		case NOTIFICATION_ENTER_TREE: {
 			update_joint_tree();
-			update_editors();
 
 			joint_tree->connect("item_selected", callable_mp(this, &Skeleton3DEditor::_joint_tree_selection_changed));
 			joint_tree->connect("item_mouse_selected", callable_mp(this, &Skeleton3DEditor::_joint_tree_rmb_select));
@@ -857,6 +852,7 @@ void Skeleton3DEditor::_notification(int p_what) {
 			key_scale_button->set_icon(get_theme_icon(SNAME("KeyScale"), SNAME("EditorIcons")));
 			key_insert_button->set_icon(get_theme_icon(SNAME("Key"), SNAME("EditorIcons")));
 			key_insert_all_button->set_icon(get_theme_icon(SNAME("NewKey"), SNAME("EditorIcons")));
+			bones_section->set_bg_color(get_theme_color(SNAME("prop_subsection"), SNAME("Editor")));
 
 			update_joint_tree();
 		} break;

[akien-mga]

Nitpick, but the commit message and PR title are slightly wrong, as it's not crashing in RC3, but in 4.0.3 RC2.

Generally speaking the version isn't really that relevant to mention in the commit log, and the bug also happens in 4.1 dev. It could just be "Fix Skeleton3D editor crash regression".

[akien-mga]

Thanks!

[YuriSizov]

Cherry-picked for 4.0.3.