From f07c4817952c26b6da0aaa64ee18a6f8894251a0 Mon Sep 17 00:00:00 2001 From: "Addisu Z. Taddese" Date: Thu, 12 Sep 2024 14:28:51 -0700 Subject: [PATCH 1/4] Fix log playback GUI display (#2611) Loading the logging playback plugin before `SceneBroadcaster` prevents playback from being displayed on the GUI. --------- Signed-off-by: Addisu Z. Taddese Co-authored-by: Arjo Chakravarty (cherry picked from commit ca40c1db3dc0b6d862916c4990a9bfb35a62d0e8) # Conflicts: # src/SimulationRunner.cc --- src/SimulationRunner.cc | 42 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/SimulationRunner.cc b/src/SimulationRunner.cc index 5ce443ff19..816d5a0bf9 100644 --- a/src/SimulationRunner.cc +++ b/src/SimulationRunner.cc @@ -1587,8 +1587,6 @@ void SimulationRunner::CreateEntities(const sdf::World &_world) this->entityCompMgr.ProcessRemoveEntityRequests(); this->entityCompMgr.ClearRemovedComponents(); - this->LoadLoggingPlugins(this->serverConfig); - // Load any additional plugins from the Server Configuration this->LoadServerPlugins(this->serverConfig.Plugins()); @@ -1598,9 +1596,49 @@ void SimulationRunner::CreateEntities(const sdf::World &_world) { gzmsg << "No systems loaded from SDF, loading defaults" << std::endl; bool isPlayback = !this->serverConfig.LogPlaybackPath().empty(); +<<<<<<< HEAD auto plugins = gz::sim::loadPluginInfo(isPlayback); this->LoadServerPlugins(plugins); } +======= + auto defaultPlugins = gz::sim::loadPluginInfo(isPlayback); + if (loadedWorldPlugins.empty()) + { + gzmsg << "No systems loaded from SDF, loading defaults" << std::endl; + } + else + { + std::unordered_set loadedWorldPluginFileNames; + for (const auto &pl : loadedWorldPlugins) + { + loadedWorldPluginFileNames.insert(pl.fname); + } + auto isPluginLoaded = + [&loadedWorldPluginFileNames](const ServerConfig::PluginInfo &_pl) + { + return loadedWorldPluginFileNames.count(_pl.Plugin().Filename()) != 0; + }; + + // Remove plugin if it's already loaded so as to not duplicate world + // plugins. + defaultPlugins.remove_if(isPluginLoaded); + + gzdbg << "Additional plugins to load:\n"; + for (const auto &plugin : defaultPlugins) + { + gzdbg << plugin.Plugin().Name() << " " << plugin.Plugin().Filename() + << "\n"; + } + } + + this->LoadServerPlugins(defaultPlugins); + // Load logging plugins after all server plugins so that necessary + // plugins such as SceneBroadcaster are loaded first. This might be + // a bug or an assumption made in the logging plugins. + this->LoadLoggingPlugins(this->serverConfig); + + }; +>>>>>>> ca40c1db (Fix log playback GUI display (#2611)) // Store the initial state of the ECM; this->initialEntityCompMgr.CopyFrom(this->entityCompMgr); From d1d9440245734aa63e02ddab8b6be93d88fdf172 Mon Sep 17 00:00:00 2001 From: Arjo Chakravarty Date: Tue, 5 Aug 2025 07:56:23 +0000 Subject: [PATCH 2/4] Backport changes Signed-off-by: Arjo Chakravarty --- src/SimulationRunner.cc | 47 +++++------------------------------------ 1 file changed, 5 insertions(+), 42 deletions(-) diff --git a/src/SimulationRunner.cc b/src/SimulationRunner.cc index 816d5a0bf9..899ec1cf06 100644 --- a/src/SimulationRunner.cc +++ b/src/SimulationRunner.cc @@ -504,7 +504,7 @@ void SimulationRunner::AddSystem(const SystemPluginPtr &_system, void SimulationRunner::AddSystem( const std::shared_ptr &_system, std::optional _entity, - std::optional> _sdf) + std::optional> _sdf) { auto entity = _entity.value_or(worldEntity(this->entityCompMgr)); auto sdf = _sdf.value_or(createEmptyPluginElement()); @@ -1596,50 +1596,13 @@ void SimulationRunner::CreateEntities(const sdf::World &_world) { gzmsg << "No systems loaded from SDF, loading defaults" << std::endl; bool isPlayback = !this->serverConfig.LogPlaybackPath().empty(); -<<<<<<< HEAD auto plugins = gz::sim::loadPluginInfo(isPlayback); this->LoadServerPlugins(plugins); } -======= - auto defaultPlugins = gz::sim::loadPluginInfo(isPlayback); - if (loadedWorldPlugins.empty()) - { - gzmsg << "No systems loaded from SDF, loading defaults" << std::endl; - } - else - { - std::unordered_set loadedWorldPluginFileNames; - for (const auto &pl : loadedWorldPlugins) - { - loadedWorldPluginFileNames.insert(pl.fname); - } - auto isPluginLoaded = - [&loadedWorldPluginFileNames](const ServerConfig::PluginInfo &_pl) - { - return loadedWorldPluginFileNames.count(_pl.Plugin().Filename()) != 0; - }; - - // Remove plugin if it's already loaded so as to not duplicate world - // plugins. - defaultPlugins.remove_if(isPluginLoaded); - - gzdbg << "Additional plugins to load:\n"; - for (const auto &plugin : defaultPlugins) - { - gzdbg << plugin.Plugin().Name() << " " << plugin.Plugin().Filename() - << "\n"; - } - } - - this->LoadServerPlugins(defaultPlugins); - // Load logging plugins after all server plugins so that necessary - // plugins such as SceneBroadcaster are loaded first. This might be - // a bug or an assumption made in the logging plugins. - this->LoadLoggingPlugins(this->serverConfig); - - }; ->>>>>>> ca40c1db (Fix log playback GUI display (#2611)) - + // Load logging plugins after all server plugins so that necessary + // plugins such as SceneBroadcaster are loaded first. This might be + // a bug or an assumption made in the logging plugins. + this->LoadLoggingPlugins(this->serverConfig); // Store the initial state of the ECM; this->initialEntityCompMgr.CopyFrom(this->entityCompMgr); From 57a18b8ab5d4fff1cb972535ed376966befd732a Mon Sep 17 00:00:00 2001 From: Arjo Chakravarty Date: Tue, 5 Aug 2025 08:35:55 +0000 Subject: [PATCH 3/4] Style Signed-off-by: Arjo Chakravarty --- src/SimulationRunner.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SimulationRunner.cc b/src/SimulationRunner.cc index 899ec1cf06..21cf1ca3f7 100644 --- a/src/SimulationRunner.cc +++ b/src/SimulationRunner.cc @@ -504,7 +504,7 @@ void SimulationRunner::AddSystem(const SystemPluginPtr &_system, void SimulationRunner::AddSystem( const std::shared_ptr &_system, std::optional _entity, - std::optional> _sdf) + std::optional> _sdf) { auto entity = _entity.value_or(worldEntity(this->entityCompMgr)); auto sdf = _sdf.value_or(createEmptyPluginElement()); From 1d04b4c4b5636ee8bdbc6f9d405041bb33ebc5da Mon Sep 17 00:00:00 2001 From: Arjo Chakravarty Date: Wed, 6 Aug 2025 05:45:06 +0000 Subject: [PATCH 4/4] Update test expectations Signed-off-by: Arjo Chakravarty --- src/Server_TEST.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Server_TEST.cc b/src/Server_TEST.cc index b07157b094..a7c327f0db 100644 --- a/src/Server_TEST.cc +++ b/src/Server_TEST.cc @@ -442,8 +442,7 @@ TEST_P(ServerFixture, GZ_UTILS_TEST_DISABLED_ON_WIN32(ServerConfigLogRecord)) EXPECT_EQ(0u, *server.IterationCount()); EXPECT_EQ(3u, *server.EntityCount()); - // Only the log record system is needed and therefore loaded. - EXPECT_EQ(1u, *server.SystemCount()); + EXPECT_EQ(4u, *server.SystemCount()); EXPECT_TRUE(serverConfig.LogRecordTopics().empty()); serverConfig.AddLogRecordTopic("test_topic1"); @@ -482,9 +481,7 @@ TEST_P(ServerFixture, sim::Server server(serverConfig); EXPECT_EQ(0u, *server.IterationCount()); EXPECT_EQ(3u, *server.EntityCount()); - - // Only the log record system is needed and therefore loaded. - EXPECT_EQ(1u, *server.SystemCount()); + EXPECT_EQ(4u, *server.SystemCount()); } EXPECT_FALSE(common::exists(logFile));