Merge pull request #51062 from lawnjelly/portals_no_names

scene/3d/portal.cpp

@@ -145,6 +145,7 @@ void Portal::clear() {
 	_internal = false;
 	_linkedroom_ID[0] = -1;
 	_linkedroom_ID[1] = -1;
+	_importing_portal = false;
 }
 
 void Portal::_notification(int p_what) {
@@ -279,34 +280,14 @@ 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
+	// see if the link looks legit
 	Room *linkedroom = nullptr;
 	if (has_node(link_path)) {
 		linkedroom = Object::cast_to<Room>(get_node(link_path));
 
 		if (linkedroom) {
 			if (linkedroom != get_parent()) {
-				_settings_path_linkedroom = link_path;
-
-				// change the portal name
-				String string_link_room = RoomManager::_find_name_after(linkedroom, "Room");
-
-				// we need a unique name for the portal
-				String string_name_base = "Portal" + GODOT_PORTAL_DELINEATOR + string_link_room;
-				if (!try_set_unique_name(string_name_base)) {
-					bool success = false;
-					for (int n = 0; n < 128; n++) {
-						String string_name = string_name_base + GODOT_PORTAL_WILDCARD + itos(n);
-						if (try_set_unique_name(string_name)) {
-							success = true;
-							break;
-						}
-					}
-
-					if (!success) {
-						WARN_PRINT("Could not set portal name, suggest setting name manually instead.");
-					}
-				}
+				// was ok
 			} else {
 				WARN_PRINT("Linked room cannot be the parent room of a portal.");
 			}

scene/3d/portal.h

@@ -154,6 +154,13 @@ private:
 	real_t _margin;
 	bool _use_default_margin;
 
+	// during conversion, we need to know
+	// whether this portal is being imported from a mesh
+	// and is using an explicitly named link room with prefix.
+	// If this is not the case, and it is already a Godot Portal node,
+	// we will either use the assigned nodepath, or autolink.
+	bool _importing_portal = false;
+
 	// for editing
 #ifdef TOOLS_ENABLED
 	ObjectID _room_manager_godot_ID;

scene/3d/room_manager.cpp

@@ -639,10 +639,10 @@ void RoomManager::_second_pass_room(Room *p_room, const LocalVector<RoomGroup *>
 		Spatial *child = Object::cast_to<Spatial>(p_room->get_child(n));
 
 		if (child) {
-			if (_name_starts_with(child, "GPortal", true) || _node_is_type<Portal>(child)) {
+			if (_node_is_type<Portal>(child) || child->is_queued_for_deletion()) {
 				// the adding of portal points is done after this stage, because
 				// we need to take into account incoming as well as outgoing portals
-			} else if (_name_starts_with(child, "Bound", true)) {
+			} else if (_name_ends_with(child, "-bound")) {
 				manual_bound_found = _convert_manual_bound(p_room, child, p_portals);
 			} else {
 				// don't add the instances to the portal renderer on the first pass of _find_statics,
@@ -776,16 +776,34 @@ void RoomManager::_third_pass_rooms(const LocalVector<Portal *> &p_portals) {
 	for (int n = 0; n < _rooms.size(); n++) {
 		Room *room = _rooms[n];
 
-		String room_short_name = _find_name_after(room, "ROOM");
-		convert_log("ROOM\t" + room_short_name);
+		// no need to do all these string operations if we are not debugging and don't need logs
+		if (_show_debug) {
+			String room_short_name = _find_name_before(room, "-room", true);
+			convert_log("ROOM\t" + room_short_name);
 
-		// log output the portals associated with this room
-		for (int p = 0; p < room->_portals.size(); p++) {
-			const Portal &portal = *p_portals[room->_portals[p]];
+			// log output the portals associated with this room
+			for (int p = 0; p < room->_portals.size(); p++) {
+				const Portal &portal = *p_portals[room->_portals[p]];
 
-			String in_or_out = (portal._linkedroom_ID[0] == room->_room_ID) ? "POUT" : "PIN ";
-			convert_log("\t\t" + in_or_out + "\t" + portal.get_name());
-		}
+				bool portal_links_out = portal._linkedroom_ID[0] == room->_room_ID;
+
+				int linked_room_id = (portal_links_out) ? portal._linkedroom_ID[1] : portal._linkedroom_ID[0];
+
+				// this shouldn't be out of range, but just in case
+				if (linked_room_id < _rooms.size()) {
+					Room *linked_room = _rooms[linked_room_id];
+
+					String portal_link_room_name = _find_name_before(linked_room, "-room", true);
+					String in_or_out = (portal_links_out) ? "POUT" : "PIN ";
+
+					// display the name of the room linked to
+					convert_log("\t\t" + in_or_out + "\t" + portal_link_room_name);
+				} else {
+					WARN_PRINT_ONCE("linked_room_id is out of range");
+				}
+			}
+
+		} // if _show_debug
 
 		// do a second pass finding the statics, where they are
 		// finally added to the rooms in the portal_renderer.
@@ -808,21 +826,26 @@ void RoomManager::_third_pass_rooms(const LocalVector<Portal *> &p_portals) {
 }
 
 void RoomManager::_second_pass_portals(Spatial *p_roomlist, LocalVector<Portal *> &r_portals) {
-	convert_log("_second_pass_portals");
-
 	for (unsigned int n = 0; n < r_portals.size(); n++) {
 		Portal *portal = r_portals[n];
-		String string_link_room_shortname = _find_name_after(portal, "Portal");
-		String string_link_room = "Room" + GODOT_PORTAL_DELINEATOR + string_link_room_shortname;
-
-		if (string_link_room_shortname != "") {
-			Room *linked_room = Object::cast_to<Room>(p_roomlist->find_node(string_link_room, true, false));
-			if (linked_room) {
-				NodePath path = portal->get_path_to(linked_room);
-				portal->set_linked_room_internal(path);
-			} else {
-				WARN_PRINT("Portal link room : " + string_link_room + " not found.");
-				_warning_portal_link_room_not_found = true;
+
+		// we have a choice here.
+		// If we are importing, we will try linking using the naming convention method.
+		// We do this by setting the assigned nodepath if we find the link room, then
+		// the resolving links is done in the usual manner from the nodepath.
+		if (portal->_importing_portal) {
+			String string_link_room_shortname = _find_name_before(portal, "-portal");
+			String string_link_room = string_link_room_shortname + "-room";
+
+			if (string_link_room_shortname != "") {
+				Room *linked_room = Object::cast_to<Room>(p_roomlist->find_node(string_link_room, true, false));
+				if (linked_room) {
+					NodePath path = portal->get_path_to(linked_room);
+					portal->set_linked_room_internal(path);
+				} else {
+					WARN_PRINT("Portal link room : " + string_link_room + " not found.");
+					_warning_portal_link_room_not_found = true;
+				}
 			}
 		}
 
@@ -849,8 +872,6 @@ void RoomManager::_second_pass_portals(Spatial *p_roomlist, LocalVector<Portal *
 }
 
 void RoomManager::_autolink_portals(Spatial *p_roomlist, LocalVector<Portal *> &r_portals) {
-	convert_log("_autolink_portals");
-
 	for (unsigned int n = 0; n < r_portals.size(); n++) {
 		Portal *portal = r_portals[n];
 
@@ -958,12 +979,12 @@ bool RoomManager::_check_roomlist_validity(Node *p_node) {
 
 void RoomManager::_convert_rooms_recursive(Spatial *p_node, LocalVector<Portal *> &r_portals, LocalVector<RoomGroup *> &r_roomgroups, int p_roomgroup) {
 	// is this a room?
-	if (_name_starts_with(p_node, "Room") || _node_is_type<Room>(p_node)) {
+	if (_node_is_type<Room>(p_node) || _name_ends_with(p_node, "-room")) {
 		_convert_room(p_node, r_portals, r_roomgroups, p_roomgroup);
 	}
 
 	// is this a roomgroup?
-	if (_name_starts_with(p_node, "RoomGroup") || _node_is_type<RoomGroup>(p_node)) {
+	if (_node_is_type<RoomGroup>(p_node) || _name_ends_with(p_node, "-roomgroup")) {
 		p_roomgroup = _convert_roomgroup(p_node, r_roomgroups);
 	}
 
@@ -1052,7 +1073,7 @@ void RoomManager::_convert_room(Spatial *p_node, LocalVector<Portal *> &r_portal
 
 void RoomManager::_find_portals_recursive(Spatial *p_node, Room *p_room, LocalVector<Portal *> &r_portals) {
 	MeshInstance *mi = Object::cast_to<MeshInstance>(p_node);
-	if ((mi && _name_starts_with(mi, "Portal", true)) || _node_is_type<Portal>(p_node)) {
+	if (_node_is_type<Portal>(p_node) || (mi && _name_ends_with(mi, "-portal"))) {
 		_convert_portal(p_room, p_node, r_portals);
 	}
 
@@ -1570,12 +1591,16 @@ bool RoomManager::_add_plane_if_unique(const Room *p_room, LocalVector<Plane, in
 void RoomManager::_convert_portal(Room *p_room, Spatial *p_node, LocalVector<Portal *> &portals) {
 	Portal *portal = Object::cast_to<Portal>(p_node);
 
+	bool importing = false;
+
 	// if not a gportal already, convert the node type
 	if (!portal) {
+		importing = true;
 		portal = _change_node_type<Portal>(p_node, "G", false);
 		portal->create_from_mesh_instance(Object::cast_to<MeshInstance>(p_node));
 
 		p_node->queue_delete();
+
 	} else {
 		// only allow converting once
 		if (portal->_conversion_tick == _conversion_tick) {
@@ -1586,6 +1611,10 @@ 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 the portal if we are importing, because we will need to use the naming
+	// prefix system to look for linked rooms in that case
+	portal->_importing_portal = importing;
+
 	// mark so as only to convert once
 	portal->_conversion_tick = _conversion_tick;
 
@@ -1927,74 +1956,58 @@ void RoomManager::_set_owner_recursive(Node *p_node, Node *p_owner) {
 	}
 }
 
-void RoomManager::_check_for_misnamed_node(const Node *p_node, String p_start_string) {
-	// don't check the roomlist name, as it often has a conflict with Room
-	if (p_node == _roomlist) {
-		return;
-	}
-
+bool RoomManager::_name_ends_with(const Node *p_node, String p_postfix) const {
+	ERR_FAIL_NULL_V(p_node, false);
 	String name = p_node->get_name();
 
-	int ss_length = p_start_string.length();
-
-	if (name.substr(0, ss_length).to_lower() == p_start_string.to_lower()) {
-		if (p_start_string == "Room") {
-			// do allow RoomGroup and RoomManager
-			if (name.substr(0, 9) == "RoomGroup") {
-				return;
-			}
-
-			if (name.substr(0, 11) == "RoomManager") {
-				return;
-			}
-		} else {
-			if (p_start_string == "RoomGroup") {
-				return;
-			}
-		}
+	int pf_l = p_postfix.length();
+	int l = name.length();
 
-		WARN_PRINT("Possible misnamed node : " + name);
-		_warning_misnamed_nodes_detected = true;
-	}
-}
-
-bool RoomManager::_name_starts_with(const Node *p_node, String p_search_string, bool p_allow_no_delineator) {
-	String name = p_node->get_name();
-
-	if (p_allow_no_delineator && (name == p_search_string)) {
-		return true;
+	if (pf_l > l) {
+		return false;
 	}
 
-	String search_string = p_search_string + GODOT_PORTAL_DELINEATOR;
-	int sl = search_string.length();
-
-	if (name.substr(0, sl) == search_string) {
+	// allow capitalization errors
+	if (name.substr(l - pf_l, pf_l).to_lower() == p_postfix) {
 		return true;
 	}
 
-	_check_for_misnamed_node(p_node, p_search_string);
 	return false;
 }
 
-String RoomManager::_find_name_after(Node *p_node, String p_string_start) {
-	p_string_start += GODOT_PORTAL_DELINEATOR;
-
-	String string_result;
+String RoomManager::_find_name_before(Node *p_node, String p_postfix, bool p_allow_no_postfix) {
+	ERR_FAIL_NULL_V(p_node, String());
 	String name = p_node->get_name();
-	string_result = name.substr(p_string_start.length());
+
+	int pf_l = p_postfix.length();
+	int l = name.length();
+
+	if (pf_l > l) {
+		if (!p_allow_no_postfix) {
+			return String();
+		}
+	} else {
+		if (name.substr(l - pf_l, pf_l) == p_postfix) {
+			name = name.substr(0, l - pf_l);
+		} else {
+			if (!p_allow_no_postfix) {
+				return String();
+			}
+		}
+	}
 
 	// because godot doesn't support multiple nodes with the same name, we will strip e.g. a number
-	// after an @ on the end of the name...
-	// e.g. portal_kitchen@2
-	for (int c = 0; c < string_result.length(); c++) {
-		if (string_result[c] == '*') {
+	// after an * on the end of the name...
+	// e.g. kitchen*2-portal
+	for (int c = 0; c < name.length(); c++) {
+		if (name[c] == GODOT_PORTAL_WILDCARD) {
 			// remove everything after and including this character
-			string_result = string_result.substr(0, c);
+			name = name.substr(0, c);
 			break;
 		}
 	}
 
-	return string_result;
+	return name;
 }
 
 void RoomManager::_merge_meshes_in_room(Room *p_room) {
@@ -2156,7 +2169,7 @@ void RoomManager::_list_mergeable_mesh_instances(Spatial *p_node, LocalVector<Me
 		if (vi && vi->get_portal_mode() == CullInstance::PORTAL_MODE_STATIC) {
 			// disallow for portals or bounds
 			// mesh instance portals should be queued for deletion by this point, we don't want to merge portals!
-			if (!_node_is_type<Portal>(mi) && !_name_starts_with(mi, "Bound", true) && !mi->is_queued_for_deletion()) {
+			if (!_node_is_type<Portal>(mi) && !_name_ends_with(mi, "-bound") && !mi->is_queued_for_deletion()) {
 				// only merge if visible
 				if (mi->is_inside_tree() && mi->is_visible()) {
 					r_list.push_back(mi);

scene/3d/room_manager.h

@@ -41,8 +41,7 @@ class MeshInstance;
 class GeometryInstance;
 class VisualInstance;
 
-#define GODOT_PORTAL_DELINEATOR String("_")
-#define GODOT_PORTAL_WILDCARD String("*")
+#define GODOT_PORTAL_WILDCARD ('*')
 
 class RoomManager : public Spatial {
 	GDCLASS(RoomManager, Spatial);
@@ -194,8 +193,7 @@ private:
 	bool _remove_redundant_dangling_nodes(Spatial *p_node);
 
 	// helper funcs
-	bool _name_starts_with(const Node *p_node, String p_search_string, bool p_allow_no_delineator = false);
-	void _check_for_misnamed_node(const Node *p_node, String p_start_string);
+	bool _name_ends_with(const Node *p_node, String p_postfix) const;
 	template <class NODE_TYPE>
 	NODE_TYPE *_resolve_path(NodePath p_path) const;
 	template <class NODE_TYPE>
@@ -215,7 +213,7 @@ private:
 	void debug_print_line(String p_string, int p_priority = 0);
 
 public:
-	static String _find_name_after(Node *p_node, String p_string_start);
+	static String _find_name_before(Node *p_node, String p_postfix, bool p_allow_no_postfix = false);
 	static void show_warning(const String &p_string, const String &p_extra_string = "", bool p_alert = true);
 	static real_t _get_default_portal_margin() { return _default_portal_margin; }