Delete Slot() class, move add_connection_by_slot_id() and are_slots_connected() to Graph() class, Add a get_slots() call to the Node() class that maps all slots on a given node to a dict with a key corresponding to the slot name and the value corresponding to the graph.GraphModelSlotId(slot_name) for that slot name. Also removed __init__ constructors for Graph() class and it only requires a document_id to initialize. Old functions re-added as part of Graph() class methods.

Signed-off-by: NULL <[email protected]>

AutomatedTesting/Gem/PythonTests/Atom/atom_utils/atom_constants.py

@@ -1585,9 +1585,26 @@ class GraphControllerRequestBusEvents(object):
     """
     ADD_NODE = "AddNode"
     REMOVE_NODE = "RemoveNode"
+    GET_POSITION = "GetPosition"
     WRAP_NODE = "WrapNode"
+    WRAP_NODE_ORDERED = "WrapNodeOrdered"
+    UNWRAP_NODE = "UnwrapNode"
+    IS_NODE_WRAPPED = "IsNodeWrapped"
+    SET_WRAPPER_NODE_ACTION_STRING = "SetWrapperNodeActionString"
     ADD_CONNECTION = "AddConnection"
     ADD_CONNECTION_BY_SLOT_ID = "AddConnectionBySlotId"
     ARE_SLOTS_CONNECTED = "AreSlotsConnected"
     REMOVE_CONNECTION = "RemoveConnection"
     EXTEND_SLOT = "ExtendSlot"
+    GET_NODE_BY_ID = "GetNodeById"
+    GET_NODES_FROM_GRAPH_NODE_IDS = "GetNodesFromGraphNodeIds"
+    GET_NODE_IDS_BY_NODE = "GetNodeIdByNode"
+    GET_SLOT_ID_BY_SLOT = "GetSlotIdBySlot"
+    GET_NODES = "GetNodes"
+    GET_SELECTED_NODES = "GetSelectedNodes"
+    SET_SELECTED = "SetSelected"
+    CLEAR_SELECTION = "ClearSelection"
+    ENABLE_NODE = "EnableNode"
+    DISABLE_NODE = "DisableNode"
+    CENTER_ON_NODES = "CenterOnNodes"
+    GET_MAJOR_PITCH = "GetMajorPitch"

AutomatedTesting/Gem/PythonTests/Atom/atom_utils/material_canvas_utils.py

@@ -11,106 +11,139 @@ import azlmbr.math as math
 import azlmbr.object
 import azlmbr.bus as bus
 
-import Atom.atom_utils.atom_tools_utils as atom_tools_utils
 from Atom.atom_utils.atom_constants import (
     DynamicNodeManagerRequestBusEvents, GraphControllerRequestBusEvents, GraphDocumentRequestBusEvents)
 
 
-class Graph(object):
+class Node(object):
     """
-    Opens the specified materialgraph file when inside MaterialCanvas which appears as a Graph.
-    Opening the Graph is required since it also sets the document_id for use with other functions.
+    Represents a node inside of a graph and contains node related functions.
     """
 
-    def __init__(self, material_graph_file_path: str):
-        self.material_graph_file_path = material_graph_file_path
-        self.document_id = atom_tools_utils.open_document(self.material_graph_file_path)
-        self.graph_id = atomtools.GraphDocumentRequestBus(
-            bus.Event, GraphDocumentRequestBusEvents.GET_GRAPH_ID, self.document_id)
-        self.name = atomtools.GraphDocumentRequestBus(
-            bus.Event, GraphDocumentRequestBusEvents.GET_GRAPH_NAME, self.document_id)
-        self.graph_object = atomtools.GraphDocumentRequestBus(
-            bus.Event, GraphDocumentRequestBusEvents.GET_GRAPH, self.document_id)
-        self.nodes = []
-
-    def _create_node_by_name(self, node_name: str) -> object:
+    def __init__(self, node_python_proxy: azlmbr.object.PythonProxyObject, node_id: azlmbr.object.PythonProxyObject):
         """
-        Creates a new node in memory matching the node_name string on the specified graph object.
-        i.e. "World Position" for node_name would create a World Position node.
-        :param node_name: String representing the type of node to add to the graph.
-        :return: azlmbr.object.PythonProxyObject representing a C++ AZStd::shared_ptr<Node> object.
+        :param node_python_proxy: azlmbr.object.PythonProxyObject representing a node object.
+        :param node_id: azlmbr.object.PythonProxyObject containing an int value for the node.
         """
-        return atomtools.DynamicNodeManagerRequestBus(
-            bus.Broadcast, DynamicNodeManagerRequestBusEvents.CREATE_NODE_BY_NAME, self.graph_object, node_name)
+        self.node_python_proxy = node_python_proxy
+        self.node_id = node_id
 
-    def add_node(self, node_name: str, position: math.Vector2) -> None:
+    def get_slots(self) -> dict:
         """
-        Creates a node in memory using _create_node_by_name then adds it to this Graph object.
-        :param node_name: String representing the type of node to add to the graph.
-        :param position: math.Vector2(x,y) value that determines where to place the node on the Graph's grid.
-        :return: None, but appends the new node to self.nodes list.
+        Returns a dictionary mapping of all slots on this Node.
+        :return: a dict containing slot name keys with graph.GraphModelSlotId slot values.
         """
-        node_object = self._create_node_by_name(node_name)
-        node_id = graph.GraphControllerRequestBus(
-            bus.Event, GraphControllerRequestBusEvents.ADD_NODE, self.graph_id, node_object, position)
-        if not node_id:
-            return  # If the node isn't valid, don't add it.
-        self.nodes.append(Node(node_object, node_id, node_name, self.graph_id, position))
+        mapped_node_slots = {}
+        raw_node_slots_dict = self.node_python_proxy.invoke('GetSlots')
+
+        for k, v in raw_node_slots_dict.items():
+            mapped_node_slots[k.name] = graph.GraphModelSlotId(k.name)
+
+        return mapped_node_slots
 
 
-class Slot(object):
+class Graph(object):
     """
-    Represents a slot that is part of a Node (required).
+    Binds this Graph object to the opened graph matching the document_id passed to construct this object.
     """
 
-    def __init__(self, slot_name):
-        self.slot_name = slot_name
-        self.id = graph.GraphModelSlotId(slot_name)
-        if not self.id:
-            self.id = None
+    def __init__(self, document_id: str):
+        self.document_id = document_id
 
+    def get_graph_id(self) -> azlmbr.object.PythonProxyObject:
+        """
+        Returns the graph ID value for this Graph object.
+        :return: azlmbr.object.PythonProxyObject containing an int value that represents the graph ID.
+        """
+        return atomtools.GraphDocumentRequestBus(
+            bus.Event, GraphDocumentRequestBusEvents.GET_GRAPH_ID, self.document_id)
 
-class Node(object):
-    """
-    Represents a node inside of a Graph (required).
-    """
+    def get_graph_name(self) -> str:
+        """
+        Returns the graph name for this Graph object.
+        :return: string representing the name of this Graph object.
+        """
+        return atomtools.GraphDocumentRequestBus(
+            bus.Event, GraphDocumentRequestBusEvents.GET_GRAPH_NAME, self.document_id)
 
-    def __init__(self, node_object: object, node_id: int, node_name: str, graph_id: math.Uuid, position: math.Vector2):
-        self.node_object = node_object
-        self.node_id = node_id
-        self.node_name = node_name
-        self.graph_id = graph_id
-        self.position = position
-        self.slots = []
+    def get_graph(self) -> azlmbr.object.PythonProxyObject:
+        """
+        Returns the AZStd::shared_ptr<Graph> object for this Graph object.
+        :return: azlmbr.object.PythonProxyObject containing a AZStd::shared_ptr<Graph> object.
+        """
+        return atomtools.GraphDocumentRequestBus(bus.Event, GraphDocumentRequestBusEvents.GET_GRAPH, self.document_id)
 
-    def add_slot(self, slot_name: str) -> None:
+    def get_nodes(self, graph_id: azlmbr.object.PythonProxyObject) -> list[azlmbr.object.PythonProxyObject]:
         """
-        Adds a new Slot object to this Node to identify one of its existing slots.
-        :param slot_name: Name of the slot to lookup using graph.GraphModelSlotId().
-        :return: None, but adds the new slot to the self.slots list.
+        Gets the current Nodes in this Graph and returns them as a list of azlmbr.object.PythonProxyObject objects.
+        :param graph_id: azlmbr.object.PythonProxyObject containing an int value for the graph.
+        :return: list of azlmbr.object.PythonProxyObject objects each representing a Node in this Graph.
         """
-        self.slots.append(Slot(slot_name))
+        nodes = graph.GraphControllerRequestBus(bus.Event, GraphControllerRequestBusEvents.GET_NODES, graph_id)
+        return nodes
 
-    def connect_slots(self, source_slot: Slot, target_node: (), target_slot: Slot) -> azlmbr.object.PythonProxyObject:
+    def create_node_by_name(
+            self, graph: azlmbr.object.PythonProxyObject, node_name: str) -> azlmbr.object.PythonProxyObject:
+        """
+        Creates a new node in memory matching the node_name string on the specified graph object.
+        i.e. "World Position" for node_name would create a World Position node.
+        We create them in memory for use with the GraphControllerRequestBusEvents.ADD_NODE bus call on this Graph.
+        :param graph: azlmbr.object.PythonProxyObject containing an AZStd::shared_ptr<Graph> object.
+        :param node_name: String representing the type of node to add to the graph.
+        :return: azlmbr.object.PythonProxyObject representing a C++ AZStd::shared_ptr<Node> object.
+        """
+        return atomtools.DynamicNodeManagerRequestBus(
+            bus.Broadcast, DynamicNodeManagerRequestBusEvents.CREATE_NODE_BY_NAME, graph, node_name)
+
+    def add_node(self,
+                 graph_id: azlmbr.object.PythonProxyObject,
+                 node: azlmbr.object.PythonProxyObject,
+                 position: math.Vector2) -> Node:
+        """
+        Adds a node generated from the NodeManager to this Graph, then returns a Node object for that node.
+        :param graph_id: azlmbr.object.PythonProxyObject containing an int value for the graph.
+        :param node: A azlmbr.object.PythonProxyObject containing the node we wish to add to the graph.
+        :param position: math.Vector2(x,y) value that determines where to place the node on the Graph's grid.
+        :return: A Node object containing node object and node id via azlmbr.object.PythonProxyObject objects.
+        """
+        node_id = graph.GraphControllerRequestBus(
+            bus.Event, GraphControllerRequestBusEvents.ADD_NODE, graph_id, node, position)
+        return Node(node, node_id)
+
+    def add_connection_by_slot_id(self,
+                                  graph_id: azlmbr.object.PythonProxyObject,
+                                  source_node: Node,
+                                  source_slot: graph.GraphModelSlotId,
+                                  target_node: Node,
+                                  target_slot: graph.GraphModelSlotId) -> azlmbr.object.PythonProxyObject:
         """
         Connects a starting source_slot to a destination target_slot.
-        :param source_slot: A Slot() object for the source_node in a given Node() object.
-        :param target_node: A Node() object for the target node that holds the target_slot we are connecting to.
-        :param target_slot: A Slot() object for the slot we are connecting to from the source_node.
+        :param graph_id: azlmbr.object.PythonProxyObject object containing an int for this Graph object.
+        :param source_node: A Node() object for the source node we are connecting from.
+        :param source_slot: A graph.GraphModelSlotId representing a slot on the source_node.
+        :param target_node: A Node() object for the target node we are connecting to.
+        :param target_slot: A graph.GraphModelSlotId representing a slot on the target_node.
         :return: azlmbr.object.PythonProxyObject representing a C++ AZStd::shared_ptr<Connection> object.
         """
         return graph.GraphControllerRequestBus(
-            bus.Event, GraphControllerRequestBusEvents.ADD_CONNECTION_BY_SLOT_ID, self.graph_id,
-            self.node_object, source_slot.id, target_node.node_object, target_slot.id)
-
-    def are_slots_connected(self, source_slot: Slot, target_node: (), target_slot: Slot) -> bool:
+            bus.Event, GraphControllerRequestBusEvents.ADD_CONNECTION_BY_SLOT_ID, graph_id,
+            source_node.node_python_proxy, source_slot, target_node.node_python_proxy, target_slot)
+
+    def are_slots_connected(self,
+                            graph_id: azlmbr.object.PythonProxyObject,
+                            source_node: Node,
+                            source_slot: graph.GraphModelSlotId,
+                            target_node: Node,
+                            target_slot: graph.GraphModelSlotId) -> bool:
         """
         Determines if the source_slot is connected to the target_slot on the target_node.
-        :param source_slot: A Slot() object for the source_node in a given Node() object.
+        :param graph_id: azlmbr.object.PythonProxyObject object containing an int for this Graph object.
+        :param source_node: A Node() object for the source node that should be connected to the target node.
+        :param source_slot: A graph.GraphModelSlotId representing a slot on the source_node.
         :param target_node: A Node() object for the target node that should be connected to the source node.
-        :param target_slot: A Slot() object for the slot we should be connected to from the source_node.
+        :param target_slot: A graph.GraphModelSlotId representing a slot on the target_node..
         :return: True if the source_slot and target_slot are connected, False otherwise.
         """
         return graph.GraphControllerRequestBus(
-            bus.Event, GraphControllerRequestBusEvents.ARE_SLOTS_CONNECTED, self.graph_id,
-            self.node_object, source_slot.id, target_node.node_object, target_slot.id)
+            bus.Event, GraphControllerRequestBusEvents.ARE_SLOTS_CONNECTED, graph_id,
+            source_node.node_python_proxy, source_slot, target_node.node_python_proxy, target_slot)

AutomatedTesting/Gem/PythonTests/Atom/tests/MaterialCanvas_Atom_BasicTests.py

@@ -86,7 +86,7 @@ def MaterialCanvas_BasicFunctionalityChecks_AllChecksPass():
         test_5_material_graph_file = os.path.join(atom_tools_utils.MATERIALCANVAS_GRAPH_PATH, "test5.materialgraph")
 
         # 1. Open an existing material graph document.
-        test_1_material_graph = Graph(test_1_material_graph_file)
+        test_1_material_graph = Graph(atom_tools_utils.open_document(test_1_material_graph_file))
         Report.result(
             Tests.open_existing_material_graph,
             atom_tools_utils.is_document_open(test_1_material_graph.document_id) is True)
@@ -102,7 +102,7 @@ def MaterialCanvas_BasicFunctionalityChecks_AllChecksPass():
             test_1_material_graph_file, test_2_material_graph_file, test_3_material_graph_file,
             test_4_material_graph_file, test_5_material_graph_file]
         for material_graph_document_file in material_graph_document_files:
-            Graph(material_graph_document_file)
+            Graph(atom_tools_utils.open_document(material_graph_document_file))
         Report.result(
             Tests.close_all_opened_material_graphs,
             atom_tools_utils.close_all_documents() is True)
@@ -110,7 +110,7 @@ def MaterialCanvas_BasicFunctionalityChecks_AllChecksPass():
         # 4. Open multiple material graph documents then verify all material documents are opened.
         test_material_graphs = []
         for material_graph_document_file in material_graph_document_files:
-            test_material_graph = Graph(material_graph_document_file)
+            test_material_graph = Graph(atom_tools_utils.open_document(material_graph_document_file))
             Report.result(Tests.verify_all_material_graphs_are_opened,
                           atom_tools_utils.is_document_open(test_material_graph.document_id) is True)
             test_material_graphs.append(test_material_graph)
@@ -135,28 +135,37 @@ def MaterialCanvas_BasicFunctionalityChecks_AllChecksPass():
         # 7. Verify test_1_material_graph.name is 'test1'.
         Report.result(
             Tests.material_graph_name_is_test1,
-            test_1_material_graph.name == "test1")
+            test_1_material_graph.get_graph_name() == "test1")
 
         # 8. Create a new world_position_node inside test_1_material_graph.
-        test_1_material_graph.add_node('World Position', math.Vector2(-200.0, 10.0))
-        world_position_node = test_1_material_graph.nodes[0]
+        created_world_position_node = test_1_material_graph.create_node_by_name(
+            test_1_material_graph.get_graph(), 'World Position')
+        world_position_node = test_1_material_graph.add_node(
+            test_1_material_graph.get_graph_id(), created_world_position_node, math.Vector2(-200.0, 10.0))
         Report.result(
             Tests.world_position_node_created,
-            world_position_node.node_object.typename == "AZStd::shared_ptr<Node>")
+            world_position_node.node_python_proxy.typename == "AZStd::shared_ptr<Node>")
 
         # 9. Create a new standard_pbr_node inside test_1_material_graph.
-        test_1_material_graph.add_node('Standard PBR', math.Vector2(10.0, 220.0))
-        standard_pbr_node = test_1_material_graph.nodes[1]
+        created_standard_pbr_node = test_1_material_graph.create_node_by_name(
+            test_1_material_graph.get_graph(), 'Standard PBR')
+        standard_pbr_node = test_1_material_graph.add_node(
+            test_1_material_graph.get_graph_id(), created_standard_pbr_node, math.Vector2(10.0, 220.0))
         Report.result(
             Tests.standard_pbr_node_created,
-            standard_pbr_node.node_object.typename == "AZStd::shared_ptr<Node>")
+            standard_pbr_node.node_python_proxy.typename == "AZStd::shared_ptr<Node>")
 
         # 10. Create a node connection between world_position_node outbound slot and standard_pbr_node inbound slot.
-        world_position_node.add_slot('outPosition')
-        standard_pbr_node.add_slot('inPositionOffset')
-        world_position_node.connect_slots(world_position_node.slots[0], standard_pbr_node, standard_pbr_node.slots[0])
-        are_slots_connected = world_position_node.are_slots_connected(
-            world_position_node.slots[0], standard_pbr_node, standard_pbr_node.slots[0])
+        world_position_node_slots = world_position_node.get_slots()
+        standard_pbr_node_slots = standard_pbr_node.get_slots()
+        test_1_material_graph.add_connection_by_slot_id(
+            test_1_material_graph.get_graph_id(),
+            world_position_node, world_position_node_slots['outPosition'],
+            standard_pbr_node, standard_pbr_node_slots['inPositionOffset'])
+        are_slots_connected = test_1_material_graph.are_slots_connected(
+            test_1_material_graph.get_graph_id(),
+            world_position_node, world_position_node_slots['outPosition'],
+            standard_pbr_node, standard_pbr_node_slots['inPositionOffset'])
         Report.result(Tests.nodes_connected, are_slots_connected is True)
 
         # 11. Look for errors and asserts.