Clear old item groups when they are overridden. (#8753)

This fixes overridden items keeping their old groups in the group to
items mapping even after their groups have been changed in lua.
It also prevents a more widespread issue where overriding an item
will add its content ID *twice* to the mapping, resulting in odd
behaviour in features such as ABMs.

diff --git a/src/nodedef.cpp b/src/nodedef.cpp
index 2ffdf2fc2a6ab7e163adac7a7e978fe9055fb159..977a4533dbc149940a8da4338cddf70ba0fee958 100644 (file)
--- a/src/nodedef.cpp
+++ b/src/nodedef.cpp
@@ -1202,6 +1202,26 @@ inline void NodeDefManager::fixSelectionBoxIntUnion()
 }
 
 
+void NodeDefManager::eraseIdFromGroups(content_t id)
+{
+       // For all groups in m_group_to_items...
+       for (auto iter_groups = m_group_to_items.begin();
+                       iter_groups != m_group_to_items.end();) {
+               // Get the group items vector.
+               std::vector<content_t> &items = iter_groups->second;
+
+               // Remove any occurence of the id in the group items vector.
+               items.erase(std::remove(items.begin(), items.end(), id), items.end());
+
+               // If group is empty, erase its vector from the map.
+               if (items.empty())
+                       iter_groups = m_group_to_items.erase(iter_groups);
+               else
+                       ++iter_groups;
+       }
+}
+
+
 // IWritableNodeDefManager
 content_t NodeDefManager::set(const std::string &name, const ContentFeatures &def)
 {
@@ -1222,19 +1242,24 @@ content_t NodeDefManager::set(const std::string &name, const ContentFeatures &de
                assert(id != CONTENT_IGNORE);
                addNameIdMapping(id, name);
        }
+
+       // If there is already ContentFeatures registered for this id, clear old groups
+       if (id < m_content_features.size())
+               eraseIdFromGroups(id);
+
        m_content_features[id] = def;
        verbosestream << "NodeDefManager: registering content id \"" << id
                << "\": name=\"" << def.name << "\""<<std::endl;
 
        getNodeBoxUnion(def.selection_box, def, &m_selection_box_union);
        fixSelectionBoxIntUnion();
+
        // Add this content to the list of all groups it belongs to
-       // FIXME: This should remove a node from groups it no longer
-       // belongs to when a node is re-registered
        for (const auto &group : def.groups) {
                const std::string &group_name = group.first;
                m_group_to_items[group_name].push_back(id);
        }
+
        return id;
 }
 
@@ -1260,18 +1285,7 @@ void NodeDefManager::removeNode(const std::string &name)
                m_name_id_mapping_with_aliases.erase(name);
        }
 
-       // Erase node content from all groups it belongs to
-       for (std::unordered_map<std::string, std::vector<content_t>>::iterator iter_groups =
-                       m_group_to_items.begin(); iter_groups != m_group_to_items.end();) {
-               std::vector<content_t> &items = iter_groups->second;
-               items.erase(std::remove(items.begin(), items.end(), id), items.end());
-
-               // Check if group is empty
-               if (items.empty())
-                       m_group_to_items.erase(iter_groups++);
-               else
-                       ++iter_groups;
-       }
+       eraseIdFromGroups(id);
 }
 
 

diff --git a/src/nodedef.h b/src/nodedef.h
index 60d91f8d99b689f921ae840f66b6cca66a8ae120..1a12aae939580f66e041a9748c24f85d046191c0 100644 (file)
--- a/src/nodedef.h
+++ b/src/nodedef.h
@@ -668,6 +668,14 @@ private:
         */
        void addNameIdMapping(content_t i, std::string name);
 
+       /*!
+        * Removes a content ID from all groups.
+        * Erases content IDs from vectors in \ref m_group_to_items and
+        * removes empty vectors.
+        * @param id Content ID
+        */
+       void eraseIdFromGroups(content_t id);
+
        /*!
         * Recalculates m_selection_box_int_union based on
         * m_selection_box_union.