Skip to content

Commit

Permalink
#5613: Even if a scene::Node got deleted, we still need to notify the…
Browse files Browse the repository at this point in the history
… spawnarg collection about this. Add corresponding unit test.
  • Loading branch information
codereader committed Oct 17, 2021
1 parent 464ec1f commit fb0ed63
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 7 deletions.
12 changes: 5 additions & 7 deletions libs/selection/EntitySelection.h
Expand Up @@ -57,11 +57,6 @@ class EntitySelection final :

~SpawnargTracker()
{
if (_node.expired())
{
return; // cannot unsubscribe, the node is gone
}

// 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
Expand All @@ -71,8 +66,11 @@ class EntitySelection final :
auto entity = _entity;
_entity = nullptr;

// Detaching the observer won't do anything here anymore
entity->detachObserver(this);
if (!_node.expired())
{
// Detaching the observer won't do anything here anymore
entity->detachObserver(this);
}
}

scene::INodePtr getNode() const
Expand Down
29 changes: 29 additions & 0 deletions test/EntityInspector.cpp
Expand Up @@ -7,6 +7,7 @@
#include "iselectable.h"
#include "selection/EntitySelection.h"
#include "algorithm/Entity.h"
#include "scenelib.h"

namespace test
{
Expand Down Expand Up @@ -494,4 +495,32 @@ TEST_F(EntityInspectorTest, UndoRedoKeyValueAdditionRemoval)
}
}

TEST_F(EntityInspectorTest, DeletedEntitiesAreSafelyUntracked)
{
KeyValueStore keyValueStore;

// Create a single entity
auto cls = GlobalEntityClassManager().findClass("atdm:ai_builder_guard");
auto guardNode = GlobalEntityModule().createEntity(cls);

// Create a weak reference to check whether the entity is gone
std::weak_ptr<IEntityNode> weakGuardNode(guardNode);

scene::addNodeToContainer(guardNode, GlobalMapModule().getRoot());
Node_setSelected(guardNode, true);

keyValueStore.rescanSelection();

EXPECT_FALSE(keyValueStore.store.empty()) << "No key values visible after selecting the entity";
guardNode.reset();

// Close the existing map, this should release all references
GlobalCommandSystem().executeCommand("NewMap");

EXPECT_TRUE(weakGuardNode.expired()) << "Guard node is still alive after changing maps";
keyValueStore.rescanSelection();

EXPECT_TRUE(keyValueStore.store.empty()) << "No key values should be visible after changing maps";
}

}

0 comments on commit fb0ed63

Please sign in to comment.