Portals - lift roomlist restrictions and fix link bug

Allows users to have the RoomManager as the roomlist.

Fixes a couple of bugs dealing with situations where users attempt to link Portals to Rooms outside the roomlist.

Adds a PortalEditorPlugin allowing you to flip individual portals.

editor/editor_node.cpp

@@ -6820,6 +6820,7 @@ EditorNode::EditorNode() {
 	add_editor_plugin(memnew(BakedLightmapEditorPlugin(this)));
 	add_editor_plugin(memnew(RoomManagerEditorPlugin(this)));
 	add_editor_plugin(memnew(RoomEditorPlugin(this)));
+	add_editor_plugin(memnew(PortalEditorPlugin(this)));
 	add_editor_plugin(memnew(Path2DEditorPlugin(this)));
 	add_editor_plugin(memnew(PathEditorPlugin(this)));
 	add_editor_plugin(memnew(Line2DEditorPlugin(this)));

editor/plugins/room_manager_editor_plugin.cpp

@@ -170,3 +170,53 @@ RoomEditorPlugin::RoomEditorPlugin(EditorNode *p_node) {
 
 RoomEditorPlugin::~RoomEditorPlugin() {
 }
+
+///////////////////////
+
+void PortalEditorPlugin::_flip_portal() {
+	if (_portal) {
+		_portal->flip();
+		_portal->_changed();
+	}
+}
+
+void PortalEditorPlugin::edit(Object *p_object) {
+	Portal *p = Object::cast_to<Portal>(p_object);
+	if (!p) {
+		return;
+	}
+
+	_portal = p;
+}
+
+bool PortalEditorPlugin::handles(Object *p_object) const {
+	return p_object->is_class("Portal");
+}
+
+void PortalEditorPlugin::make_visible(bool p_visible) {
+	if (p_visible) {
+		button_flip->show();
+	} else {
+		button_flip->hide();
+	}
+}
+
+void PortalEditorPlugin::_bind_methods() {
+	ClassDB::bind_method("_flip_portal", &PortalEditorPlugin::_flip_portal);
+}
+
+PortalEditorPlugin::PortalEditorPlugin(EditorNode *p_node) {
+	editor = p_node;
+
+	button_flip = memnew(ToolButton);
+	button_flip->set_icon(editor->get_gui_base()->get_icon("Portal", "EditorIcons"));
+	button_flip->set_text(TTR("Flip Portal"));
+	button_flip->hide();
+	button_flip->connect("pressed", this, "_flip_portal");
+	add_control_to_container(CONTAINER_SPATIAL_EDITOR_MENU, button_flip);
+
+	_portal = nullptr;
+}
+
+PortalEditorPlugin::~PortalEditorPlugin() {
+}

editor/plugins/room_manager_editor_plugin.h

@@ -33,6 +33,7 @@
 
 #include "editor/editor_node.h"
 #include "editor/editor_plugin.h"
+#include "scene/3d/portal.h"
 #include "scene/3d/room.h"
 #include "scene/3d/room_manager.h"
 #include "scene/resources/material.h"
@@ -89,4 +90,29 @@ public:
 	~RoomEditorPlugin();
 };
 
+///////////////////////
+
+class PortalEditorPlugin : public EditorPlugin {
+	GDCLASS(PortalEditorPlugin, EditorPlugin);
+
+	Portal *_portal;
+	ToolButton *button_flip;
+	EditorNode *editor;
+
+	void _flip_portal();
+
+protected:
+	static void _bind_methods();
+
+public:
+	virtual String get_name() const { return "Portal"; }
+	bool has_main_screen() const { return false; }
+	virtual void edit(Object *p_object);
+	virtual bool handles(Object *p_object) const;
+	virtual void make_visible(bool p_visible);
+
+	PortalEditorPlugin(EditorNode *p_node);
+	~PortalEditorPlugin();
+};
+
 #endif

scene/3d/portal.cpp

@@ -44,6 +44,20 @@ bool Portal::_settings_gizmo_show_margins = true;
 Portal::Portal() {
 	clear();
 
+	_settings_active = true;
+	_settings_two_way = true;
+	_internal = false;
+	_linkedroom_ID[0] = -1;
+	_linkedroom_ID[1] = -1;
+	_pts_world.clear();
+	_pts_local.clear();
+	_pts_local_raw.resize(0);
+	_pt_center_world = Vector3();
+	_plane = Plane();
+	_margin = 1.0f;
+	_default_margin = 1.0f;
+	_use_default_margin = true;
+
 	// the visual server portal lifetime is linked to the lifetime of this object
 	_portal_rid = VisualServer::get_singleton()->portal_create();
 
@@ -129,19 +143,9 @@ void Portal::_changed() {
 }
 
 void Portal::clear() {
-	_settings_active = true;
-	_settings_two_way = true;
 	_internal = false;
 	_linkedroom_ID[0] = -1;
 	_linkedroom_ID[1] = -1;
-	_pts_world.clear();
-	_pts_local.clear();
-	_pts_local_raw.resize(0);
-	_pt_center_world = Vector3();
-	_plane = Plane();
-	_margin = 1.0f;
-	_default_margin = 1.0f;
-	_use_default_margin = true;
 }
 
 void Portal::_notification(int p_what) {
@@ -199,10 +203,26 @@ real_t Portal::get_portal_margin() const {
 	return _margin;
 }
 
-void Portal::resolve_links(const RID &p_from_room_rid) {
+void Portal::resolve_links(const LocalVector<Room *, int32_t> &p_rooms, const RID &p_from_room_rid) {
 	Room *linkedroom = nullptr;
 	if (has_node(_settings_path_linkedroom)) {
 		linkedroom = Object::cast_to<Room>(get_node(_settings_path_linkedroom));
+
+		// only allow linking to rooms that are part of the roomlist
+		// (already recognised).
+		// If we don't check this, it will start trying to link to Room nodes that are invalid,
+		// and crash.
+		if (linkedroom && (p_rooms.find(linkedroom) == -1)) {
+			// invalid room
+			WARN_PRINT("Portal attempting to link to Room outside the roomlist : " + linkedroom->get_name());
+			linkedroom = nullptr;
+		}
+
+		// this should not happen, but just in case
+		if (linkedroom && (linkedroom->_room_ID >= p_rooms.size())) {
+			WARN_PRINT("Portal attempting to link to invalid Room : " + linkedroom->get_name());
+			linkedroom = nullptr;
+		}
 	}
 
 	if (linkedroom) {
@@ -249,6 +269,8 @@ bool Portal::try_set_unique_name(const String &p_name) {
 }
 
 void Portal::set_linked_room(const NodePath &link_path) {
+	_settings_path_linkedroom = link_path;
+
 	// change the name of the portal as well, if the link looks legit
 	Room *linkedroom = nullptr;
 	if (has_node(link_path)) {
@@ -269,26 +291,23 @@ void Portal::set_linked_room(const NodePath &link_path) {
 						String string_name = string_name_base + GODOT_PORTAL_WILDCARD + itos(n);
 						if (try_set_unique_name(string_name)) {
 							success = true;
-							_changed();
 							break;
 						}
 					}
 
 					if (!success) {
-						WARN_PRINT("Could not set portal name, set name manually instead.");
+						WARN_PRINT("Could not set portal name, suggest setting name manually instead.");
 					}
-				} else {
-					_changed();
 				}
 			} else {
-				WARN_PRINT("Linked room cannot be portal's parent room, ignoring.");
+				WARN_PRINT("Linked room cannot be the parent room of a portal.");
 			}
 		} else {
-			WARN_PRINT("Linked room path is not a room, ignoring.");
+			WARN_PRINT("Linked room path is not a room.");
 		}
-	} else {
-		WARN_PRINT("Linked room path not found.");
 	}
+
+	_changed();
 }
 
 NodePath Portal::get_linked_room() const {

scene/3d/portal.h

@@ -31,11 +31,13 @@
 #ifndef PORTAL_H
 #define PORTAL_H
 
+#include "core/local_vector.h"
 #include "core/rid.h"
 #include "spatial.h"
 
 class RoomManager;
 class MeshInstance;
+class Room;
 
 class Portal : public Spatial {
 	GDCLASS(Portal, Spatial);
@@ -44,6 +46,7 @@ class Portal : public Spatial {
 
 	friend class RoomManager;
 	friend class PortalGizmoPlugin;
+	friend class PortalEditorPlugin;
 
 public:
 	// ui interface .. will have no effect after room conversion
@@ -61,6 +64,7 @@ public:
 	}
 	bool is_two_way() const { return _settings_two_way; }
 
+	// call during each conversion
 	void clear();
 
 	// whether to use the room manager default
@@ -104,7 +108,7 @@ private:
 	Vector3 _vec2to3(const Vector2 &p_pt) const { return Vector3(p_pt.x, p_pt.y, 0.0); }
 	void _sort_verts_clockwise(bool portal_plane_convention, Vector<Vector3> &r_verts);
 	Plane _plane_from_points_newell(const Vector<Vector3> &p_pts);
-	void resolve_links(const RID &p_from_room_rid);
+	void resolve_links(const LocalVector<Room *, int32_t> &p_rooms, const RID &p_from_room_rid);
 	void _changed();
 
 	// nodepath to the room this outgoing portal leads to

scene/3d/room.h

@@ -77,7 +77,9 @@ public:
 	String get_configuration_warning() const;
 
 private:
+	// call during each conversion
 	void clear();
+
 	void _changed(bool p_regenerate_bounds = false);
 	template <class T>
 	static bool detect_nodes_of_type(const Node *p_node, bool p_ignore_first_node = true);

scene/3d/room_manager.cpp

@@ -36,6 +36,7 @@
 #include "core/os/os.h"
 #include "editor/editor_node.h"
 #include "mesh_instance.h"
+#include "multimesh_instance.h"
 #include "portal.h"
 #include "room_group.h"
 #include "scene/3d/camera.h"
@@ -86,11 +87,12 @@ String RoomManager::get_configuration_warning() const {
 		warning += TTR("The RoomList has not been assigned.");
 	} else {
 		Spatial *roomlist = _resolve_path<Spatial>(_settings_path_roomlist);
-		if (!roomlist || (roomlist->get_class_name() != StringName("Spatial"))) {
+		if (!roomlist) {
+			// possibly also check (roomlist->get_class_name() != StringName("Spatial"))
 			if (!warning.empty()) {
 				warning += "\n\n";
 			}
-			warning += TTR("The RoomList should be a Spatial.");
+			warning += TTR("The RoomList node should be a Spatial (or derived from Spatial).");
 		}
 	}
 
@@ -101,38 +103,11 @@ String RoomManager::get_configuration_warning() const {
 		warning += TTR("Portal Depth Limit is set to Zero.\nOnly the Room that the Camera is in will render.");
 	}
 
-	auto lambda = [](const Node *p_node) {
+	if (Room::detect_nodes_of_type<RoomManager>(this)) {
-		return static_cast<bool>((Object::cast_to<Room>(p_node) || Object::cast_to<RoomGroup>(p_node) || Object::cast_to<Portal>(p_node) || Object::cast_to<RoomManager>(p_node)));
+		if (!warning.empty()) {
-	};
+			warning += "\n\n";
-
-	if (Room::detect_nodes_using_lambda(this, lambda)) {
-		if (Room::detect_nodes_of_type<Room>(this)) {
-			if (!warning.empty()) {
-				warning += "\n\n";
-			}
-			warning += TTR("Rooms should not be children of the RoomManager.");
-		}
-
-		if (Room::detect_nodes_of_type<RoomGroup>(this)) {
-			if (!warning.empty()) {
-				warning += "\n\n";
-			}
-			warning += TTR("RoomGroups should not be children of the RoomManager.");
-		}
-
-		if (Room::detect_nodes_of_type<Portal>(this)) {
-			if (!warning.empty()) {
-				warning += "\n\n";
-			}
-			warning += TTR("Portals should not be children of the RoomManager.");
-		}
-
-		if (Room::detect_nodes_of_type<RoomManager>(this)) {
-			if (!warning.empty()) {
-				warning += "\n\n";
-			}
-			warning += TTR("There should only be one RoomManager in the SceneTree.");
 		}
+		warning += TTR("There should only be one RoomManager in the SceneTree.");
 	}
 
 	return warning;
@@ -851,7 +826,7 @@ void RoomManager::_second_pass_portals(Spatial *p_roomlist, LocalVector<Portal *
 		int room_from_id = portal->_linkedroom_ID[0];
 		if (room_from_id != -1) {
 			Room *room_from = _rooms[room_from_id];
-			portal->resolve_links(room_from->_room_rid);
+			portal->resolve_links(_rooms, room_from->_room_rid);
 
 			// add the portal id to the room from and the room to.
 			// These are used so we can later add the portal geometry to the room bounds.
@@ -969,11 +944,7 @@ void RoomManager::_autolink_portals(Spatial *p_roomlist, LocalVector<Portal *> &
 // to prevent users creating mistakes for themselves, we limit what can be put into the room list branch.
 // returns invalid node, or NULL
 bool RoomManager::_check_roomlist_validity(Node *p_node) {
-	if (Room::detect_nodes_of_type<RoomManager>(p_node, false)) {
+	// restrictions lifted here, but we can add more if required
-		show_warning("RoomList should not be the RoomManager,\nor contain a RoomManager as child or grandchild.");
-		return false;
-	}
-
 	return true;
 }
 
@@ -1586,6 +1557,9 @@ void RoomManager::_convert_portal(Room *p_room, Spatial *p_node, LocalVector<Por
 		}
 	}
 
+	// make sure to start with fresh internal data each time (for linked rooms etc)
+	portal->clear();
+
 	// mark so as only to convert once
 	portal->_conversion_tick = _conversion_tick;
 
@@ -1600,12 +1574,12 @@ void RoomManager::_convert_portal(Room *p_room, Spatial *p_node, LocalVector<Por
 }
 
 bool RoomManager::_bound_findpoints_geom_instance(GeometryInstance *p_gi, Vector<Vector3> &r_room_pts, AABB &r_aabb) {
-#ifdef MODULE_CSG_ENABLED
 	// max opposite extents .. note AABB storing size is rubbish in this aspect
 	// it can fail once mesh min is larger than FLT_MAX / 2.
 	r_aabb.position = Vector3(FLT_MAX / 2, FLT_MAX / 2, FLT_MAX / 2);
 	r_aabb.size = Vector3(-FLT_MAX, -FLT_MAX, -FLT_MAX);
 
+#ifdef MODULE_CSG_ENABLED
 	CSGShape *shape = Object::cast_to<CSGShape>(p_gi);
 	if (shape) {
 		Array arr = shape->get_meshes();
@@ -1645,12 +1619,68 @@ bool RoomManager::_bound_findpoints_geom_instance(GeometryInstance *p_gi, Vector
 
 		} // for through the surfaces
 
+		return true;
 	} // if csg shape
-
-	return true;
-#else
-	return false;
 #endif
+
+	// multimesh
+	MultiMeshInstance *mmi = Object::cast_to<MultiMeshInstance>(p_gi);
+	if (mmi) {
+		Ref<MultiMesh> rmm = mmi->get_multimesh();
+		if (!rmm.is_valid()) {
+			return false;
+		}
+
+		// first get the mesh verts in local space
+		LocalVector<Vector3, int32_t> local_verts;
+		Ref<Mesh> rmesh = rmm->get_mesh();
+
+		if (rmesh->get_surface_count() == 0) {
+			String string;
+			string = "MultiMeshInstance '" + mmi->get_name() + "' has no surfaces, ignoring";
+			WARN_PRINT(string);
+			return false;
+		}
+
+		for (int surf = 0; surf < rmesh->get_surface_count(); surf++) {
+			Array arrays = rmesh->surface_get_arrays(surf);
+
+			if (!arrays.size()) {
+				WARN_PRINT_ONCE("MultiMesh mesh surface with no mesh, ignoring");
+				continue;
+			}
+
+			const PoolVector<Vector3> &vertices = arrays[VS::ARRAY_VERTEX];
+
+			int count = local_verts.size();
+			local_verts.resize(local_verts.size() + vertices.size());
+
+			for (int n = 0; n < vertices.size(); n++) {
+				local_verts[count++] = vertices[n];
+			}
+		}
+
+		if (!local_verts.size()) {
+			return false;
+		}
+
+		// now we have the local space verts, add a bunch for each instance, and find the AABB
+		for (int i = 0; i < rmm->get_instance_count(); i++) {
+			Transform trans = rmm->get_instance_transform(i);
+			trans = mmi->get_global_transform() * trans;
+
+			for (int n = 0; n < local_verts.size(); n++) {
+				Vector3 ptWorld = trans.xform(local_verts[n]);
+				r_room_pts.push_back(ptWorld);
+
+				// keep the bound up to date
+				r_aabb.expand_to(ptWorld);
+			}
+		}
+		return true;
+	}
+
+	return false;
 }
 
 bool RoomManager::_bound_findpoints_mesh_instance(MeshInstance *p_mi, Vector<Vector3> &r_room_pts, AABB &r_aabb) {
@@ -1681,7 +1711,7 @@ bool RoomManager::_bound_findpoints_mesh_instance(MeshInstance *p_mi, Vector<Vec
 
 		// possible to have a meshinstance with no geometry .. don't want to crash
 		if (!arrays.size()) {
-			WARN_PRINT_ONCE("PConverter::bound_findpoints MeshInstance surface with no mesh, ignoring");
+			WARN_PRINT_ONCE("MeshInstance surface with no mesh, ignoring");
 			continue;
 		}