Set sdf::Root to contain only one model/actor/light

Migration.md

@@ -23,6 +23,13 @@ but with improved human-readability..
 1. **sdf/Model.hh**:
     + std::pair<const Link *, std::string> CanonicalLinkAndRelativeName() const;
 
+1. **sdf/Root.hh** sdf::Root elements can now only contain one of either Model,
+      Light or Actor since multiple items would conflict with overrides
+      specified in an <include> tag.
+    + const sdf::Model \*Model();
+    + const sdf::Light \*Light();
+    + const sdf::Actor \*Actor();
+
 ### Modifications
 
 1. **sdf/Model.hh**: the following methods now accept nested names relative to
@@ -36,6 +43,21 @@ but with improved human-readability..
     + bool JointNameExists(const std::string &) const
     + bool LinkNameExists(const std::string &) const
 
+### Deprecations
+
+1. **src/Root.hh**: The following methods have been deprecated in favor of the
+      new methods. For now the behavior is unchanged, but Root elements should
+      only contain one or none of Model/Light/Actor.
+    + const sdf::Model \*ModelByIndex();
+    + uint64_t ModelCount();
+    + bool ModelNameExists(const std::string &\_name) const;
+    + const sdf::Light \*LightByIndex();
+    + uint64_t LightCount();
+    + bool LightNameExists(const std::string &\_name) const;
+    + const sdf::Actor \*ActorByIndex();
+    + uint64_t ActorCount();
+    + bool ActorNameExists(const std::string &\_name) const;
+
 ## SDFormat 9.x to 10.0
 
 ### Modifications

include/sdf/Root.hh

@@ -127,51 +127,92 @@ namespace sdf
     /// \brief Get the number of models that are immediate (not nested) children
     /// of this Root object.
     /// \return Number of models contained in this Root object.
-    public: uint64_t ModelCount() const;
+    public: uint64_t ModelCount() const SDF_DEPRECATED(11.0);
 
     /// \brief Get a model based on an index.
     /// \param[in] _index Index of the model. The index should be in the
     /// range [0..ModelCount()).
     /// \return Pointer to the model. Nullptr if the index does not exist.
     /// \sa uint64_t ModelCount() const
-    public: const Model *ModelByIndex(const uint64_t _index) const;
+    public: const sdf::Model *ModelByIndex(const uint64_t _index) const
+        SDF_DEPRECATED(11.0);
 
     /// \brief Get whether a model name exists.
     /// \param[in] _name Name of the model to check.
     /// \return True if there exists a model with the given name.
-    public: bool ModelNameExists(const std::string &_name) const;
+    public: bool ModelNameExists(const std::string &_name) const
+        SDF_DEPRECATED(11.0);
+
+    /// \brief Get a pointer to the model object if it exists.
+    ///
+    /// If there is more than one model, this will return the first element.
+    /// This method is preferred to ModelByIndex, as its behavior is
+    /// consistent with the planned future API. Having more than one Model, or
+    /// more than one of Model/Actor/Light is now considered deprecated and
+    /// should not be relied upon going forward.
+    ///
+    /// \return A pointer to the model, nullptr if it doesn't exist
+    public: const sdf::Model *Model() const;
 
     /// \brief Get the number of lights.
     /// \return Number of lights contained in this Root object.
-    public: uint64_t LightCount() const;
+    public: uint64_t LightCount() const SDF_DEPRECATED(11.0);
 
     /// \brief Get a light based on an index.
     /// \param[in] _index Index of the light. The index should be in the
     /// range [0..LightCount()).
     /// \return Pointer to the light. Nullptr if the index does not exist.
     /// \sa uint64_t LightCount() const
-    public: const Light *LightByIndex(const uint64_t _index) const;
+    public: const sdf::Light *LightByIndex(const uint64_t _index) const
+        SDF_DEPRECATED(11.0);
 
     /// \brief Get whether a light name exists.
     /// \param[in] _name Name of the light to check.
     /// \return True if there exists a light with the given name.
-    public: bool LightNameExists(const std::string &_name) const;
+    public: bool LightNameExists(const std::string &_name) const
+        SDF_DEPRECATED(11.0);
+
+    /// \brief Get a pointer to the light object if it exists.
+    ///
+    /// If there is more than one light, this will return the first element. If
+    /// there is already a model element, this will return null.
+    /// This method is preferred to LightByIndex, as its behavior is
+    /// consistent with the planned future API. Having more than one Light, or
+    /// more than one of Model/Actor/Light is now considered deprecated and
+    /// should not be relied upon going forward.
+    ///
+    /// \return A pointer to the light, nullptr if it doesn't exist
+    public: const sdf::Light *Light() const;
 
     /// \brief Get the number of actors.
     /// \return Number of actors contained in this Root object.
-    public: uint64_t ActorCount() const;
+    public: uint64_t ActorCount() const SDF_DEPRECATED(11.0);
 
     /// \brief Get an actor based on an index.
     /// \param[in] _index Index of the actor. The actor should be in the
     /// range [0..ActorCount()).
     /// \return Pointer to the actor. Nullptr if the index does not exist.
     /// \sa uint64_t ActorCount() const
-    public: const Actor *ActorByIndex(const uint64_t _index) const;
+    public: const sdf::Actor *ActorByIndex(const uint64_t _index) const
+        SDF_DEPRECATED(11.0);
 
     /// \brief Get whether an actor name exists.
     /// \param[in] _name Name of the actor to check.
     /// \return True if there exists an actor with the given name.
-    public: bool ActorNameExists(const std::string &_name) const;
+    public: bool ActorNameExists(const std::string &_name) const
+        SDF_DEPRECATED(11.0);
+
+    /// \brief Get a pointer to the actor object if it exists.
+    ///
+    /// If there is more than one actor, this will return the first element. If
+    /// there is already a model or light element, this will return null.
+    /// This method is preferred to ActorByIndex, as its behavior is
+    /// consistent with the planned future API. Having more than one Actor, or
+    /// more than one of Model/Actor/Light is now considered deprecated and
+    /// should not be relied upon going forward.
+    ///
+    /// \return A pointer to the actor, nullptr if it doesn't exist
+    public: const sdf::Actor *Actor() const;
 
     /// \brief Get a pointer to the SDF element that was generated during
     /// load.

src/FrameSemantics_TEST.cc

@@ -48,7 +48,7 @@ TEST(FrameSemantics, buildFrameAttachedToGraph_Model)
   EXPECT_TRUE(errors.empty()) << errors;
 
   // Get the first model
-  const sdf::Model *model = root.ModelByIndex(0);
+  const sdf::Model *model = root.Model();
 
   auto ownedGraph = std::make_shared<sdf::FrameAttachedToGraph>();
   sdf::ScopedGraph<sdf::FrameAttachedToGraph> graph(ownedGraph);
@@ -217,7 +217,7 @@ TEST(FrameSemantics, buildPoseRelativeToGraph)
   EXPECT_TRUE(root.Load(testFile).empty());
 
   // Get the first model
-  const sdf::Model *model = root.ModelByIndex(0);
+  const sdf::Model *model = root.Model();
 
   auto ownedGraph = std::make_shared<sdf::PoseRelativeToGraph>();
   sdf::ScopedGraph<sdf::PoseRelativeToGraph> graph(ownedGraph);
@@ -313,7 +313,7 @@ TEST(NestedFrameSemantics, buildFrameAttachedToGraph_Model)
   EXPECT_TRUE(errors.empty()) << errors;
 
   // Get the first model
-  const sdf::Model *model = root.ModelByIndex(0);
+  const sdf::Model *model = root.Model();
 
   auto ownedGraph = std::make_shared<sdf::FrameAttachedToGraph>();
   sdf::ScopedGraph<sdf::FrameAttachedToGraph> graph(ownedGraph);
@@ -592,7 +592,7 @@ TEST(NestedFrameSemantics, ModelWithoutLinksWithNestedStaticModel)
   auto errors = root.Load(testFile);
   EXPECT_TRUE(errors.empty()) << errors;
 
-  const sdf::Model *model = root.ModelByIndex(0);
+  const sdf::Model *model = root.Model();
   ASSERT_NE(nullptr, model);
 
   auto ownedModelGraph = std::make_shared<sdf::FrameAttachedToGraph>();

src/Root.cc

@@ -15,6 +15,7 @@
  *
 */
 #include <string>
+#include <variant>
 #include <vector>
 #include <utility>
 
@@ -41,20 +42,32 @@ class sdf::RootPrivate
   /// \brief The worlds specified under the root SDF element
   public: std::vector<World> worlds;
 
+  /// \brief A model, light or actor under the root SDF element
+  /// This variant does not currently retain ownership, and instead points to
+  /// the first model/light/actor if they exist. When the vectors below are
+  /// removed this should be changed from a variant of pointers to a variant of
+  /// objects
+  public: std::variant<std::monostate, sdf::Model*, sdf::Light*, sdf::Actor*>
+              modelLightOrActor;
+
   /// \brief The models specified under the root SDF element
-  public: std::vector<Model> models;
+  /// Deprecated: to be removed in libsdformat12
+  public: std::vector<sdf::Model> models;
 
   /// \brief The lights specified under the root SDF element
-  public: std::vector<Light> lights;
+  /// Deprecated: to be removed in libsdformat12
+  public: std::vector<sdf::Light> lights;
 
   /// \brief The actors specified under the root SDF element
-  public: std::vector<Actor> actors;
+  /// Deprecated: to be removed in libsdformat12
+  public: std::vector<sdf::Actor> actors;
 
   /// \brief Frame Attached-To Graphs constructed when loading Worlds.
   public: std::vector<sdf::ScopedGraph<FrameAttachedToGraph>>
               worldFrameAttachedToGraphs;
 
   /// \brief Frame Attached-To Graphs constructed when loading Models.
+  /// Deprecated: to be removed in libsdformat12 and converted to a single graph
   public: std::vector<sdf::ScopedGraph<FrameAttachedToGraph>>
               modelFrameAttachedToGraphs;
 
@@ -63,6 +76,7 @@ class sdf::RootPrivate
               worldPoseRelativeToGraphs;
 
   /// \brief Pose Relative-To Graphs constructed when loading Models.
+  /// Deprecated: to be removed in libsdformat12 and converted to a single graph
   public: std::vector<sdf::ScopedGraph<PoseRelativeToGraph>>
               modelPoseRelativeToGraphs;
 
@@ -254,9 +268,13 @@ Errors Root::Load(SDFPtr _sdf)
   }
 
   // Load all the models.
-  Errors modelLoadErrors = loadUniqueRepeated<Model>(
+  Errors modelLoadErrors = loadUniqueRepeated<sdf::Model>(
       this->dataPtr->sdf, "model", this->dataPtr->models);
   errors.insert(errors.end(), modelLoadErrors.begin(), modelLoadErrors.end());
+  if (!this->dataPtr->models.empty())
+  {
+    this->dataPtr->modelLightOrActor = &this->dataPtr->models.front();
+  }
 
   // Build the graphs.
   for (sdf::Model &model : this->dataPtr->models)
@@ -272,14 +290,42 @@ Errors Root::Load(SDFPtr _sdf)
   }
 
   // Load all the lights.
-  Errors lightLoadErrors = loadUniqueRepeated<Light>(this->dataPtr->sdf,
+  Errors lightLoadErrors = loadUniqueRepeated<sdf::Light>(this->dataPtr->sdf,
       "light", this->dataPtr->lights);
   errors.insert(errors.end(), lightLoadErrors.begin(), lightLoadErrors.end());
+  if (!this->dataPtr->lights.empty())
+  {
+    if (std::holds_alternative<std::monostate>(
+      this->dataPtr->modelLightOrActor))
+    {
+      this->dataPtr->modelLightOrActor = &this->dataPtr->lights.front();
+    }
+    else
+    {
+      sdfwarn << "Root object can only contain one of model, light or actor. "
+              << "This behavior was deprecated in libsdformat11 and will soon "
+              << "be removed";
+    }
+  }
 
   // Load all the actors.
-  Errors actorLoadErrors = loadUniqueRepeated<Actor>(this->dataPtr->sdf,
+  Errors actorLoadErrors = loadUniqueRepeated<sdf::Actor>(this->dataPtr->sdf,
       "actor", this->dataPtr->actors);
   errors.insert(errors.end(), actorLoadErrors.begin(), actorLoadErrors.end());
+  if (!this->dataPtr->actors.empty())
+  {
+    if (std::holds_alternative<std::monostate>(
+      this->dataPtr->modelLightOrActor))
+    {
+      this->dataPtr->modelLightOrActor = &this->dataPtr->actors.front();
+    }
+    else
+    {
+      sdfwarn << "Root object can only contain one of model, light or actor. "
+              << "This behavior was deprecated in libsdformat11 and will soon "
+              << "be removed";
+    }
+  }
 
   return errors;
 }
@@ -322,7 +368,6 @@ bool Root::WorldNameExists(const std::string &_name) const
   }
   return false;
 }
-
 /////////////////////////////////////////////////
 uint64_t Root::ModelCount() const
 {
@@ -350,6 +395,17 @@ bool Root::ModelNameExists(const std::string &_name) const
   return false;
 }
 
+/////////////////////////////////////////////////
+const Model *Root::Model() const
+{
+  if (std::holds_alternative<sdf::Model*>(this->dataPtr->modelLightOrActor))
+  {
+    return std::get<sdf::Model*>(this->dataPtr->modelLightOrActor);
+  }
+  return nullptr;
+}
+
+
 /////////////////////////////////////////////////
 uint64_t Root::LightCount() const
 {
@@ -377,6 +433,16 @@ bool Root::LightNameExists(const std::string &_name) const
   return false;
 }
 
+/////////////////////////////////////////////////
+const Light *Root::Light() const
+{
+  if (std::holds_alternative<sdf::Light*>(this->dataPtr->modelLightOrActor))
+  {
+    return std::get<sdf::Light*>(this->dataPtr->modelLightOrActor);
+  }
+  return nullptr;
+}
+
 /////////////////////////////////////////////////
 uint64_t Root::ActorCount() const
 {
@@ -404,6 +470,16 @@ bool Root::ActorNameExists(const std::string &_name) const
   return false;
 }
 
+/////////////////////////////////////////////////
+const Actor *Root::Actor() const
+{
+  if (std::holds_alternative<sdf::Actor*>(this->dataPtr->modelLightOrActor))
+  {
+    return std::get<sdf::Actor*>(this->dataPtr->modelLightOrActor);
+  }
+  return nullptr;
+}
+
 /////////////////////////////////////////////////
 sdf::ElementPtr Root::Element() const
 {