From 633f15ba1f061ed3953483efab65aa167c68db0a Mon Sep 17 00:00:00 2001 From: Ashton Larkin <42042756+adlarkin@users.noreply.github.com> Date: Fri, 22 Oct 2021 09:19:22 -0600 Subject: [PATCH] backport #1131 Signed-off-by: Ashton Larkin --- src/EntityComponentManager.cc | 50 +++++++--------- src/EntityComponentManager_TEST.cc | 96 ++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 28 deletions(-) diff --git a/src/EntityComponentManager.cc b/src/EntityComponentManager.cc index e23f8674479..024d62f34d9 100644 --- a/src/EntityComponentManager.cc +++ b/src/EntityComponentManager.cc @@ -17,6 +17,8 @@ #include #include +#include +#include #include #include #include @@ -1106,49 +1108,41 @@ void EntityComponentManager::SetState( continue; } - // Create component - auto newComp = components::Factory::Instance()->New(compMsg.type()); - - if (nullptr == newComp) - { - ignerr << "Failed to deserialize component of type [" << compMsg.type() - << "]" << std::endl; - continue; - } - - std::istringstream istr(compMsg.component()); - newComp->Deserialize(istr); - - // Get type id - auto typeId = newComp->TypeId(); - - // TODO(louise) Move into if, see TODO below - this->RemoveComponent(entity, typeId); - // Remove component if (compMsg.remove()) { + this->RemoveComponent(entity, type); continue; } // Get Component - auto comp = this->ComponentImplementation(entity, typeId); + auto comp = this->ComponentImplementation(entity, type); // Create if new if (nullptr == comp) { - this->CreateComponentImplementation(entity, typeId, newComp.get()); + auto newComp = components::Factory::Instance()->New(type); + if (nullptr == newComp) + { + ignerr << "Failed to create component type [" + << compMsg.type() << "]" << std::endl; + continue; + } + std::istringstream istr(compMsg.component()); + newComp->Deserialize(istr); + this->CreateComponentImplementation(entity, type, newComp.get()); } // Update component value else { - ignerr << "Internal error" << std::endl; - // TODO(louise) We're shortcutting above and always removing the - // component so that we don't get here, gotta figure out why this - // doesn't update the component. The following line prints the correct - // values. - // igndbg << *comp << " " << *newComp.get() << std::endl; - // *comp = *newComp.get(); + std::istringstream istr(compMsg.component()); + comp->Deserialize(istr); + // TODO(anyone) mark the component as a one time or periodic change? + // This is done for the SetState method with a msgs::SerializedStateMap + // type (see method below), but nothing is currently being done for a + // msgs::SerializedState type. In ign-gazebo6, there's an + // AddModifiedComponent method that takes care of this, but that method + // was introduced in ign-gazebo4 (see ign-gazebo PR #742) } } } diff --git a/src/EntityComponentManager_TEST.cc b/src/EntityComponentManager_TEST.cc index 824d30758b4..4b688a9e3e3 100644 --- a/src/EntityComponentManager_TEST.cc +++ b/src/EntityComponentManager_TEST.cc @@ -2198,6 +2198,102 @@ TEST_P(EntityComponentManagerFixture, SetEntityCreateOffset) EXPECT_EQ(501u, entity3); } +////////////////////////////////////////////////// +/// \brief Test using msgs::SerializedStateMap and msgs::SerializedState +/// to update existing component data between multiple ECMs +TEST_P(EntityComponentManagerFixture, StateMsgUpdateComponent) +{ + // create 2 ECMs: one will be modified directly, and the other should be + // updated to match the first via msgs::SerializedStateMap + EntityComponentManager originalECMStateMap; + EntityComponentManager otherECMStateMap; + + // create an entity and component + auto entity = originalECMStateMap.CreateEntity(); + originalECMStateMap.CreateComponent(entity, components::IntComponent(1)); + + int foundEntities = 0; + otherECMStateMap.Each( + [&](const Entity &, const components::IntComponent *) + { + foundEntities++; + return true; + }); + EXPECT_EQ(0, foundEntities); + + // update the other ECM to have the new entity and component + msgs::SerializedStateMap stateMapMsg; + originalECMStateMap.State(stateMapMsg); + otherECMStateMap.SetState(stateMapMsg); + foundEntities = 0; + otherECMStateMap.Each( + [&](const Entity &, const components::IntComponent *_intComp) + { + foundEntities++; + EXPECT_EQ(1, _intComp->Data()); + return true; + }); + EXPECT_EQ(1, foundEntities); + + // modify a component and then share the update with the other ECM + stateMapMsg.Clear(); + originalECMStateMap.SetComponentData(entity, 2); + originalECMStateMap.State(stateMapMsg); + otherECMStateMap.SetState(stateMapMsg); + foundEntities = 0; + otherECMStateMap.Each( + [&](const Entity &, const components::IntComponent *_intComp) + { + foundEntities++; + EXPECT_EQ(2, _intComp->Data()); + return true; + }); + EXPECT_EQ(1, foundEntities); + + // Run the same test as above, but this time, use a msgs::SerializedState + // instead of a msgs::SerializedStateMap + EntityComponentManager originalECMState; + EntityComponentManager otherECMState; + + foundEntities = 0; + otherECMState.Each( + [&](const Entity &, const components::IntComponent *) + { + foundEntities++; + return true; + }); + EXPECT_EQ(0, foundEntities); + + entity = originalECMState.CreateEntity(); + originalECMState.CreateComponent(entity, components::IntComponent(1)); + + auto stateMsg = originalECMState.State(); + otherECMState.SetState(stateMsg); + foundEntities = 0; + otherECMState.Each( + [&](const Entity &, const components::IntComponent *_intComp) + { + foundEntities++; + EXPECT_EQ(1, _intComp->Data()); + return true; + }); + EXPECT_EQ(1, foundEntities); + + stateMsg.Clear(); + originalECMState.SetComponentData(entity, 2); + stateMsg = originalECMState.State(); + otherECMState.SetState(stateMsg); + foundEntities = 0; + otherECMState.Each( + [&](const Entity &, const components::IntComponent *_intComp) + { + foundEntities++; + EXPECT_EQ(2, _intComp->Data()); + return true; + }); + EXPECT_EQ(1, foundEntities); +} + // Run multiple times. We want to make sure that static globals don't cause // problems. INSTANTIATE_TEST_SUITE_P(EntityComponentManagerRepeat,