Review comments plus improvement in parent search

Signed-off-by: Adam Dąbrowski <[email protected]>

Gems/ROS2/Code/Source/Frame/NamespaceConfiguration.cpp

@@ -15,9 +15,9 @@
 
 namespace ROS2
 {
-    void NamespaceConfiguration::PopulateNamespace(bool isTop, const AZStd::string &entityName)
+    void NamespaceConfiguration::PopulateNamespace(bool isRoot, const AZStd::string &entityName)
     {
-        m_isTop = isTop;
+        m_isRoot = isRoot;
         m_entityName = entityName;
         OnNamespaceStrategySelected();
     }
@@ -30,11 +30,11 @@ namespace ROS2
             return;
         }
 
-        if (NamespaceStrategy::Empty == nss || (!m_isTop && NamespaceStrategy::Default == nss))
+        if (NamespaceStrategy::Empty == nss || (!m_isRoot && NamespaceStrategy::Default == nss))
         {
             m_namespace = "";
         }
-        else if (NamespaceStrategy::FromEntityName == nss || (m_isTop && NamespaceStrategy::Default == nss))
+        else if (NamespaceStrategy::FromEntityName == nss || (m_isRoot && NamespaceStrategy::Default == nss))
         {
             m_namespace = ROS2Names::RosifyName(m_entityName);
         }
@@ -88,6 +88,7 @@ namespace ROS2
                         ->EnumAttribute(NamespaceConfiguration::NamespaceStrategy::Custom, "Custom")
                     ->DataElement(AZ::Edit::UIHandlers::Default, &NamespaceConfiguration::m_namespace, "Namespace", "Namespace")
                         ->Attribute(AZ::Edit::Attributes::Visibility, &NamespaceConfiguration::IsNamespaceCustom)
+                        // TODO - hide for now, but desired Editor component behavior would be to show a read only value
                         ;
             }
         }

Gems/ROS2/Code/Source/Frame/NamespaceConfiguration.h

@@ -30,7 +30,7 @@ namespace ROS2
             Custom   // Non-empty but unrelated to entity name
         };
 
-        void PopulateNamespace(bool isTop, const AZStd::string &entityName);
+        void PopulateNamespace(bool isRoot, const AZStd::string &entityName);
         AZStd::string GetNamespace(const AZStd::string& parentNamespace) const;
 
         static void Reflect(AZ::ReflectContext* context);
@@ -38,7 +38,7 @@ namespace ROS2
     private:
         AZStd::string m_namespace;
         NamespaceStrategy m_namespaceStrategy;
-        bool m_isTop;
+        bool m_isRoot;
         AZStd::string m_entityName;
 
         bool IsNamespaceCustom();

Gems/ROS2/Code/Source/Frame/ROS2FrameComponent.cpp

@@ -67,11 +67,9 @@ namespace ROS2
 
     const ROS2FrameComponent* ROS2FrameComponent::GetParentROS2FrameComponent() const
     {
-        if (const AZ::EntityId parentEntityId = GetEntityTransformInterface()->GetParentId(); parentEntityId.IsValid())
-        {
-            // TODO - is there really no better way???
-            AZ::Entity* parentEntity = nullptr;
-            AZ::ComponentApplicationBus::BroadcastResult(parentEntity, &AZ::ComponentApplicationRequests::FindEntity, parentEntityId);
+        if (const AZ::TransformInterface* parentTI = GetEntityTransformInterface()->GetParent(); parentTI != nullptr)
+        {   // Does not use BroadCastResult as opposed to AZ::EntityUtils::FirstDerivedComponent
+            const AZ::Entity* parentEntity = azrtti_cast<const AzFramework::TransformComponent*>(parentTI)->GetEntity();
             return parentEntity->FindComponent<ROS2FrameComponent>();
         }
         return nullptr;

Gems/ROS2/Code/Source/Utilities/ROS2Names.cpp

@@ -25,11 +25,8 @@ namespace ROS2
     AZStd::string ROS2Names::RosifyName(const AZStd::string& input)
     {
         // TODO - add unit tests
-
         // TODO - implement stricter guidelines and differentiate: https://design.ros2.org/articles/topic_and_service_names.html
         // TODO - add check whether it begins with a number (and if so, prepend underscore)
-
-        AZStd::string rosified;
         const AZStd::regex ros2Disallowedlist("[^0-9|a-z|A-Z|_]");
         return AZStd::regex_replace(input, ros2Disallowedlist, "_");
     }