Rework MeshInstance3d collision shape creation into a subMenu and add all static body options

Apply suggestions from code review

Co-Authored-By: A Thousand Ships <[email protected]>

editor/plugins/mesh_instance_3d_editor_plugin.cpp

@@ -59,200 +59,159 @@ void MeshInstance3DEditor::edit(MeshInstance3D *p_mesh) {
 	node = p_mesh;
 }
 
-void MeshInstance3DEditor::_menu_option(int p_option) {
-	Ref<Mesh> mesh = node->get_mesh();
-	if (mesh.is_null()) {
-		err_dialog->set_text(TTR("Mesh is empty!"));
-		err_dialog->popup_centered();
-		return;
-	}
-
+Vector<Ref<Shape3D>> MeshInstance3DEditor::create_shape_from_mesh(Ref<Mesh> p_mesh, int p_option, bool p_verbose) {
+	Vector<Ref<Shape3D>> shapes;
 	switch (p_option) {
-		case MENU_OPTION_CREATE_STATIC_TRIMESH_BODY: {
-			EditorSelection *editor_selection = EditorNode::get_singleton()->get_editor_selection();
-			EditorUndoRedoManager *ur = EditorUndoRedoManager::get_singleton();
-
-			List<Node *> selection = editor_selection->get_selected_node_list();
-
-			if (selection.is_empty()) {
-				Ref<ConcavePolygonShape3D> shape = mesh->create_trimesh_shape();
-				if (shape.is_null()) {
-					err_dialog->set_text(TTR("Couldn't create a Trimesh collision shape."));
-					err_dialog->popup_centered();
-					return;
-				}
-
-				CollisionShape3D *cshape = memnew(CollisionShape3D);
-				cshape->set_shape(shape);
-				StaticBody3D *body = memnew(StaticBody3D);
-				body->add_child(cshape, true);
-
-				Node *owner = get_tree()->get_edited_scene_root();
-
-				ur->create_action(TTR("Create Static Trimesh Body"));
-				ur->add_do_method(node, "add_child", body, true);
-				ur->add_do_method(body, "set_owner", owner);
-				ur->add_do_method(cshape, "set_owner", owner);
-				ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, body);
-				ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, cshape);
-				ur->add_do_reference(body);
-				ur->add_undo_method(node, "remove_child", body);
-				ur->commit_action();
-				return;
-			}
+		case SHAPE_TYPE_TRIMESH: {
+			shapes.push_back(p_mesh->create_trimesh_shape());
 
-			ur->create_action(TTR("Create Static Trimesh Body"));
-
-			for (Node *E : selection) {
-				MeshInstance3D *instance = Object::cast_to<MeshInstance3D>(E);
-				if (!instance) {
-					continue;
-				}
-
-				Ref<Mesh> m = instance->get_mesh();
-				if (m.is_null()) {
-					continue;
-				}
-
-				Ref<ConcavePolygonShape3D> shape = m->create_trimesh_shape();
-				if (shape.is_null()) {
-					continue;
-				}
-
-				CollisionShape3D *cshape = memnew(CollisionShape3D);
-				cshape->set_shape(shape);
-				StaticBody3D *body = memnew(StaticBody3D);
-				body->add_child(cshape, true);
-
-				Node *owner = get_tree()->get_edited_scene_root();
-
-				ur->add_do_method(instance, "add_child", body, true);
-				ur->add_do_method(body, "set_owner", owner);
-				ur->add_do_method(cshape, "set_owner", owner);
-				ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, body);
-				ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, cshape);
-				ur->add_do_reference(body);
-				ur->add_undo_method(instance, "remove_child", body);
+			if (p_verbose && shapes.is_empty()) {
+				err_dialog->set_text(TTR("Couldn't create a Trimesh collision shape."));
+				err_dialog->popup_centered();
 			}
-
-			ur->commit_action();
-
 		} break;
 
-		case MENU_OPTION_CREATE_TRIMESH_COLLISION_SHAPE: {
-			if (node == get_tree()->get_edited_scene_root()) {
-				err_dialog->set_text(TTR("This doesn't work on scene root!"));
-				err_dialog->popup_centered();
-				return;
-			}
+		case SHAPE_TYPE_SINGLE_CONVEX: {
+			shapes.push_back(p_mesh->create_convex_shape(true, false));
 
-			Ref<ConcavePolygonShape3D> shape = mesh->create_trimesh_shape();
-			if (shape.is_null()) {
-				return;
+			if (p_verbose && shapes.is_empty()) {
+				err_dialog->set_text(TTR("Couldn't create a single collision shape."));
+				err_dialog->popup_centered();
 			}
-
-			CollisionShape3D *cshape = memnew(CollisionShape3D);
-			cshape->set_shape(shape);
-			cshape->set_transform(node->get_transform());
-
-			Node *owner = get_tree()->get_edited_scene_root();
-
-			EditorUndoRedoManager *ur = EditorUndoRedoManager::get_singleton();
-
-			ur->create_action(TTR("Create Trimesh Static Shape"));
-
-			ur->add_do_method(node->get_parent(), "add_child", cshape, true);
-			ur->add_do_method(node->get_parent(), "move_child", cshape, node->get_index() + 1);
-			ur->add_do_method(cshape, "set_owner", owner);
-			ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, cshape);
-			ur->add_do_reference(cshape);
-			ur->add_undo_method(node->get_parent(), "remove_child", cshape);
-			ur->commit_action();
 		} break;
 
-		case MENU_OPTION_CREATE_SINGLE_CONVEX_COLLISION_SHAPE:
-		case MENU_OPTION_CREATE_SIMPLIFIED_CONVEX_COLLISION_SHAPE: {
-			if (node == get_tree()->get_edited_scene_root()) {
-				err_dialog->set_text(TTR("Can't create a single convex collision shape for the scene root."));
+		case SHAPE_TYPE_SIMPLIFIED_CONVEX: {
+			shapes.push_back(p_mesh->create_convex_shape(true, true));
+
+			if (p_verbose && shapes.is_empty()) {
+				err_dialog->set_text(TTR("Couldn't create a simplified collision shape."));
 				err_dialog->popup_centered();
-				return;
 			}
+		} break;
 
-			bool simplify = (p_option == MENU_OPTION_CREATE_SIMPLIFIED_CONVEX_COLLISION_SHAPE);
+		case SHAPE_TYPE_MULTIPLE_CONVEX: {
+			Ref<MeshConvexDecompositionSettings> settings;
+			settings.instantiate();
+			settings->set_max_convex_hulls(32);
+			settings->set_max_concavity(0.001);
 
-			Ref<ConvexPolygonShape3D> shape = mesh->create_convex_shape(true, simplify);
+			shapes = p_mesh->convex_decompose(settings);
 
-			if (shape.is_null()) {
-				err_dialog->set_text(TTR("Couldn't create a single convex collision shape."));
+			if (p_verbose && shapes.is_empty()) {
+				err_dialog->set_text(TTR("Couldn't create any collision shapes."));
 				err_dialog->popup_centered();
-				return;
 			}
-			EditorUndoRedoManager *ur = EditorUndoRedoManager::get_singleton();
+		} break;
 
-			if (simplify) {
-				ur->create_action(TTR("Create Simplified Convex Shape"));
-			} else {
-				ur->create_action(TTR("Create Single Convex Shape"));
-			}
+		default:
+			break;
+	}
+	return shapes;
+}
 
-			CollisionShape3D *cshape = memnew(CollisionShape3D);
-			cshape->set_shape(shape);
-			cshape->set_transform(node->get_transform());
+void MeshInstance3DEditor::_create_collision_shape() {
+	int placement_option = shape_placement->get_selected();
+	int shape_type_option = shape_type->get_selected();
 
-			Node *owner = get_tree()->get_edited_scene_root();
+	EditorSelection *editor_selection = EditorNode::get_singleton()->get_editor_selection();
+	EditorUndoRedoManager *ur = EditorUndoRedoManager::get_singleton();
 
-			ur->add_do_method(node->get_parent(), "add_child", cshape, true);
-			ur->add_do_method(node->get_parent(), "move_child", cshape, node->get_index() + 1);
-			ur->add_do_method(cshape, "set_owner", owner);
-			ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, cshape);
-			ur->add_do_reference(cshape);
-			ur->add_undo_method(node->get_parent(), "remove_child", cshape);
+	switch (shape_type_option) {
+		case SHAPE_TYPE_TRIMESH: {
+			ur->create_action(TTR(placement_option == SHAPE_PLACEMENT_SIBLING ? "Create Trimesh Collision Shape Sibling" : "Create Trimesh Static Body"));
+		} break;
+		case SHAPE_TYPE_SINGLE_CONVEX: {
+			ur->create_action(TTR(placement_option == SHAPE_PLACEMENT_SIBLING ? "Create Single Convex Collision Shape Sibling" : "Create Single Convex Static Body"));
+		} break;
+		case SHAPE_TYPE_SIMPLIFIED_CONVEX: {
+			ur->create_action(TTR(placement_option == SHAPE_PLACEMENT_SIBLING ? "Create Simplified Convex Collision Shape Sibling" : "Create Simplified Convex Static Body"));
+		} break;
+		case SHAPE_TYPE_MULTIPLE_CONVEX: {
+			ur->create_action(TTR(placement_option == SHAPE_PLACEMENT_SIBLING ? "Create Multiple Convex Collision Shape Siblings" : "Create Multiple Convex Static Body"));
+		} break;
+		default:
+			break;
+	}
 
-			ur->commit_action();
+	List<Node *> selection = editor_selection->get_selected_node_list();
 
-		} break;
+	bool verbose = false;
+	if (selection.is_empty()) {
+		selection.push_back(node);
+		verbose = true;
+	}
 
-		case MENU_OPTION_CREATE_MULTIPLE_CONVEX_COLLISION_SHAPES: {
-			if (node == get_tree()->get_edited_scene_root()) {
-				err_dialog->set_text(TTR("Can't create multiple convex collision shapes for the scene root."));
+	for (Node *E : selection) {
+		if (placement_option == SHAPE_PLACEMENT_SIBLING && E == get_tree()->get_edited_scene_root()) {
+			if (verbose) {
+				err_dialog->set_text(TTR("Can't create a collision shape as sibling for the scene root."));
 				err_dialog->popup_centered();
-				return;
 			}
+			continue;
+		}
 
-			Ref<MeshConvexDecompositionSettings> settings = Ref<MeshConvexDecompositionSettings>();
-			settings.instantiate();
-			settings->set_max_convex_hulls(32);
-			settings->set_max_concavity(0.001);
+		MeshInstance3D *instance = Object::cast_to<MeshInstance3D>(E);
+		if (!instance) {
+			continue;
+		}
 
-			Vector<Ref<Shape3D>> shapes = mesh->convex_decompose(settings);
+		Ref<Mesh> m = instance->get_mesh();
+		if (m.is_null()) {
+			continue;
+		}
 
-			if (!shapes.size()) {
-				err_dialog->set_text(TTR("Couldn't create any collision shapes."));
-				err_dialog->popup_centered();
-				return;
-			}
-			EditorUndoRedoManager *ur = EditorUndoRedoManager::get_singleton();
+		Vector<Ref<Shape3D>> shapes = create_shape_from_mesh(m, shape_type_option, verbose);
+		if (shapes.is_empty()) {
+			return;
+		}
+
+		Node *owner = get_tree()->get_edited_scene_root();
+		if (placement_option == SHAPE_PLACEMENT_STATIC_BODY_CHILD) {
+			StaticBody3D *body = memnew(StaticBody3D);
 
-			ur->create_action(TTR("Create Multiple Convex Shapes"));
+			ur->add_do_method(instance, "add_child", body, true);
+			ur->add_do_method(body, "set_owner", owner);
+			ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, body);
 
-			for (int i = 0; i < shapes.size(); i++) {
+			for (Ref<Shape3D> shape : shapes) {
 				CollisionShape3D *cshape = memnew(CollisionShape3D);
+				cshape->set_shape(shape);
+				body->add_child(cshape, true);
+				ur->add_do_method(cshape, "set_owner", owner);
+				ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, cshape);
+			}
+			ur->add_do_reference(body);
+			ur->add_undo_method(instance, "remove_child", body);
+		} else {
+			for (Ref<Shape3D> shape : shapes) {
+				CollisionShape3D *cshape = memnew(CollisionShape3D);
+				cshape->set_shape(shape);
 				cshape->set_name("CollisionShape3D");
-
-				cshape->set_shape(shapes[i]);
 				cshape->set_transform(node->get_transform());
-
-				Node *owner = get_tree()->get_edited_scene_root();
-
-				ur->add_do_method(node->get_parent(), "add_child", cshape);
-				ur->add_do_method(node->get_parent(), "move_child", cshape, node->get_index() + 1);
+				ur->add_do_method(E->get_parent(), "add_child", cshape);
+				ur->add_do_method(E->get_parent(), "move_child", cshape, E->get_index() + 1);
 				ur->add_do_method(cshape, "set_owner", owner);
 				ur->add_do_method(Node3DEditor::get_singleton(), SceneStringNames::get_singleton()->_request_gizmo, cshape);
 				ur->add_do_reference(cshape);
 				ur->add_undo_method(node->get_parent(), "remove_child", cshape);
 			}
-			ur->commit_action();
+		}
+	}
+
+	ur->commit_action();
+}
+
+void MeshInstance3DEditor::_menu_option(int p_option) {
+	Ref<Mesh> mesh = node->get_mesh();
+	if (mesh.is_null()) {
+		err_dialog->set_text(TTR("Mesh is empty!"));
+		err_dialog->popup_centered();
+		return;
+	}
 
+	switch (p_option) {
+		case MENU_OPTION_CREATE_COLLISION_SHAPE: {
+			shape_dialog->popup_centered();
 		} break;
 
 		case MENU_OPTION_CREATE_NAVMESH: {
@@ -571,18 +530,7 @@ MeshInstance3DEditor::MeshInstance3DEditor() {
 	options->set_switch_on_hover(true);
 	Node3DEditor::get_singleton()->add_control_to_menu_panel(options);
 
-	options->get_popup()->add_item(TTR("Create Trimesh Static Body"), MENU_OPTION_CREATE_STATIC_TRIMESH_BODY);
-	options->get_popup()->set_item_tooltip(-1, TTR("Creates a StaticBody3D and assigns a polygon-based collision shape to it automatically.\nThis is the most accurate (but slowest) option for collision detection."));
-	options->get_popup()->add_separator();
-	options->get_popup()->add_item(TTR("Create Trimesh Collision Sibling"), MENU_OPTION_CREATE_TRIMESH_COLLISION_SHAPE);
-	options->get_popup()->set_item_tooltip(-1, TTR("Creates a polygon-based collision shape.\nThis is the most accurate (but slowest) option for collision detection."));
-	options->get_popup()->add_item(TTR("Create Single Convex Collision Sibling"), MENU_OPTION_CREATE_SINGLE_CONVEX_COLLISION_SHAPE);
-	options->get_popup()->set_item_tooltip(-1, TTR("Creates a single convex collision shape.\nThis is the fastest (but least accurate) option for collision detection."));
-	options->get_popup()->add_item(TTR("Create Simplified Convex Collision Sibling"), MENU_OPTION_CREATE_SIMPLIFIED_CONVEX_COLLISION_SHAPE);
-	options->get_popup()->set_item_tooltip(-1, TTR("Creates a simplified convex collision shape.\nThis is similar to single collision shape, but can result in a simpler geometry in some cases, at the cost of accuracy."));
-	options->get_popup()->add_item(TTR("Create Multiple Convex Collision Siblings"), MENU_OPTION_CREATE_MULTIPLE_CONVEX_COLLISION_SHAPES);
-	options->get_popup()->set_item_tooltip(-1, TTR("Creates a polygon-based collision shape.\nThis is a performance middle-ground between a single convex collision and a polygon-based collision."));
-	options->get_popup()->add_separator();
+	options->get_popup()->add_item(TTR("Create Collision Shape..."), MENU_OPTION_CREATE_COLLISION_SHAPE);
 	options->get_popup()->add_item(TTR("Create Navigation Mesh"), MENU_OPTION_CREATE_NAVMESH);
 	options->get_popup()->add_separator();
 	options->get_popup()->add_item(TTR("Create Outline Mesh..."), MENU_OPTION_CREATE_OUTLINE_MESH);
@@ -613,6 +561,44 @@ MeshInstance3DEditor::MeshInstance3DEditor() {
 	add_child(outline_dialog);
 	outline_dialog->connect("confirmed", callable_mp(this, &MeshInstance3DEditor::_create_outline_mesh));
 
+	shape_dialog = memnew(ConfirmationDialog);
+	shape_dialog->set_title(TTR("Create Collision Shape"));
+	shape_dialog->set_ok_button_text(TTR("Create"));
+
+	VBoxContainer *shape_dialog_vbc = memnew(VBoxContainer);
+	shape_dialog->add_child(shape_dialog_vbc);
+
+	Label *l = memnew(Label);
+	l->set_text(TTR("Collision Shape placement"));
+	shape_dialog_vbc->add_child(l);
+
+	shape_placement = memnew(OptionButton);
+	shape_placement->set_h_size_flags(SIZE_EXPAND_FILL);
+	shape_placement->add_item(TTR("Sibling"), SHAPE_PLACEMENT_SIBLING);
+	shape_placement->set_item_tooltip(-1, TTR("Creates collision shapes as Sibling."));
+	shape_placement->add_item(TTR("Static Body Child"), SHAPE_PLACEMENT_STATIC_BODY_CHILD);
+	shape_placement->set_item_tooltip(-1, TTR("Creates a StaticBody3D as child and assigns collision shapes to it."));
+	shape_dialog_vbc->add_child(shape_placement);
+
+	l = memnew(Label);
+	l->set_text(TTR("Collision Shape Type"));
+	shape_dialog_vbc->add_child(l);
+
+	shape_type = memnew(OptionButton);
+	shape_type->set_h_size_flags(SIZE_EXPAND_FILL);
+	shape_type->add_item(TTR("Trimesh"), SHAPE_TYPE_TRIMESH);
+	shape_type->set_item_tooltip(-1, TTR("Creates a polygon-based collision shape.\nThis is the most accurate (but slowest) option for collision detection."));
+	shape_type->add_item(TTR("Single Convex"), SHAPE_TYPE_SINGLE_CONVEX);
+	shape_type->set_item_tooltip(-1, TTR("Creates a single convex collision shape.\nThis is the fastest (but least accurate) option for collision detection."));
+	shape_type->add_item(TTR("Simplified Convex"), SHAPE_TYPE_SIMPLIFIED_CONVEX);
+	shape_type->set_item_tooltip(-1, TTR("Creates a simplified convex collision shape.\nThis is similar to single collision shape, but can result in a simpler geometry in some cases, at the cost of accuracy."));
+	shape_type->add_item(TTR("Multiple Convex"), SHAPE_TYPE_MULTIPLE_CONVEX);
+	shape_type->set_item_tooltip(-1, TTR("Creates a polygon-based collision shape.\nThis is a performance middle-ground between a single convex collision and a polygon-based collision."));
+	shape_dialog_vbc->add_child(shape_type);
+
+	add_child(shape_dialog);
+	shape_dialog->connect("confirmed", callable_mp(this, &MeshInstance3DEditor::_create_collision_shape));
+
 	err_dialog = memnew(AcceptDialog);
 	add_child(err_dialog);
 

editor/plugins/mesh_instance_3d_editor_plugin.h

@@ -33,6 +33,7 @@
 
 #include "editor/editor_plugin.h"
 #include "scene/3d/mesh_instance_3d.h"
+#include "scene/gui/option_button.h"
 
 class AcceptDialog;
 class ConfirmationDialog;
@@ -43,11 +44,7 @@ class MeshInstance3DEditor : public Control {
 	GDCLASS(MeshInstance3DEditor, Control);
 
 	enum Menu {
-		MENU_OPTION_CREATE_STATIC_TRIMESH_BODY,
-		MENU_OPTION_CREATE_TRIMESH_COLLISION_SHAPE,
-		MENU_OPTION_CREATE_SINGLE_CONVEX_COLLISION_SHAPE,
-		MENU_OPTION_CREATE_SIMPLIFIED_CONVEX_COLLISION_SHAPE,
-		MENU_OPTION_CREATE_MULTIPLE_CONVEX_COLLISION_SHAPES,
+		MENU_OPTION_CREATE_COLLISION_SHAPE,
 		MENU_OPTION_CREATE_NAVMESH,
 		MENU_OPTION_CREATE_OUTLINE_MESH,
 		MENU_OPTION_CREATE_DEBUG_TANGENTS,
@@ -56,6 +53,18 @@ class MeshInstance3DEditor : public Control {
 		MENU_OPTION_DEBUG_UV2,
 	};
 
+	enum ShapePlacement {
+		SHAPE_PLACEMENT_SIBLING,
+		SHAPE_PLACEMENT_STATIC_BODY_CHILD,
+	};
+
+	enum ShapeType {
+		SHAPE_TYPE_TRIMESH,
+		SHAPE_TYPE_SINGLE_CONVEX,
+		SHAPE_TYPE_SIMPLIFIED_CONVEX,
+		SHAPE_TYPE_MULTIPLE_CONVEX,
+	};
+
 	MeshInstance3D *node = nullptr;
 
 	MenuButton *options = nullptr;
@@ -63,12 +72,18 @@ class MeshInstance3DEditor : public Control {
 	ConfirmationDialog *outline_dialog = nullptr;
 	SpinBox *outline_size = nullptr;
 
+	ConfirmationDialog *shape_dialog = nullptr;
+	OptionButton *shape_type = nullptr;
+	OptionButton *shape_placement = nullptr;
+
 	AcceptDialog *err_dialog = nullptr;
 
 	AcceptDialog *debug_uv_dialog = nullptr;
 	Control *debug_uv = nullptr;
 	Vector<Vector2> uv_lines;
 
+	void _create_collision_shape();
+	Vector<Ref<Shape3D>> create_shape_from_mesh(Ref<Mesh> p_mesh, int p_option, bool p_verbose);
 	void _menu_option(int p_option);
 	void _create_outline_mesh();