Organize 3D editor code and rename a few 3D things #108767

aaronfranke · 2025-07-19T05:14:18Z

As a follow-up to some recent editor organization efforts, this PR organizes more of the 3D editor code.

The way I made this PR is by working on my own changes to allow disabling the 3D in the editor, but without any intention to merge those in. Instead, I want to only merge in organizational changes that makes sense even without that ability.

Rather than making a bulleted list, I have left review comments below. It may be a good idea to review this PR by reading those and using 👍 if that part looks good to you, or replying if not.

aaronfranke · 2025-07-19T05:16:16Z

core/debugger/engine_profiler.h


 #include "core/object/gdvirtual.gen.inc"
 #include "core/object/ref_counted.h"
+#include "core/object/script_language.h"


There are a small number of cases where a header needs a class to work, but wasn't including it. These things were being included transitively when 3D is enabled in the editor, but fail to compile otherwise. This PR fixes these cases by adding the includes, including this one include in the core folder, and a few others elsewhere.

aaronfranke · 2025-07-19T05:18:18Z

editor/animation/animation_player_editor_plugin.cpp

 	switch (p_what) {
 		case NOTIFICATION_ENTER_TREE: {
-			Node3DEditor::get_singleton()->connect(SNAME("transform_key_request"), callable_mp(this, &AnimationPlayerEditorPlugin::_transform_key_request));
+			Node3DEditor::get_singleton()->connect(SNAME("transform_3d_key_request"), callable_mp(this, &AnimationPlayerEditorPlugin::_transform_3d_key_request));


There were a few APIs in the animation editor, and this signal for communicating with Node3DEditor, with "transform" in the name, but these APIs only operate on 3D transform. I've renamed them with "3d" for clarity. These are internal APIs, not exposed anywhere.

editor/editor_interface.cpp

aaronfranke · 2025-07-19T05:24:25Z

editor/editor_node.cpp

+				CanvasItem *canvas_item = Object::cast_to<CanvasItem>(p_node);
+				if (canvas_item) {
+					new_additive_node_entry.transform_2d = canvas_item->get_transform();


This preserves the transform data of CanvasItems, though the data still doesn't end up being used currently. I didn't want to change too much in this PR but this would enable us to support Control nodes in EditorNode::reload_instances_with_path_in_edited_scenes() later if we wanted to.

aaronfranke · 2025-07-19T05:25:47Z

editor/editor_node.cpp

+
+		Ref<ORMMaterial3DConversionPlugin> orm_mat_convert;
+		orm_mat_convert.instantiate();
+		resource_conversion_plugins.push_back(orm_mat_convert);


This groups together EditorNode's registration of 3D material conversion plugins.

aaronfranke · 2025-07-19T05:33:22Z

editor/scene/3d/node_3d_editor_plugin.cpp

+void _insert_collision_object_rid_recursive(Node *p_node, HashSet<RID> &p_col_obj_rids) {
+	CollisionObject3D *col_obj = Object::cast_to<CollisionObject3D>(p_node);


Several renames for clarity, and to match Godot's p_ naming convention. node -> p_node, rids -> p_col_obj_rids, co -> col_obj, and below rids -> col_obj_rids_to_exclude.

aaronfranke · 2025-07-19T05:35:38Z

editor/scene/sprite_frames_editor_plugin.cpp

-	AnimatedSprite2D *as2d = Object::cast_to<AnimatedSprite2D>(selected);
-	AnimatedSprite3D *as3d = Object::cast_to<AnimatedSprite3D>(selected);
-	if (as2d || as3d) {
+	if (selected->has_method("get_sprite_frames")) {


This is using ->call("get_sprite_frames") immediately below, so this is less code and more straightforward of a check. Technically could allow user-made AnimatedSprite classes, but that wasn't a goal here, just a happy consequence.

aaronfranke · 2025-07-19T05:38:14Z

editor/scene/texture/texture_region_editor_plugin.cpp

-									} else if (res_atlas_texture.is_valid()) {
-										r = res_atlas_texture->get_region();
-									}
+									Rect2 r = _get_edited_object_region();


⚠️ This changes behavior, this part needs close review.

This change reduces code duplication, since this logic is already present in the _get_edited_object_region helper function. However, that function also contains this code:

const Ref<Texture2D> object_texture = _get_edited_object_texture(); if (region == Rect2() && object_texture.is_valid()) { region = Rect2(Vector2(), object_texture->get_size()); }

I think that's fine, since it only happens when region is a zero rect, and my guess is that using object_texture's rect as a fallback is better than no fallback, but I can't be certain of that.

I tested it and don't see any difference in behavior.

aaronfranke · 2025-07-19T05:38:44Z

editor/scene/texture/texture_region_editor_plugin.cpp

-						undo_redo->add_do_method(node_sprite_3d, "set_region_rect", node_sprite_3d->get_region_rect());
-						undo_redo->add_undo_method(node_sprite_3d, "set_region_rect", rect_prev);
-					} else if (node_ninepatch) {
+					if (node_ninepatch) {


In these if statements, put Control-based things first, and Node2D/Node3D things after it. This is arguably more organized, but the motivation here was so you can put #ifndef _2D_DISABLED above the } else if (condition) {.

aaronfranke · 2025-07-19T05:40:55Z

modules/csg/csg_shape.cpp

-		PhysicsServer3D::get_singleton()->body_add_shape(root_collision_instance, root_collision_shape->get_rid());
-		PhysicsServer3D::get_singleton()->body_set_space(root_collision_instance, get_world_3d()->get_space());
-		PhysicsServer3D::get_singleton()->body_attach_object_instance_id(root_collision_instance, get_instance_id());
+		root_collision_body = PhysicsServer3D::get_singleton()->body_create();


This variable is only ever null or the result of body_create(), so it's a body. The word "instance" is too generic, it's not clear if it's a body, area, shape, etc. Therefore, I renamed it.

editor/scene/canvas_item_editor_plugin.cpp

Calinou

Tested locally, it works as expected. Code looks good to me.

AThousandShips · 2025-07-28T10:08:25Z

editor/scene/canvas_item_editor_plugin.cpp

 				undo_redo->add_undo_method(this, "_set_owner_for_node_and_children", n2d, editor_root);
 			}
 			undo_redo->commit_action();
-


Unrelated noisy change IMO, I'd restore

AThousandShips · 2025-08-24T09:52:48Z

editor/plugins/editor_plugin.cpp


 		case CONTAINER_SPATIAL_EDITOR_MENU: {
 			Node3DEditor::get_singleton()->add_control_to_menu_panel(p_control);
-


I'd remove them, it's unnecessary noise for minor nitpicking change

AThousandShips · 2025-08-24T09:53:30Z

editor/editor_node.cpp

 				// Make sure MeshLibrary is updated in the editor.
 				ResourceLoader::load(p_file)->reload_from_file();
 			}
-


Please restore

aaronfranke added this to the 4.6 milestone Jul 19, 2025

aaronfranke requested review from a team as code owners July 19, 2025 05:14

aaronfranke added the enhancement label Jul 19, 2025

aaronfranke requested review from a team as code owners July 19, 2025 05:14

aaronfranke added the topic:editor label Jul 19, 2025

aaronfranke requested review from a team as code owners July 19, 2025 05:14

aaronfranke added the topic:codestyle label Jul 19, 2025

aaronfranke requested review from a team as code owners July 19, 2025 05:14

aaronfranke added the topic:3d label Jul 19, 2025

aaronfranke force-pushed the editor-3d-org branch from 3667a95 to b60ef17 Compare July 19, 2025 05:20

aaronfranke commented Jul 19, 2025

View reviewed changes

aaronfranke requested a review from KoBeWi July 21, 2025 16:24

This comment was marked as resolved.

Sign in to view

KoBeWi reviewed Jul 25, 2025

View reviewed changes

editor/scene/canvas_item_editor_plugin.cpp Outdated Show resolved Hide resolved

aaronfranke force-pushed the editor-3d-org branch from b60ef17 to 3525dcf Compare July 27, 2025 23:54

KoBeWi approved these changes Jul 28, 2025

View reviewed changes

aaronfranke force-pushed the editor-3d-org branch from 3525dcf to 49eed2c Compare August 23, 2025 00:47

aaronfranke force-pushed the editor-3d-org branch from 49eed2c to 73478e9 Compare September 25, 2025 03:15

Calinou approved these changes Sep 25, 2025

View reviewed changes

AThousandShips reviewed Sep 25, 2025

View reviewed changes

Organize 3D editor code and rename a few 3D things

b6fe79b

aaronfranke force-pushed the editor-3d-org branch from 73478e9 to b6fe79b Compare September 25, 2025 15:01

		void _insert_collision_object_rid_recursive(Node *p_node, HashSet<RID> &p_col_obj_rids) {
		CollisionObject3D *col_obj = Object::cast_to<CollisionObject3D>(p_node);


		case CONTAINER_SPATIAL_EDITOR_MENU: {
		Node3DEditor::get_singleton()->add_control_to_menu_panel(p_control);

Uh oh!

Organize 3D editor code and rename a few 3D things #108767

Are you sure you want to change the base?

Organize 3D editor code and rename a few 3D things #108767

Conversation

aaronfranke commented Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Calinou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aaronfranke commented Jul 19, 2025 •

edited

Loading