Allow special characters in AnimGraph node group names

Relying on the command system's string processing syntax prevents certain
names from being used. This converts the AnimGraphAdjustNodeGroup command
to be directly invokable, so that arguments can be passed directly, instead
of going through the CommandLine string parsing.

Signed-off-by: Chris Burel <burelc@amazon.com>

Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/AnimGraphNodeCommands.cpp

@@ -11,6 +11,7 @@
 #include "CommandManager.h"
 
 #include <EMotionFX/CommandSystem/Source/AnimGraphTriggerActionCommands.h>
+#include <EMotionFX/CommandSystem/Source/AnimGraphNodeGroupCommands.h>
 #include <EMotionFX/Source/ActorInstance.h>
 #include <EMotionFX/Source/ActorManager.h>
 #include <EMotionFX/Source/AnimGraph.h>
@@ -858,8 +859,16 @@ namespace CommandSystem
         // add it to the old node group if it was assigned to one before
         if (!mNodeGroupName.empty())
         {
-            commandString = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -nodeNames \"%s\" -nodeAction \"add\"", animGraph->GetID(), mNodeGroupName.c_str(), mName.c_str());
-            if (GetCommandManager()->ExecuteCommandInsideCommand(commandString.c_str(), outResult) == false)
+            auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+                GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+                /*animGraphId = */ animGraph->GetID(),
+                /*name = */ mNodeGroupName,
+                /*visible = */ AZStd::nullopt,
+                /*newName = */ AZStd::nullopt,
+                /*nodeNames = */ {{mName}},
+                /*nodeAction = */ CommandSystem::CommandAnimGraphAdjustNodeGroup::NodeAction::Add
+            );
+            if (GetCommandManager()->ExecuteCommandInsideCommand(command, outResult) == false)
             {
                 if (outResult.size() > 0)
                 {
@@ -1363,11 +1372,16 @@ namespace CommandSystem
         EMotionFX::AnimGraphNodeGroup* nodeGroup = node->GetAnimGraph()->FindNodeGroupForNode(node);
         if (nodeGroup && !cutMode)
         {
-            commandString = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %d -name \"%s\" -nodeNames \"%s\" -nodeAction \"add\"",
-                targetAnimGraph->GetID(),
-                nodeGroup->GetName(),
-                nodeName.c_str());
-            commandGroup->AddCommandString(commandString);
+            auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+                GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+                /*animGraphId = */ targetAnimGraph->GetID(),
+                /*name = */ nodeGroup->GetNameString(),
+                /*visible = */ AZStd::nullopt,
+                /*newName = */ AZStd::nullopt,
+                /*nodeNames = */ {{nodeName}},
+                /*nodeAction = */ CommandSystem::CommandAnimGraphAdjustNodeGroup::NodeAction::Add
+            );
+            commandGroup->AddCommand(command);
         }
 
         // Recurse through the child nodes.

Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/AnimGraphNodeGroupCommands.cpp

@@ -6,6 +6,7 @@
  *
  */
 
+#include <AzCore/std/optional.h>
 #include <AzFramework/StringFunc/StringFunc.h>
 #include "AnimGraphNodeGroupCommands.h"
 #include "AnimGraphConnectionCommands.h"
@@ -22,45 +23,46 @@
 
 namespace CommandSystem
 {
+    AZ_CLASS_ALLOCATOR_IMPL(CommandAnimGraphAdjustNodeGroup, EMotionFX::CommandAllocator, 0)
+
     //--------------------------------------------------------------------------------
     // CommandAnimGraphAdjustNodeGroup
     //--------------------------------------------------------------------------------
-    CommandAnimGraphAdjustNodeGroup::CommandAnimGraphAdjustNodeGroup(MCore::Command* orgCommand)
-        : MCore::Command("AnimGraphAdjustNodeGroup", orgCommand)
+    CommandAnimGraphAdjustNodeGroup::CommandAnimGraphAdjustNodeGroup(
+        MCore::Command* orgCommand,
+        AZ::u32 animGraphId,
+        AZStd::string name,
+        AZStd::optional<bool> visible,
+        AZStd::optional<AZStd::string> newName,
+        AZStd::optional<AZStd::vector<AZStd::string>> nodeNames,
+        AZStd::optional<NodeAction> nodeAction,
+        AZStd::optional<AZ::u32> color,
+        AZStd::optional<bool> updateUI
+    )
+        : MCore::Command(s_commandName, orgCommand)
+        , ParameterMixinAnimGraphId(animGraphId)
+        , m_name(AZStd::move(name))
+        , m_isVisible(visible)
+        , m_newName(AZStd::move(newName))
+        , m_nodeNames(AZStd::move(nodeNames))
+        , m_nodeAction(nodeAction)
+        , m_color(color)
+        , m_updateUI(updateUI)
     {
     }
 
-
-    CommandAnimGraphAdjustNodeGroup::~CommandAnimGraphAdjustNodeGroup()
+    AZStd::vector<AZStd::string> CommandAnimGraphAdjustNodeGroup::GenerateNodeNameVector(EMotionFX::AnimGraph* animGraph, const AZStd::vector<EMotionFX::AnimGraphNodeId>& nodeIDs)
     {
-    }
-
-
-    AZStd::string CommandAnimGraphAdjustNodeGroup::GenerateNodeNameString(EMotionFX::AnimGraph* animGraph, const AZStd::vector<EMotionFX::AnimGraphNodeId>& nodeIDs)
-    {
-        if (nodeIDs.empty())
+        AZStd::vector<AZStd::string> result;
+        for (const auto& nodeID : nodeIDs)
         {
-            return "";
-        }
-
-        AZStd::string result;
-
-        const size_t numNodes = nodeIDs.size();
-        for (size_t i = 0; i < numNodes; ++i)
-        {
-            EMotionFX::AnimGraphNode* animGraphNode = animGraph->RecursiveFindNodeById(nodeIDs[i]);
+            const EMotionFX::AnimGraphNode* animGraphNode = animGraph->RecursiveFindNodeById(nodeID);
             if (!animGraphNode)
             {
                 continue;
             }
-
-            result += animGraphNode->GetName();
-            if (i < numNodes - 1)
-            {
-                result += ';';
-            }
+            result.emplace_back(animGraphNode->GetName());
         }
-
         return result;
     }
 
@@ -80,78 +82,51 @@ namespace CommandSystem
     }
 
 
-    bool CommandAnimGraphAdjustNodeGroup::Execute(const MCore::CommandLine& parameters, AZStd::string& outResult)
+    bool CommandAnimGraphAdjustNodeGroup::Execute(const MCore::CommandLine&, AZStd::string& outResult)
     {
-        EMotionFX::AnimGraph* animGraph = CommandsGetAnimGraph(parameters, this, outResult);
+        EMotionFX::AnimGraph* animGraph = EMotionFX::GetAnimGraphManager().FindAnimGraphByID(m_animGraphId);
         if (!animGraph)
         {
             return false;
         }
 
-        // get the node group name
-        AZStd::string groupName;
-        parameters.GetValue("name", this, groupName);
-
         // find the node group index
-        const uint32 groupIndex = animGraph->FindNodeGroupIndexByName(groupName.c_str());
+        const uint32 groupIndex = animGraph->FindNodeGroupIndexByName(m_name.c_str());
         if (groupIndex == MCORE_INVALIDINDEX32)
         {
-            outResult = AZStd::string::format("Node group \"%s\" can not be found.", groupName.c_str());
+            outResult = AZStd::string::format("Node group \"%s\" can not be found.", m_name.c_str());
             return false;
         }
 
-        // get a pointer to the node group and keep the old name
         EMotionFX::AnimGraphNodeGroup* nodeGroup = animGraph->GetNodeGroup(groupIndex);
-        mOldName = nodeGroup->GetName();
 
-        // is visible?
-        if (parameters.CheckIfHasParameter("isVisible"))
+        if (m_isVisible.has_value())
         {
-            const bool isVisible = parameters.GetValueAsBool("isVisible", this);
-            mOldIsVisible = nodeGroup->GetIsVisible();
-            nodeGroup->SetIsVisible(isVisible);
+            m_oldIsVisible = nodeGroup->GetIsVisible();
+            nodeGroup->SetIsVisible(*m_isVisible);
         }
 
-        // background color
-        if (parameters.CheckIfHasParameter("color"))
+        if (m_color.has_value())
         {
-            const AZ::Vector4 colorVector4 = parameters.GetValueAsVector4("color", this);
-            const AZ::u32 color = AZ::Color(static_cast<float>(colorVector4.GetX()), static_cast<float>(colorVector4.GetY()), static_cast<float>(colorVector4.GetZ()), static_cast<float>(colorVector4.GetW())).ToU32();
-            mOldColor = nodeGroup->GetColor();
-            nodeGroup->SetColor(color);
+            m_oldColor = nodeGroup->GetColor();
+            nodeGroup->SetColor(*m_color);
         }
 
-        // set the new name
-        // if the new name is empty, the name is not changed
-        AZStd::string newGroupName;
-        parameters.GetValue("newName", this, newGroupName);
-        if (!newGroupName.empty())
+        if (m_newName.has_value())
         {
-            nodeGroup->SetName(newGroupName.c_str());
+            nodeGroup->SetName(m_newName->c_str());
         }
 
         // check if parametes nodeNames is set
-        if (parameters.CheckIfHasParameter("nodeNames"))
+        if (m_nodeNames.has_value())
         {
             // keep the old nodes IDs
-            mOldNodeIds = CollectNodeIdsFromGroup(nodeGroup);
-
-            // get the node action
-            AZStd::string nodeAction;
-            parameters.GetValue("nodeAction", this, nodeAction);
-
-            // get the node names and split the string
-            AZStd::string nodeNamesString;
-            parameters.GetValue("nodeNames", this, nodeNamesString);
-
-            
-            AZStd::vector<AZStd::string> nodeNames;
-            AzFramework::StringFunc::Tokenize(nodeNamesString.c_str(), nodeNames, ";", false, true);
+            m_oldNodeIds = CollectNodeIdsFromGroup(nodeGroup);
 
             // remove the selected nodes from the given node group
-            if (AzFramework::StringFunc::Equal(nodeAction.c_str(), "remove"))
+            if (*m_nodeAction == NodeAction::Remove)
             {
-                for (const AZStd::string& nodeName : nodeNames)
+                for (const AZStd::string& nodeName : *m_nodeNames)
                 {
                     EMotionFX::AnimGraphNode* animGraphNode = animGraph->RecursiveFindNodeByName(nodeName.c_str());
                     if (!animGraphNode)
@@ -163,9 +138,9 @@ namespace CommandSystem
                     nodeGroup->RemoveNodeById(animGraphNode->GetId());
                 }
             }
-            else if (AzFramework::StringFunc::Equal(nodeAction.c_str(), "add")) // add the selected nodes to the given node group
+            else if (*m_nodeAction == NodeAction::Add)
             {
-                for (const AZStd::string& nodeName : nodeNames)
+                for (const AZStd::string& nodeName : *m_nodeNames)
                 {
                     EMotionFX::AnimGraphNode* animGraphNode = animGraph->RecursiveFindNodeByName(nodeName.c_str());
                     if (!animGraphNode)
@@ -184,12 +159,12 @@ namespace CommandSystem
                     nodeGroup->AddNode(animGraphNode->GetId());
                 }
             }
-            else if (AzFramework::StringFunc::Equal(nodeAction.c_str(), "replace")) // clear the node group and then add the selected nodes to the given node group
+            else if (*m_nodeAction == NodeAction::Replace)
             {
                 // clear the node group upfront
                 nodeGroup->RemoveAllNodes();
 
-                for (const AZStd::string& nodeName : nodeNames)
+                for (const AZStd::string& nodeName : *m_nodeNames)
                 {
                     EMotionFX::AnimGraphNode* animGraphNode = animGraph->RecursiveFindNodeByName(nodeName.c_str());
                     if (!animGraphNode)
@@ -211,68 +186,40 @@ namespace CommandSystem
         }
 
         // save the current dirty flag and tell the anim graph that something got changed
-        mOldDirtyFlag = animGraph->GetDirtyFlag();
+        m_oldDirtyFlag = animGraph->GetDirtyFlag();
         animGraph->SetDirtyFlag(true);
         return true;
     }
 
 
     // undo the command
-    bool CommandAnimGraphAdjustNodeGroup::Undo(const MCore::CommandLine& parameters, AZStd::string& outResult)
+    bool CommandAnimGraphAdjustNodeGroup::Undo(const MCore::CommandLine&, AZStd::string& outResult)
     {
-        EMotionFX::AnimGraph* animGraph = CommandsGetAnimGraph(parameters, this, outResult);
+        EMotionFX::AnimGraph* animGraph = EMotionFX::GetAnimGraphManager().FindAnimGraphByID(m_animGraphId);
         if (!animGraph)
         {
             return false;
         }
 
-        AZStd::string commandString = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i", animGraph->GetID());
-
-        // set the old name or simply set the name if the name is not changed
-        if (parameters.CheckIfHasParameter("newName"))
-        {
-            AZStd::string newName;
-            parameters.GetValue("newName", this, newName);
-
-            commandString += AZStd::string::format(" -name \"%s\"", newName.c_str());
-            commandString += AZStd::string::format(" -newName \"%s\"", mOldName.c_str());
-        }
-        else
-        {
-            commandString += AZStd::string::format(" -name \"%s\"", mOldName.c_str());
-        }
-
-        // set the old visible flag
-        if (parameters.CheckIfHasParameter("isVisible"))
-        {
-            commandString += AZStd::string::format(" -isVisible %i", mOldIsVisible);
-        }
-
-        // set the old color
-        if (parameters.CheckIfHasParameter("color"))
-        {
-            AZ::Color oldColor;
-            oldColor.FromU32(mOldColor);
-            const AZStd::string oldColorString = AZStd::string::format("%.8f,%.8f,%.8f,%.8f", static_cast<float>(oldColor.GetR()), static_cast<float>(oldColor.GetG()), static_cast<float>(oldColor.GetB()), static_cast<float>(oldColor.GetA()));
-
-            commandString += AZStd::string::format(" -color \"%s\"", oldColorString.c_str());
-        }
-
-        // set the old nodes
-        if (parameters.CheckIfHasParameter("nodeNames"))
-        {
-            const AZStd::string nodeNamesString = CommandAnimGraphAdjustNodeGroup::GenerateNodeNameString(animGraph, mOldNodeIds);
-            commandString += AZStd::string::format(" -nodeNames \"%s\" -nodeAction \"replace\"", nodeNamesString.c_str());
-        }
+        CommandAnimGraphAdjustNodeGroup* command = aznew CommandAnimGraphAdjustNodeGroup(
+            GetCommandManager()->FindCommand(CommandAnimGraphAdjustNodeGroup::s_commandName),
+            /*animGraphId = */ m_animGraphId,
+            /*name = */ m_newName.has_value() ? *m_newName : m_name,
+            /*visible = */ m_isVisible.has_value() ? AZStd::optional<bool>(m_oldIsVisible) : AZStd::nullopt,
+            /*newName = */ m_newName.has_value() ? AZStd::optional<AZStd::string>(m_name) : AZStd::nullopt,
+            /*nodeNames = */ m_nodeNames.has_value() ? AZStd::optional<AZStd::vector<AZStd::string>>(GenerateNodeNameVector(animGraph, m_oldNodeIds)) : AZStd::nullopt,
+            /*nodeAction = */ m_nodeNames.has_value() ? AZStd::optional<NodeAction>(NodeAction::Replace) : AZStd::nullopt,
+            /*color = */ m_color.has_value() ? AZStd::optional<AZ::u32>(m_oldColor) : AZStd::nullopt
+        );
 
         // execute the command
-        if (!GetCommandManager()->ExecuteCommandInsideCommand(commandString, outResult))
+        if (!GetCommandManager()->ExecuteCommandInsideCommand(command, outResult))
         {
             AZ_Error("EMotionFX", false, outResult.c_str());
         }
 
         // set the dirty flag back to the old value
-        animGraph->SetDirtyFlag(mOldDirtyFlag);
+        animGraph->SetDirtyFlag(m_oldDirtyFlag);
 
         return true;
     }
@@ -282,7 +229,7 @@ namespace CommandSystem
     {
         GetSyntax().ReserveParameters(8);
         GetSyntax().AddRequiredParameter("name", "The name of the node group to adjust.", MCore::CommandSyntax::PARAMTYPE_STRING);
-        GetSyntax().AddParameter("animGraphID", "The id of the blend set the node group belongs to.", MCore::CommandSyntax::PARAMTYPE_INT, "-1");
+        EMotionFX::ParameterMixinAnimGraphId::InitSyntax(GetSyntax(), /*isParameterRequired=*/ false);
         GetSyntax().AddParameter("isVisible", "The visibility flag of the node group.", MCore::CommandSyntax::PARAMTYPE_BOOLEAN, "true");
         GetSyntax().AddParameter("newName", "The new name of the node group.", MCore::CommandSyntax::PARAMTYPE_STRING, "");
         GetSyntax().AddParameter("nodeNames", "A list of node names that should be added/removed to/from the node group.", MCore::CommandSyntax::PARAMTYPE_STRING, "");
@@ -291,6 +238,51 @@ namespace CommandSystem
         GetSyntax().AddParameter("updateUI", "Setting this to true will trigger a refresh of the node groups UI.", MCore::CommandSyntax::PARAMTYPE_BOOLEAN, "true");
     }
 
+    bool CommandAnimGraphAdjustNodeGroup::SetCommandParameters(const MCore::CommandLine& parameters)
+    {
+        EMotionFX::ParameterMixinAnimGraphId::SetCommandParameters(parameters);
+        m_name = parameters.GetValue("name", this);
+
+        if (parameters.CheckIfHasParameter("isVisible"))
+        {
+            m_isVisible = parameters.GetValueAsBool("isVisible", this);
+        }
+        if (parameters.CheckIfHasParameter("newName"))
+        {
+            m_newName = parameters.GetValue("newName", this);
+        }
+        if (parameters.CheckIfHasParameter("nodeNames"))
+        {
+            m_nodeNames.emplace();
+            AzFramework::StringFunc::Tokenize(parameters.GetValue("nodeNames", this), m_nodeNames.value(), ";", false, true);
+        }
+        if (parameters.CheckIfHasValue("nodeAction"))
+        {
+            const AZStd::string& nodeActionStr = parameters.GetValue("nodeAction", this);
+            if (nodeActionStr == "add")
+            {
+                m_nodeAction = NodeAction::Add;
+            }
+            else if (nodeActionStr == "remove")
+            {
+                m_nodeAction = NodeAction::Remove;
+            }
+            else if (nodeActionStr == "replace")
+            {
+                m_nodeAction = NodeAction::Replace;
+            }
+        }
+        if (parameters.CheckIfHasParameter("color"))
+        {
+            m_color = AZ::Color(parameters.GetValueAsVector4("color", this)).ToU32();
+        }
+        if (parameters.CheckIfHasParameter("updateUI"))
+        {
+            m_updateUI = parameters.GetValueAsBool("updateUI", this);
+        }
+
+        return true;
+    }
 
     const char* CommandAnimGraphAdjustNodeGroup::GetDescription() const
     {
@@ -447,21 +439,20 @@ namespace CommandSystem
         
         MCore::CommandGroup commandGroup;
 
-        AZStd::string commandString = AZStd::string::format("AnimGraphAddNodeGroup -animGraphID %i -name \"%s\" -updateUI %s",animGraph->GetID(), mOldName.c_str(), updateWindow.c_str());
-        commandGroup.AddCommandString(commandString);
+        commandGroup.AddCommandString(AZStd::string::format("AnimGraphAddNodeGroup -animGraphID %i -name \"%s\" -updateUI %s",animGraph->GetID(), mOldName.c_str(), updateWindow.c_str()));
 
-        const AZStd::string nodeNamesString = CommandAnimGraphAdjustNodeGroup::GenerateNodeNameString(animGraph, mOldNodeIds);
+        auto* command = aznew CommandAnimGraphAdjustNodeGroup(
+            GetCommandManager()->FindCommand(CommandAnimGraphAdjustNodeGroup::s_commandName),
+            /*animGraphId = */ animGraph->GetID(),
+            /*name = */ mOldName,
+            /*visible = */ mOldIsVisible,
+            /*newName = */ AZStd::nullopt,
+            /*nodeNames = */ CommandAnimGraphAdjustNodeGroup::GenerateNodeNameVector(animGraph, mOldNodeIds),
+            /*nodeAction = */ CommandAnimGraphAdjustNodeGroup::NodeAction::Add,
+            /*color = */ mOldColor
+        );
 
-        AZ::Color oldColor;
-        oldColor.FromU32(mOldColor);
-        const AZStd::string oldColorString = AZStd::string::format("%.8f,%.8f,%.8f,%.8f",
-            static_cast<float>(oldColor.GetR()), static_cast<float>(oldColor.GetG()), static_cast<float>(oldColor.GetB()), static_cast<float>(oldColor.GetA()));
-
-        commandString = AZStd::string::format(
-            "AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -isVisible %s -color \"%s\" -nodeNames \"%s\" -nodeAction \"add\" -updateUI %s",
-            animGraph->GetID(), mOldName.c_str(), AZStd::to_string(mOldIsVisible).c_str(), oldColorString.c_str(), nodeNamesString.c_str(), updateWindow.c_str());
-
-        commandGroup.AddCommandString(commandString);
+        commandGroup.AddCommand(command);
 
         AZStd::string result;
         if (!GetCommandManager()->ExecuteCommandGroupInsideCommand(commandGroup, result))

Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/AnimGraphNodeGroupCommands.h

@@ -13,22 +13,73 @@
 #include <MCore/Source/CommandGroup.h>
 #include <EMotionFX/Source/AnimGraphNodeGroup.h>
 #include <EMotionFX/Source/AnimGraph.h>
+#include <EMotionFX/CommandSystem/Source/ParameterMixins.h>
 
 namespace CommandSystem
 {
     // adjust a node group
-    MCORE_DEFINECOMMAND_START(CommandAnimGraphAdjustNodeGroup, "Adjust anim graph node group", true)
-public:
-    static AZStd::string GenerateNodeNameString(EMotionFX::AnimGraph* animGraph, const AZStd::vector<EMotionFX::AnimGraphNodeId>& nodeIDs);
-    static AZStd::vector<EMotionFX::AnimGraphNodeId> CollectNodeIdsFromGroup(EMotionFX::AnimGraphNodeGroup* nodeGroup);
+    class CommandAnimGraphAdjustNodeGroup
+        : public MCore::Command
+        , public EMotionFX::ParameterMixinAnimGraphId
+    {
+    public:
+        AZ_CLASS_ALLOCATOR_DECL
 
-    AZStd::string                               mOldName;
-    bool                                        mOldIsVisible;
-    AZ::u32                                     mOldColor;
-    AZStd::vector<EMotionFX::AnimGraphNodeId>   mOldNodeIds;
-    bool                                        mOldDirtyFlag;
-    MCORE_DEFINECOMMAND_END
+        static constexpr inline AZStd::string_view s_commandName = "AnimGraphAdjustNodeGroup";
 
+        enum class NodeAction
+        {
+            Add,
+            Remove,
+            Replace
+        };
+
+        explicit CommandAnimGraphAdjustNodeGroup(
+            MCore::Command* orgCommand = nullptr,
+            AZ::u32 animGraphId = MCORE_INVALIDINDEX32,
+            AZStd::string name = AZStd::string{},
+            AZStd::optional<bool> visible = AZStd::nullopt,
+            AZStd::optional<AZStd::string> newName = AZStd::nullopt,
+            AZStd::optional<AZStd::vector<AZStd::string>> nodeNames = AZStd::nullopt,
+            AZStd::optional<NodeAction> nodeAction = AZStd::nullopt,
+            AZStd::optional<AZ::u32> color = AZStd::nullopt,
+            AZStd::optional<bool> updateUI = AZStd::nullopt
+        );
+        bool Execute(const MCore::CommandLine& parameters, AZStd::string& outResult) override;
+        bool Undo(const MCore::CommandLine& parameters, AZStd::string& outResult) override;
+        void InitSyntax() override;
+        bool SetCommandParameters(const MCore::CommandLine& parameters) override;
+        bool GetIsUndoable() const override
+        {
+            return true;
+        }
+        const char* GetHistoryName() const override
+        {
+            return "Adjust anim graph node group";
+        }
+        const char* GetDescription() const override;
+        MCore::Command* Create() override
+        {
+            return new CommandAnimGraphAdjustNodeGroup(this);
+        }
+
+        static AZStd::vector<AZStd::string> GenerateNodeNameVector(EMotionFX::AnimGraph* animGraph, const AZStd::vector<EMotionFX::AnimGraphNodeId>& nodeIDs);
+        static AZStd::vector<EMotionFX::AnimGraphNodeId> CollectNodeIdsFromGroup(EMotionFX::AnimGraphNodeGroup* nodeGroup);
+
+    private:
+        AZStd::string m_name;
+        AZStd::optional<bool> m_isVisible;
+        AZStd::optional<AZStd::string> m_newName;
+        AZStd::optional<AZStd::vector<AZStd::string>> m_nodeNames;
+        AZStd::optional<NodeAction> m_nodeAction;
+        AZStd::optional<AZ::u32> m_color;
+        AZStd::optional<bool> m_updateUI;
+
+        bool m_oldIsVisible;
+        AZ::u32 m_oldColor;
+        AZStd::vector<EMotionFX::AnimGraphNodeId> m_oldNodeIds;
+        bool m_oldDirtyFlag;
+    };
 
     // add node group
     MCORE_DEFINECOMMAND_START(CommandAnimGraphAddNodeGroup, "Add anim graph node group", true)

Gems/EMotionFX/Code/EMotionFX/CommandSystem/Source/ParameterMixins.h

@@ -80,6 +80,8 @@ namespace EMotionFX
         AZ_RTTI(ParameterMixinAnimGraphId, "{3F48199E-6566-471F-A7EA-ADF67CAC4DCD}")
         AZ_CLASS_ALLOCATOR_DECL
 
+        ParameterMixinAnimGraphId() = default;
+        ParameterMixinAnimGraphId(AZ::u32 id) : m_animGraphId(id) {}
         virtual ~ParameterMixinAnimGraphId() = default;
 
         static void Reflect(AZ::ReflectContext* context);

Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/AnimGraph/BlendGraphWidget.cpp

@@ -9,6 +9,7 @@
 #include <AzQtComponents/Utilities/Conversions.h>
 #include <EMotionFX/CommandSystem/Source/AnimGraphConnectionCommands.h>
 #include <EMotionFX/CommandSystem/Source/AnimGraphNodeCommands.h>
+#include <EMotionFX/CommandSystem/Source/AnimGraphNodeGroupCommands.h>
 #include <EMotionFX/CommandSystem/Source/MotionSetCommands.h>
 #include <EMotionFX/Source/AnimGraphExitNode.h>
 #include <EMotionFX/Source/AnimGraphMotionNode.h>
@@ -1208,7 +1209,7 @@ namespace EMStudio
 
         MCore::CommandGroup commandGroup("Adjust anim graph node group");
 
-        AZStd::string nodeNames;
+        AZStd::vector<AZStd::string> nodeNames;
         for (const QModelIndex& selectedIndex : selectionList)
         {
             // Skip transitions and blend tree connections.
@@ -1221,12 +1222,19 @@ namespace EMStudio
             EMotionFX::AnimGraphNodeGroup* nodeGroup = animGraph->FindNodeGroupForNode(selectedNode);
             if (nodeGroup)
             {
-                const AZStd::string command = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -nodeNames \"%s\" -nodeAction \"remove\"", animGraph->GetID(), nodeGroup->GetName(), selectedNode->GetName());
-                commandGroup.AddCommandString(command);
+                auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+                    GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+                    /*animGraphId = */ animGraph->GetID(),
+                    /*name = */ nodeGroup->GetNameString(),
+                    /*visible = */ AZStd::nullopt,
+                    /*newName = */ AZStd::nullopt,
+                    /*nodeNames = */ {{selectedNode->GetNameString()}},
+                    /*nodeAction = */ CommandSystem::CommandAnimGraphAdjustNodeGroup::NodeAction::Remove
+                );
+                commandGroup.AddCommand(command);
             }
 
-            nodeNames += selectedNode->GetName();
-            nodeNames += ";";
+            nodeNames.emplace_back(selectedNode->GetName());
         }
         if (!nodeNames.empty())
         {
@@ -1235,8 +1243,16 @@ namespace EMStudio
 
         if (newNodeGroup)
         {
-            const AZStd::string command = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -nodeNames \"%s\" -nodeAction \"add\"", animGraph->GetID(), newNodeGroup->GetName(), nodeNames.c_str());
-            commandGroup.AddCommandString(command);
+            auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+                GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+                /*animGraphId = */ animGraph->GetID(),
+                /*name = */ newNodeGroup->GetNameString(),
+                /*visible = */ AZStd::nullopt,
+                /*newName = */ AZStd::nullopt,
+                /*nodeNames = */ nodeNames,
+                /*nodeAction = */ CommandSystem::CommandAnimGraphAdjustNodeGroup::NodeAction::Add
+            );
+            commandGroup.AddCommand(command);
         }
 
         AZStd::string outResult;

Gems/EMotionFX/Code/EMotionFX/Tools/EMotionStudio/Plugins/StandardPlugins/Source/AnimGraph/NodeGroupWindow.cpp

@@ -74,11 +74,6 @@ namespace EMStudio
         mLineEdit->setText(nodeGroup.c_str());
         mLineEdit->selectAll();
 
-        // create add the error message
-        /*mErrorMsg = new QLabel("<font color='red'>Error: Duplicate name found</font>");
-        mErrorMsg->setAlignment(Qt::AlignVCenter | Qt::AlignLeft);
-        mErrorMsg->setVisible(false);*/
-
         // create the button layout
         QHBoxLayout* buttonLayout   = new QHBoxLayout();
         mOKButton                   = new QPushButton("OK");
@@ -139,10 +134,16 @@ namespace EMStudio
     void NodeGroupRenameWindow::Accepted()
     {
         // Execute the command
-        AZStd::string commandString, outResult;
+        AZStd::string outResult;
         const AZStd::string convertedNewName = FromQtString(mLineEdit->text());
-        commandString = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -newName \"%s\"", mAnimGraph->GetID(), mNodeGroup.c_str(), convertedNewName.c_str());
-        if (GetCommandManager()->ExecuteCommand(commandString.c_str(), outResult) == false)
+        auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+            GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+            mAnimGraph->GetID(),
+            /*name = */ mNodeGroup,
+            /*visible = */ AZStd::nullopt,
+            /*newName = */ convertedNewName
+        );
+        if (!GetCommandManager()->ExecuteCommand(command, outResult))
         {
             MCore::LogError(outResult.c_str());
         }
@@ -167,7 +168,7 @@ namespace EMStudio
         mAdjustCallback     = new CommandAnimGraphAdjustNodeGroupCallback(false);
         GetCommandManager()->RegisterCommandCallback("AnimGraphAddNodeGroup", mCreateCallback);
         GetCommandManager()->RegisterCommandCallback("AnimGraphRemoveNodeGroup", mRemoveCallback);
-        GetCommandManager()->RegisterCommandCallback("AnimGraphAdjustNodeGroup", mAdjustCallback);
+        GetCommandManager()->RegisterCommandCallback(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName.data(), mAdjustCallback);
 
         // add the add button
         mAddAction = new QAction(MysticQt::GetMysticQt()->FindIcon("Images/Icons/Plus.svg"), tr("Add new node group"), this);
@@ -486,13 +487,16 @@ namespace EMStudio
 
         bool isVisible = state == Qt::Checked;
 
-        // construct the command
-        AZStd::string commandString;
-        commandString = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -isVisible %s", animGraph->GetID(), nodeGroup->GetName(), AZStd::to_string(isVisible).c_str());
+        auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+            GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+            /*animGraphId = */ animGraph->GetID(),
+            /*name = */ nodeGroup->GetNameString(),
+            /*visible = */ isVisible
+        );
 
         // execute the command
         AZStd::string resultString;
-        if (GetCommandManager()->ExecuteCommand(commandString.c_str(), resultString) == false)
+        if (GetCommandManager()->ExecuteCommand(command, resultString) == false)
         {
             if (resultString.size() > 0)
             {
@@ -519,16 +523,21 @@ namespace EMStudio
         // get a pointer to the node group
         EMotionFX::AnimGraphNodeGroup* nodeGroup = animGraph->GetNodeGroup(groupIndex);
 
-        // get the color
-        AZ::Vector4 finalColor = color.GetAsVector4();
-
         // construct the command
-        AZStd::string commandString;
-        commandString = AZStd::string::format("AnimGraphAdjustNodeGroup -animGraphID %i -name \"%s\" -color \"%s\"", animGraph->GetID(), nodeGroup->GetName(), AZStd::to_string(finalColor).c_str());
+        auto* command = aznew CommandSystem::CommandAnimGraphAdjustNodeGroup(
+            GetCommandManager()->FindCommand(CommandSystem::CommandAnimGraphAdjustNodeGroup::s_commandName),
+            /*animGraphId = */ animGraph->GetID(),
+            /*name = */ nodeGroup->GetName(),
+            /*visible = */ AZStd::nullopt,
+            /*newName = */ AZStd::nullopt,
+            /*nodeNames = */ AZStd::nullopt,
+            /*nodeAction = */ AZStd::nullopt,
+            /*color = */ color.ToU32()
+        );
 
         // execute the command
         AZStd::string resultString;
-        if (GetCommandManager()->ExecuteCommand(commandString.c_str(), resultString) == false)
+        if (GetCommandManager()->ExecuteCommand(command, resultString) == false)
         {
             if (resultString.size() > 0)
             {

Gems/EMotionFX/Code/MCore/Source/Command.cpp

@@ -28,10 +28,10 @@ namespace MCore
 
 
     // constructor
-    Command::Command(const char* commandName, Command* originalCommand)
+    Command::Command(AZStd::string commandName, Command* originalCommand)
+        : mOrgCommand(originalCommand)
+        , mCommandName(AZStd::move(commandName))
     {
-        mCommandName = commandName;
-        mOrgCommand  = originalCommand;
     }
 
 

Gems/EMotionFX/Code/MCore/Source/Command.h

@@ -185,7 +185,7 @@ namespace MCore
          * @param commandName The unique identifier for the command.
          * @param originalCommand The original command, or nullptr when this is the original command.
          */
-        Command(const char* commandName, Command* originalCommand);
+        Command(AZStd::string commandName, Command* originalCommand);
 
         /**
          * Destructor.