From 8c4c6a93b0fbd6c903f77e3a64294b0ca07a49ff Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Fri, 30 Jul 2021 11:50:30 +0100 Subject: [PATCH 1/2] Portals - Remove node naming restrictions The use of special prefixes is only actually required during the import phase - the first conversion of rooms, roomgroups, and portals from Spatials and MeshInstances (based on the workflow of importing from blender). Once converted to the native Godot nodes there is no longer a need for the naming requirements. This PR removes the requirements except for the import. Manual portal linking after the initial conversion is now done exclusively using the `linked_room` nodepath property of the Portal. --- scene/3d/portal.cpp | 25 ++-------- scene/3d/portal.h | 7 +++ scene/3d/room_manager.cpp | 102 +++++++++++++++++++++++++++----------- scene/3d/room_manager.h | 2 +- 4 files changed, 85 insertions(+), 51 deletions(-) diff --git a/scene/3d/portal.cpp b/scene/3d/portal.cpp index deb6aeefa6b..353ff316994 100644 --- a/scene/3d/portal.cpp +++ b/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(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."); } diff --git a/scene/3d/portal.h b/scene/3d/portal.h index bdab51262af..b2e0e311e31 100644 --- a/scene/3d/portal.h +++ b/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; diff --git a/scene/3d/room_manager.cpp b/scene/3d/room_manager.cpp index 260b891e3d1..ea5e75352ba 100644 --- a/scene/3d/room_manager.cpp +++ b/scene/3d/room_manager.cpp @@ -776,16 +776,34 @@ void RoomManager::_third_pass_rooms(const LocalVector &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_after(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_after(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 &p_portals) { } void RoomManager::_second_pass_portals(Spatial *p_roomlist, LocalVector &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(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_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(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 &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 &r_portals, LocalVector &r_roomgroups, int p_roomgroup) { // is this a room? - if (_name_starts_with(p_node, "Room") || _node_is_type(p_node)) { + if (_node_is_type(p_node) || _name_starts_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(p_node)) { + if (_node_is_type(p_node) || _name_starts_with(p_node, "RoomGroup")) { p_roomgroup = _convert_roomgroup(p_node, r_roomgroups); } @@ -1570,12 +1591,16 @@ bool RoomManager::_add_plane_if_unique(const Room *p_room, LocalVector &portals) { Portal *portal = Object::cast_to(p_node); + bool importing = false; + // if not a gportal already, convert the node type if (!portal) { + importing = true; portal = _change_node_type(p_node, "G", false); portal->create_from_mesh_instance(Object::cast_to(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, LocalVectorclear(); + // 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; @@ -1976,12 +2005,29 @@ bool RoomManager::_name_starts_with(const Node *p_node, String p_search_string, return false; } -String RoomManager::_find_name_after(Node *p_node, String p_string_start) { - p_string_start += GODOT_PORTAL_DELINEATOR; +String RoomManager::_find_name_after(Node *p_node, String p_prefix, bool p_allow_no_prefix) { + ERR_FAIL_NULL_V(p_node, String()); + + p_prefix += GODOT_PORTAL_DELINEATOR; + p_prefix = p_prefix.to_lower(); + int prefix_length = p_prefix.length(); + + String name = p_node->get_name(); + String name_start = name.substr(0, prefix_length).to_lower(); String string_result; - String name = p_node->get_name(); - string_result = name.substr(p_string_start.length()); + + // is the prefix correct? + if (p_prefix == name_start) { + string_result = name.substr(prefix_length); + } else { + if (p_allow_no_prefix) { + // there is no prefix, or the prefix is incorrect... + // we will pass the whole name + string_result = name; + } + // else we will return a null 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... diff --git a/scene/3d/room_manager.h b/scene/3d/room_manager.h index 63a74c90e65..cd2baa89f44 100644 --- a/scene/3d/room_manager.h +++ b/scene/3d/room_manager.h @@ -215,7 +215,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_after(Node *p_node, String p_prefix, bool p_allow_no_prefix = 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; } From 2eae35693ee22db736f8dfd36fd95f9712c80474 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Mon, 2 Aug 2021 10:57:49 +0100 Subject: [PATCH 2/2] Portals - change import naming convention to postfix In response to user demand, the naming convention for importing levels from blender etc is changed from prefixes `Room_` and `Portal_` to postfixes `-room`, `-roomgroup`, `-portal`. --- scene/3d/room_manager.cpp | 109 +++++++++++++------------------------- scene/3d/room_manager.h | 8 ++- 2 files changed, 41 insertions(+), 76 deletions(-) diff --git a/scene/3d/room_manager.cpp b/scene/3d/room_manager.cpp index ea5e75352ba..cbcb46c7ba3 100644 --- a/scene/3d/room_manager.cpp +++ b/scene/3d/room_manager.cpp @@ -639,10 +639,10 @@ void RoomManager::_second_pass_room(Room *p_room, const LocalVector Spatial *child = Object::cast_to(p_room->get_child(n)); if (child) { - if (_name_starts_with(child, "GPortal", true) || _node_is_type(child)) { + if (_node_is_type(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, @@ -778,7 +778,7 @@ void RoomManager::_third_pass_rooms(const LocalVector &p_portals) { // 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_after(room, "Room", true); + 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 @@ -793,7 +793,7 @@ void RoomManager::_third_pass_rooms(const LocalVector &p_portals) { if (linked_room_id < _rooms.size()) { Room *linked_room = _rooms[linked_room_id]; - String portal_link_room_name = _find_name_after(linked_room, "Room", true); + 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 @@ -834,8 +834,8 @@ void RoomManager::_second_pass_portals(Spatial *p_roomlist, LocalVector_importing_portal) { - String string_link_room_shortname = _find_name_after(portal, "Portal"); - String string_link_room = "Room" + GODOT_PORTAL_DELINEATOR + string_link_room_shortname; + 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(p_roomlist->find_node(string_link_room, true, false)); @@ -979,12 +979,12 @@ bool RoomManager::_check_roomlist_validity(Node *p_node) { void RoomManager::_convert_rooms_recursive(Spatial *p_node, LocalVector &r_portals, LocalVector &r_roomgroups, int p_roomgroup) { // is this a room? - if (_node_is_type(p_node) || _name_starts_with(p_node, "Room")) { + if (_node_is_type(p_node) || _name_ends_with(p_node, "-room")) { _convert_room(p_node, r_portals, r_roomgroups, p_roomgroup); } // is this a roomgroup? - if (_node_is_type(p_node) || _name_starts_with(p_node, "RoomGroup")) { + if (_node_is_type(p_node) || _name_ends_with(p_node, "-roomgroup")) { p_roomgroup = _convert_roomgroup(p_node, r_roomgroups); } @@ -1073,7 +1073,7 @@ void RoomManager::_convert_room(Spatial *p_node, LocalVector &r_portal void RoomManager::_find_portals_recursive(Spatial *p_node, Room *p_room, LocalVector &r_portals) { MeshInstance *mi = Object::cast_to(p_node); - if ((mi && _name_starts_with(mi, "Portal", true)) || _node_is_type(p_node)) { + if (_node_is_type(p_node) || (mi && _name_ends_with(mi, "-portal"))) { _convert_portal(p_room, p_node, r_portals); } @@ -1956,91 +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(); + int pf_l = p_postfix.length(); + int l = name.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; - } - } - - WARN_PRINT("Possible misnamed node : " + name); - _warning_misnamed_nodes_detected = true; + if (pf_l > l) { + return false; } -} -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)) { + // allow capitalization errors + if (name.substr(l - pf_l, pf_l).to_lower() == p_postfix) { return true; } - String search_string = p_search_string + GODOT_PORTAL_DELINEATOR; - int sl = search_string.length(); - - if (name.substr(0, sl) == search_string) { - return true; - } - - _check_for_misnamed_node(p_node, p_search_string); return false; } -String RoomManager::_find_name_after(Node *p_node, String p_prefix, bool p_allow_no_prefix) { +String RoomManager::_find_name_before(Node *p_node, String p_postfix, bool p_allow_no_postfix) { ERR_FAIL_NULL_V(p_node, String()); - - p_prefix += GODOT_PORTAL_DELINEATOR; - p_prefix = p_prefix.to_lower(); - int prefix_length = p_prefix.length(); - String name = p_node->get_name(); - String name_start = name.substr(0, prefix_length).to_lower(); - String string_result; + int pf_l = p_postfix.length(); + int l = name.length(); - // is the prefix correct? - if (p_prefix == name_start) { - string_result = name.substr(prefix_length); - } else { - if (p_allow_no_prefix) { - // there is no prefix, or the prefix is incorrect... - // we will pass the whole name - string_result = name; + 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(); + } } - // else we will return a null 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) { @@ -2202,7 +2169,7 @@ void RoomManager::_list_mergeable_mesh_instances(Spatial *p_node, LocalVectorget_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(mi) && !_name_starts_with(mi, "Bound", true) && !mi->is_queued_for_deletion()) { + if (!_node_is_type(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); diff --git a/scene/3d/room_manager.h b/scene/3d/room_manager.h index cd2baa89f44..4a38d03b3b5 100644 --- a/scene/3d/room_manager.h +++ b/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 NODE_TYPE *_resolve_path(NodePath p_path) const; template @@ -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_prefix, bool p_allow_no_prefix = false); + 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; }