Fix conflicting names and bad model keys before saving a map

README.md

@@ -31,6 +31,8 @@ Here is an overview of the changes made to TrenchBroom:
 * New Doom 3 OBJ parser. My TB Interop branch automatically creates OBJ files to work with TB and it also allows seamless interop with Blender 2.8x and 2.9x with the need of additional model formats for func_static entities (like misc_model for Quake 3)
 * Game FGDs for Doom 3 and Doom 3 BFG
 * Custom PNG icons for special entities like lights, speakers, particle emitters, info_location, target_* and so on
+* After loading a map TrenchBroom generates unique entity names and also fixes missing or bad "model" keys for brush based entitites
+* Creating new entities like a light will automatically receive unique names like light_2
 
 ## Issues
 * It doesn't allow to create bezier patches at the moment so you won't be able to edit existing Doom 3 maps
@@ -41,6 +43,7 @@ Here is an overview of the changes made to TrenchBroom:
 * Some ASE models can't be loaded and materials are usually all wrong if loaded
 * TrenchBroom doesn't support the "rotation" keyword and many models have the wrong orientation
 * The custom TrenchBroom build breaks compatibility for other id Tech engines, e.g. entity links work between "target[num]" and "name" and not "targetname"
+* Linked groups do not work yet with id Tech 4 due to name conflicts
 
 
 ## Releases

common/src/Model/EntityNodeBase.cpp

@@ -382,6 +382,33 @@ bool EntityNodeBase::getTargetname(std::string& result) {
 
   return false;
 }
+
+bool EntityNodeBase::getModelname(std::string& result) {
+
+  const std::string* modelname = m_entity.property(EntityPropertyKeys::Model);
+  if (modelname != nullptr && !modelname->empty()) {
+
+    result = *modelname;
+    return true;
+  }
+
+  return false;
+}
+
+// brush entities need a "model" key that is the same as the "name" key
+bool EntityNodeBase::hasBadModelname() {
+
+  const std::string* targetname = m_entity.property(EntityPropertyKeys::Targetname);
+  if (targetname != nullptr && !targetname->empty()) {
+
+    const std::string* modelname = m_entity.property(EntityPropertyKeys::Model);
+    if (modelname != nullptr && !modelname->empty() && *targetname == *modelname) {
+      return false;
+    }
+  }
+
+  return true;
+}
 // RB end
 
 std::vector<std::string> EntityNodeBase::findMissingLinkTargets() const {

common/src/Model/EntityNodeBase.h

@@ -108,11 +108,17 @@ class EntityNodeBase : public Node {
 
   bool hasMissingSources() const;
   bool hasMissingTargetname() const;
+
+  // RB begin
   bool hasConflictingTargetname() const;
   void generateUniqueTargetname(std::string& result) const;
   void generateUniqueTargetnameForClassname(
     const std::string& classname, std::string& result) const;
   bool getTargetname(std::string& result);
+  bool getModelname(std::string& result);
+
+  bool hasBadModelname();
+  // RB end
 
   std::vector<std::string> findMissingLinkTargets() const;
   std::vector<std::string> findMissingKillTargets() const;

common/src/View/MapDocument.cpp

@@ -567,6 +567,10 @@ void MapDocument::exportDocumentAs(const IO::ExportOptions& options) {
 }
 
 void MapDocument::doSaveDocument(const IO::Path& path) {
+
+  // RB: HACK fix all duplicated names and wrong "model" values for brush entities
+  fixBadEntityNamesAndModels();
+
   saveDocumentTo(path);
   setLastSaveModificationCount();
   setPath(path);
@@ -3664,7 +3668,7 @@ void MapDocument::loadAssets() {
   setTextures();
 
   // RB: Doom 3 specific - give every entity a unique name if not done yet
-  setUniqueEntityNames();
+  fixBadEntityNamesAndModels();
 }
 
 void MapDocument::unloadAssets() {
@@ -3811,7 +3815,8 @@ void MapDocument::unsetTextures(const std::vector<Model::Node*>& nodes) {
 }
 
 // RB: give every entity a unique name like DoomEdit does
-void MapDocument::setUniqueEntityNames() {
+void MapDocument::fixBadEntityNamesAndModels() {
+
   m_world->accept(kdl::overload(
     [](auto&& thisLambda, Model::WorldNode* world) {
       world->visitChildren(thisLambda);
@@ -3833,21 +3838,40 @@ void MapDocument::setUniqueEntityNames() {
         select(entityNode);
 
         setProperty(Model::EntityPropertyKeys::Targetname, uniqueName);
-      }
 
-      // second if this is a brush entity then Doom 3 can't load the model if there is no "model"
-      // key so make sure it has one and it's the same as the name
-      if (entityNode->hasChildren()) {
+        // second if this is a brush entity then Doom 3 can't load the model if there is no "model"
+        // key so make sure it has one and it's the same as the name
+        if (
+          entityNode->hasChildren() &&
+          entityNode != reinterpret_cast<Model::EntityNode*>(m_world.get())) {
+
+          setProperty(Model::EntityPropertyKeys::Model, uniqueName);
+
+          // nodesDidChange = true;
+        }
+      }
 
+      // check if the name was proper set if there are any bad model keys left
+      if (
+        entityNode->hasChildren() &&
+        entityNode != reinterpret_cast<Model::EntityNode*>(m_world.get()) &&
+        entityNode->hasBadModelname()) {
         std::string uniqueName;
         if (entityNode->getTargetname(uniqueName)) {
           setProperty(Model::EntityPropertyKeys::Model, uniqueName);
+
+          // nodesDidChange = true;
         }
       }
     },
     [&](Model::BrushNode*) {}, [](Model::PatchNode*) {}));
 
   deselectAll();
+
+  // clear issues in the issue browser
+  // FIXME: seems not to work
+  const auto nodes = std::vector<Model::Node*>{m_world.get()};
+  NotifyBeforeAndAfter notifyNodes(nodesWillChangeNotifier, nodesDidChangeNotifier, nodes);
 }
 // RB end
 

common/src/View/MapDocument.h

@@ -676,7 +676,7 @@ class MapDocument : public Model::MapFacade, public CachingLogger {
   void unsetTextures();
   void unsetTextures(const std::vector<Model::Node*>& nodes);
 
-  void setUniqueEntityNames();
+  void fixBadEntityNamesAndModels();
 
   void setEntityDefinitions();
   void setEntityDefinitions(const std::vector<Model::Node*>& nodes);