#5613: Change ownership of the CollectiveSpawnargs helper. It's now o…

…wned and listened to by the EntityInspector, the EntitySelection class merely monitors the selection and dispatches the key observer events.

libs/selection/EntitySelection.h

@@ -11,8 +11,24 @@
 namespace selection
 {
 
-// Helper object used by the Entity Inspector to track the 
-// selected entities in the scene.
+/**
+ * Helper object used by the Entity Inspector to track all selected entities
+ * in the scene and keep the list of their spawnargs up to date.
+ * 
+ * On update() it rescans the entity selection in the scene, triggering
+ * the contained CollectiveSpawnargs object to emit its signals. The signals
+ * are emitted such that the listening EntityInspector will show the 
+ * correct set of keys: the ones with shared values will show just that,
+ * the keys with differing values will show a placeholder text instead.
+ * 
+ * This instance will monitor the selected entities to dispatch all 
+ * key value changes to the CollectiveSpawnargs.
+ * Selection updates will not take effect until update() is called, which 
+ * ideally should happen in an idle processing loop after a selection change.
+ * 
+ * This class keeps weak references to the scene::Nodes to not interfere with
+ * node destruction.
+ */
 class EntitySelection final
 {
 private:
@@ -44,7 +60,7 @@ class EntitySelection final
                 return; // cannot unsubscribe, the node is gone
             }
 
-            // Call the cleanupEntity method instead of relying on the onKeyErase()
+            // Call the onEntityRemoved method instead of relying on the onKeyErase()
             // invocations when detaching the observer. This allows us to keep some
             // assumptions about entity count in the CollectiveSpawnargs::onKeyErase method
             _spawnargCollection.onEntityRemoved(_entity);
@@ -83,19 +99,18 @@ class EntitySelection final
 
     std::list<SpawnargTracker> _trackedEntities;
 
-    CollectiveSpawnargs _spawnargs;
+    CollectiveSpawnargs& _spawnargs;
 
 public:
+    EntitySelection(CollectiveSpawnargs& spawnargs) :
+        _spawnargs(spawnargs)
+    {}
+
     ~EntitySelection()
     {
         _trackedEntities.clear();
     }
 
-    CollectiveSpawnargs& getSpawnargs()
-    {
-        return _spawnargs;
-    }
-
     std::size_t size() const
     {
         return _trackedEntities.size();
@@ -160,11 +175,6 @@ class EntitySelection final
         }
     }
 
-    void foreachKey(const std::function<void(const std::string&, const CollectiveSpawnargs::KeyValueSet&)>& functor)
-    {
-        _spawnargs.foreachKey(functor);
-    }
-
 private:
     scene::INodePtr getEntityForNode(const scene::INodePtr& selected)
     {

radiant/ui/einspector/EntityInspector.cpp

@@ -141,18 +141,20 @@ void EntityInspector::construct()
     // Register self to the SelectionSystem to get notified upon selection
     // changes.
     GlobalSelectionSystem().addObserver(this);
-    _entitySelection.reset(new selection::EntitySelection);
+    _spawnargs.reset(new selection::CollectiveSpawnargs);
 
     // Connect the signals
-    _keyValueAddedHandler = _entitySelection->getSpawnargs().signal_KeyAdded().connect(
+    _keyValueAddedHandler = _spawnargs->signal_KeyAdded().connect(
         sigc::mem_fun(this, &EntityInspector::onKeyAdded)
     );
-    _keyValueRemovedHandler = _entitySelection->getSpawnargs().signal_KeyRemoved().connect(
+    _keyValueRemovedHandler = _spawnargs->signal_KeyRemoved().connect(
         sigc::mem_fun(this, &EntityInspector::onKeyRemoved)
     );
-    _keyValueSetChangedHandler = _entitySelection->getSpawnargs().signal_KeyValueSetChanged().connect(
+    _keyValueSetChangedHandler = _spawnargs->signal_KeyValueSetChanged().connect(
         sigc::mem_fun(this, &EntityInspector::onKeyValueSetChanged)
     );
+
+    _entitySelection = std::make_unique<selection::EntitySelection>(*_spawnargs);
 
     _defsReloadedHandler = GlobalEntityClassManager().defsReloadedSignal().connect(
         sigc::mem_fun(this, &EntityInspector::onDefsReloaded)
@@ -467,6 +469,7 @@ void EntityInspector::onMainFrameShuttingDown()
     _keyValueRemovedHandler.disconnect();
     _keyValueSetChangedHandler.disconnect();
     _entitySelection.reset();
+    _spawnargs.reset();
 
     _mergeActions.clear();
     _conflictActions.clear();
@@ -1667,6 +1670,7 @@ void EntityInspector::changeSelectedEntity(const scene::INodePtr& newEntity, con
     // Reset the sorting when changing entities
     _keyValueTreeView->ResetSortingOnAllColumns();
 
+#if 0
     _entitySelection->foreachKey([&](const std::string& key, const selection::CollectiveSpawnargs::KeyValueSet& set)
     {
         if (set.valueIsEqualOnAllEntities)
@@ -1679,7 +1683,7 @@ void EntityInspector::changeSelectedEntity(const scene::INodePtr& newEntity, con
             onKeyChange(key, _("[multiple values]"));
         }
     });
-
+#endif
 #if 0
     // Attach to new entity if it is non-NULL
     if (newEntity && newEntity->getNodeType() == scene::INode::Type::Entity)

radiant/ui/einspector/EntityInspector.h

@@ -32,7 +32,11 @@ class wxTextCtrl;
 class wxBitmapButton;
 class wxDataViewColumn;
 
-namespace selection { class EntitySelection; }
+namespace selection
+{
+    class CollectiveSpawnargs;
+    class EntitySelection;
+}
 
 namespace ui
 {
@@ -80,6 +84,7 @@ class EntityInspector :
 	// Currently selected entity, this pointer is only non-NULL if the
 	// current entity selection includes exactly 1 entity.
 	scene::INodeWeakPtr _selectedEntity;
+    std::unique_ptr<selection::CollectiveSpawnargs> _spawnargs;
     std::unique_ptr<selection::EntitySelection> _entitySelection;
 
 	// Main EntityInspector widget

test/EntityInspector.cpp

@@ -27,38 +27,36 @@ class KeyValueStore :
     public sigc::trackable
 {
 private:
+    std::unique_ptr<selection::CollectiveSpawnargs> _spawnargs;
     std::unique_ptr<selection::EntitySelection> _selectionTracker;
 
 public:
     static constexpr const char* DifferingValues = "[differing values]";
 
     KeyValueStore()
     {
-        _selectionTracker.reset(new selection::EntitySelection);
+        _spawnargs.reset(new selection::CollectiveSpawnargs);
+        _selectionTracker.reset(new selection::EntitySelection(*_spawnargs));
 
-        _selectionTracker->getSpawnargs().signal_KeyAdded().connect(
+        _spawnargs->signal_KeyAdded().connect(
             sigc::mem_fun(this, &KeyValueStore::onKeyAdded)
         );
-        _selectionTracker->getSpawnargs().signal_KeyRemoved().connect(
+        _spawnargs->signal_KeyRemoved().connect(
             sigc::mem_fun(this, &KeyValueStore::onKeyRemoved)
         );
-        _selectionTracker->getSpawnargs().signal_KeyValueSetChanged().connect(
+        _spawnargs->signal_KeyValueSetChanged().connect(
             sigc::mem_fun(this, &KeyValueStore::onKeyValueSetChanged)
         );
     }
 
     ~KeyValueStore()
     {
         _selectionTracker.reset();
+        _spawnargs.reset();
     }
 
     std::map<std::string, std::string> store;
 
-    selection::CollectiveSpawnargs& getSpawnargs()
-    {
-        return _selectionTracker->getSpawnargs();
-    }
-
     void rescanSelection()
     {
         _selectionTracker->update();