Refactoring of CPUGraphicsScene

src/simudator/gui/cpu_graphics_scene.py

@@ -4,7 +4,13 @@ from json.decoder import JSONDecodeError
 from qtpy.QtCore import QPointF
 from qtpy.QtCore import Signal as pyqtSignal
 from qtpy.QtCore import Slot
-from qtpy.QtWidgets import QFileDialog, QGraphicsItem, QGraphicsScene
+from qtpy.QtWidgets import (
+    QErrorMessage,
+    QFileDialog,
+    QGraphicsItem,
+    QGraphicsScene,
+    QMessageBox,
+)
 
 from simudator.core.processor import Processor
 from simudator.gui.color_scheme import ColorScheme
@@ -24,63 +30,72 @@ class CpuGraphicsScene(QGraphicsScene):
 
     MODULE_SPACEING = 100
 
-    def __init__(self, cpu: Processor):
+    def __init__(self):
         super().__init__()
-        self.cpu = cpu
-        self.module_graphics_items = dict()
-        self.signal_graphics_items = []
+        self._module_graphics_items = dict()
+        self._signal_graphics_items = []
+        self._error_msg_box = QErrorMessage()
         self.setBackgroundBrush(ColorScheme.Window)
 
-    def resetSignals(self) -> None:
+    def reset_signals(self) -> None:
         """
-        Resets all graphical signals to their default visual representation
+        Reset all graphical signals to their default visual representation
         when initialised.
         """
-        for graphics_Signal in self.signal_graphics_items:
-            graphics_Signal.reset()
+        for graphics_signal in self._signal_graphics_items:
+            graphics_signal.reset()
 
-    def addModuleGraphicsItem(self, graphics_item: ModuleGraphicsItem) -> None:
+    def add_module(self, item: ModuleGraphicsItem) -> None:
         """
-        Takes a ModuleGraphicsItem and adds it to the scene.
-        Will give it a new position according to the default layout.
-        """
-        self.module_graphics_items[graphics_item.name] = graphics_item
-        self.placeModuleGraphicsItemDefault(graphics_item)
+        Add a graphical module item to the processor scene at a position
+        according to the default layout.
 
-    def placeModuleGraphicsItemDefault(self, graphics_item: ModuleGraphicsItem) -> None:
-        """
-        Places a module graphics items at a position where the x value
-        is equal to the y value. Used to place all graphics items in a
-        diagonal.
+        Parameters
+        ----------
+        item : ModuleGraphicsItem
+            Graphical module item to add to the scene.
         """
-        placement = len(self.module_graphics_items) * self.MODULE_SPACEING
-        graphics_item.setPos(placement, placement)
-        self.addItem(graphics_item)
+        self._module_graphics_items[item.name] = item
+        self._place_module_default(item)
 
-    def replaceModuleGraphicsItem(
-        self, graphics_item: ModuleGraphicsItem, pos_x: int, pos_y: int
-    ) -> None:
+    def _place_module_default(self, item: ModuleGraphicsItem) -> None:
         """
-        Changes the positions of an existing modules graphics item.
+        Place a graphical module item at a position according to the default
+        layout, i.e. placing all module items along a diagonal.
+
+        Parameters
+        ----------
+        item : ModuleGraphicsItem
         """
-        graphics_item.setPos(pos_x * self.MODULE_SPACEING, pos_y * self.MODULE_SPACEING)
+        placement = len(self._module_graphics_items) * self.MODULE_SPACEING
+        item.setPos(placement, placement)
+        self.addItem(item)
 
-    def updateGraphicsItems(self):
+    def move_module_to(self, item: ModuleGraphicsItem, pos_x: int, pos_y: int) -> None:
         """
-        Used to update graphicsitems when modules in the processor has changed values.
+        Place an existing graphical module item at some position.
+
+        Parameters
+        ----------
+        item : ModuleGraphicsItem
+           Graphical module item to place at a position.
+        pos_x : int
+            Position on x-axis to place the module item at.
+        pos_y : int
+            Position on y-axis to place the module item at.
         """
-        for graphics_item in self.module_graphics_items.values():
-            graphics_item.update()
+        item.setPos(pos_x * self.MODULE_SPACEING, pos_y * self.MODULE_SPACEING)
 
-    def addAllSignals(self) -> None:
+    def add_all_signals(self) -> None:
         """
-        Instantiates signals between all matching ports of all modules.
+        Instantiate graphical signals between all matching ports of all
+        graphical modules.
         """
         # Map which ports that are connected using their signals' names
         # (Every port keeps a reference to a simulation signal which has
         # a name)
         signal_to_ports = {}
-        for module_w in self.module_graphics_items.values():
+        for module_w in self._module_graphics_items.values():
             ports = module_w.getPorts()
             for port in ports:
                 s_name = port.getSignalName()
@@ -96,21 +111,34 @@ class CpuGraphicsScene(QGraphicsScene):
                 port_1 = module_graphics_items[0]
                 port_2 = module_graphics_items[1]
                 signal_w = SignalGraphicsItem(port_1, port_2)
-                self.signal_graphics_items.append(signal_w)
+                self._signal_graphics_items.append(signal_w)
                 self.addItem(signal_w)
                 port_1.moved.connect(signal_w.move)
                 port_2.moved.connect(signal_w.move)
                 port_1.toggled.connect(signal_w.toggleVisibility)
                 port_2.toggled.connect(signal_w.toggleVisibility)
 
-    def getModulesGraphicsItems(self) -> list[ModuleGraphicsItem]:
-        return list(self.module_graphics_items.values())
+    def get_modules(self) -> list[ModuleGraphicsItem]:
+        """
+        Get a list of all graphical module items in the scene.
 
-    def moduleGraphicsItemsDict(self) -> dict[str, ModuleGraphicsItem]:
-        return self.module_graphics_items
+        Returns
+        -------
+        list[ModuleGraphicsItem]
+            List of graphical module items in the scene.
+        """
+        return list(self._module_graphics_items.values())
 
-    def getSignalGraphicsItems(self) -> list[SignalGraphicsItem]:
-        return self.signal_graphics_items
+    def get_signals(self) -> list[SignalGraphicsItem]:
+        """
+        Get a list of all graphical signal items in the scene.
+
+        Returns
+        -------
+        list[SignalGraphicsItem]
+            List of graphical signal items in the scene.
+        """
+        return self._signal_graphics_items
 
     def mousePressEvent(self, event):
         super().mousePressEvent(event)
@@ -118,75 +146,46 @@ class CpuGraphicsScene(QGraphicsScene):
     @Slot()
     def load_layout(self) -> None:
         """
-        Loads a given layout from a selected file.
-
-        If the layout was
-        unable to load, an error message will pop up informing the user
-        and the default layout will be loaded.
+        Prompt the user for a layout file and load the layout from the file.
         """
 
-        path = self.folderLoadDialog()
+        # Prompt the user for a file
+        dialog = QFileDialog()
+        dialog.setFileMode(QFileDialog.AnyFile)
+        dialog.setAcceptMode(QFileDialog.AcceptOpen)
+        dialog.setDirectory("~/simudator")  # TODO: does this work when exported?
+        path = dialog.getOpenFileName()[0]
 
         # If no file was selected, do nothing
         if path == '':
             return
 
         try:
-            self.loadLayoutFromFile(path)
+            self.load_layout_from_file(path)
 
         # Anything goes wrong with loading the selected one,
         # we dont care about what went wrong
         except (OSError, JSONDecodeError):
             self.load_default_layout()
-            self.messageBox("Unable to load given file.", "Error")
+            self._error_msg_box.showMessage(
+                "Unable to load given file.", "load_layout_err"
+            )
         else:
-            self.messageBox("File loaded successfully.")
+            QMessageBox.information(
+                self.parent(), "SimuDator", "File loaded succesfully."
+            )
 
-    def load_layout_from_file(self, file_path: str) -> None:
-        """
-        Loads a layout for a processor from a saved filed.
+    def _save_layout_to_file(self, file_path: str) -> None:
         """
-        file = open(file_path)
-
-        graphics_item_name = None
-        graphics_item_str = ""
-
-        # Go through each line in the file
-        for line in file.readlines():
-            # If no name currently saved then get name from current line
-            if graphics_item_name is None:
-                graphics_item_name = line.partition(":")[0]
-
-            # if already has name get line content
-            else:
-                # if not empty line, then save content
-                if line.strip():
-                    graphics_item_str += line
-
-                # if line is empty then give saved content to name file
-                else:
-                    graphics_item = self.module_graphics_items[graphics_item_name]
-                    try:
-                        graphics_item.load_state_from_str(graphics_item_str)
-                    except Exception as exc:
-                        raise exc
+        Save the layout of the scene to file.
 
-                    # set back to empty
-                    graphics_item_str = ""
-                    graphics_item_name = None
-
-        # give module anything that is left
-        if graphics_item_name and graphics_item_str:
-            graphics_item = self.module_graphics_items[graphics_item_name]
-            graphics_item.load_state_from_str(graphics_item_str)
-
-    def save_layout_to_file(self, file_path: str) -> None:
-        """
-        Saves the positions and visibility of graphicsitems in the scene (this
-        includes module and signal representations) to a file.
+        Parameters
+        ----------
+        file_path : str
+            Path to the file to save to layout to.
         """
         layout_str = ""
-        for graphics_item in self.module_graphics_items.values():
+        for graphics_item in self._module_graphics_items.values():
             layout_str += graphics_item.save_state_as_str() + "\n"
 
         file = open(file_path, "w")
@@ -194,69 +193,66 @@ class CpuGraphicsScene(QGraphicsScene):
         file.close()
 
     @Slot(bool)
-    def show_all_signals(self, is_signals_visible: bool) -> None:
-        for item in self.signal_graphics_items:
-            item.setVisible(is_signals_visible)
-
-    @Slot(bool)
-    def show_port_names(self, is_ports_visible: bool) -> None:
-        for item in self.module_graphics_items.values():
-            for port in item.ports:
-                port.setNameVisibility(is_ports_visible)
-
-    def toggle_layout_lock(self, is_layout_locked: bool) -> None:
+    def show_all_signals(self, value: bool) -> None:
         """
-        Toggles the layout lock making it so items in the scene can not be moved.
-        """
-
-        for item in self.module_graphics_items.values():
-            item.setFlag(QGraphicsItem.ItemIsMovable, not is_layout_locked)
-            item.setLocked(is_layout_locked)
+        Set the visibility of all graphical signals in the scene.
 
-        for item in self.signal_graphics_items:
-            item.setFlag(QGraphicsItem.ItemIsMovable, not is_layout_locked)
-
-            # We use this value so lines in the signal can not be moved or edited
-            item.is_locked = is_layout_locked
-
-    def folderSaveDialog(self) -> str:
+        Parameters
+        ----------
+        value : bool
+            ``True`` to show all graphical signals, ``False`` to hide them.
         """
-        Open a file explorer in a new window. Return the absolute path to the selected file.
+        for item in self._signal_graphics_items:
+            item.setVisible(value)
 
-        Can return existing as well as non-existing files.
-        If the selected files does not exist it will be created.
+    @Slot(bool)
+    def show_port_names(self, value: bool) -> None:
         """
-        dialog = QFileDialog()
-        dialog.setFileMode(QFileDialog.AnyFile)
-        dialog.setAcceptMode(QFileDialog.AcceptOpen)
-        dialog.setDirectory("~/simudator")  # TODO: does this work when exported?
+        Set the visibility of the names of all ports of graphical modules
+        in the scene.
 
-        # getSaveFileName() can returncan return existing file but also create new ones
-        return dialog.getSaveFileName()[0]
+        Parameters
+        ----------
+        value : bool
+            ``True`` to show the port names, ``False`` to hide them.
+        """
+        for item in self._module_graphics_items.values():
+            for port in item.ports:
+                port.setNameVisibility(value)
 
-    def folderLoadDialog(self) -> str:
+    def toggle_layout_lock(self, value: bool) -> None:
         """
-        Open a file explorer in a new window. Return the absolute path to the selected file.
+        Toggle the layout between locked and unlocked. Locked means that
+        nothing can be moved around in the layout.
 
-        Can only return existing files.
+        Parameters
+        ----------
+        value : bool
+            ``True`` to lock the layout, ``False`` to unlock it.
         """
-        dialog = QFileDialog()
-        dialog.setFileMode(QFileDialog.AnyFile)
-        dialog.setAcceptMode(QFileDialog.AcceptOpen)
-        dialog.setDirectory("~/simudator")  # TODO: does this work when exported?
 
-        # getOpenFileName() will only return already existing files
-        return dialog.getOpenFileName()[0]
+        for item in self._module_graphics_items.values():
+            item.setFlag(QGraphicsItem.ItemIsMovable, not value)
+            item.setLocked(value)
+
+        for item in self._signal_graphics_items:
+            item.setFlag(QGraphicsItem.ItemIsMovable, not value)
+
+            # We use this value so lines in the signal can not be moved or edited
+            item.is_locked = value
 
     @Slot()
     def save_layout(self) -> None:
         """
-        Saves the layout of all the modules in a (somewhat) human readable format.
+        Prompt the user for a file and save the scene layout to the file.
 
-        This also erases the previous content of the file before saving
-        the data.
+        This overwrites the previous content of the file.
         """
-        path = self.folderSaveDialog()
+        # Prompt the user for a file
+        dialog = QFileDialog()
+        dialog.setFileMode(QFileDialog.AnyFile)
+        dialog.setAcceptMode(QFileDialog.AcceptOpen)
+        path = dialog.getSaveFileName()[0]
 
         # Change file to the selected file if a file was selected
         if path == '':
@@ -268,7 +264,7 @@ class CpuGraphicsScene(QGraphicsScene):
             ports_data = {}
             graphics_signals_data = {}
 
-            graphics_modules = self.getModulesGraphicsItems()
+            graphics_modules = self.get_modules()
             for graphics_module in graphics_modules:
                 pos = (graphics_module.x(), graphics_module.y())
                 graphics_modules_data[graphics_module.getName()] = pos
@@ -279,7 +275,7 @@ class CpuGraphicsScene(QGraphicsScene):
                     data = (port.x(), port.y(), orientation, visibility)
                     ports_data[port.getID()] = data
 
-            for graphics_signal in self.getSignalGraphicsItems():
+            for graphics_signal in self.get_signals():
                 visibility = graphics_signal.isVisible()
                 points = []
                 for point in graphics_signal.getPoints():
@@ -295,25 +291,31 @@ class CpuGraphicsScene(QGraphicsScene):
     @Slot()
     def load_default_layout(self) -> None:
         """
-        Places all the module_graphic objects in a diagonal going down to the right.
+        Place all graphical modules in the scene according to the default
+        layout, i.e. along a diagonal.
         """
         counter = 0
-        for key in self.module_graphics_items:
-            module_graphic = self.module_graphics_items[key]
+        for key in self._module_graphics_items:
+            module_graphic = self._module_graphics_items[key]
             module_graphic.showPorts()
             counter += 1
-            self.replaceModuleGraphicsItem(module_graphic, counter, counter)
-        self.resetSignals()
+            self.move_module_to(module_graphic, counter, counter)
+        self.reset_signals()
 
-    def loadLayoutFromFile(self, file) -> None:
+    def load_layout_from_file(self, file_path: str) -> None:
         """
-        Loads a layout for the current cpu from the selected file.
+        Load a layout from a file.
 
         If at anypoint this function would error, the default layout
         is loaded instead.
+
+        Parameters
+        ----------
+        file_path : str
+            Path to a file containing a layout for the processor scene.
         """
         # TODO: speed(?)
-        graphics_modules = self.moduleGraphicsItemsDict()
+        graphics_modules = self._module_graphics_items
         ports = {}
         graphics_signals = {}
 
@@ -321,11 +323,11 @@ class CpuGraphicsScene(QGraphicsScene):
             for port in graphics_module.getPorts():
                 ports[port.getID()] = port
 
-        for graphics_signal in self.getSignalGraphicsItems():
+        for graphics_signal in self.get_signals():
             graphics_signals[graphics_signal.getID()] = graphics_signal
 
         # Open the file in 'read-only'
-        with open(file, 'rb') as fp:
+        with open(file_path, 'rb') as fp:
             data = json.load(fp)
             graphics_modules_data = data[0]
             ports_data = data[1]
@@ -335,26 +337,36 @@ class CpuGraphicsScene(QGraphicsScene):
                 g_module = graphics_modules[g_module_name]
                 x = g_module_data[0]
                 y = g_module_data[1]
-                self.loadGraphicsModule(g_module, x, y)
+                self._load_module(g_module, x, y)
 
             for port_id, port_data in ports_data.items():
                 port = ports[int(port_id)]
-                self.loadPort(port, *port_data)
+                self._load_port(port, *port_data)
 
             for g_signal_id, g_signal_data in graphics_signals_data.items():
                 g_signal = graphics_signals[int(g_signal_id)]
-                self.loadSignal(g_signal, *g_signal_data)
+                self._load_signal(g_signal, *g_signal_data)
 
         fp.close()
 
-    def loadSignal(
+    def _load_signal(
         self,
-        graphic_signal: SignalGraphicsItem,
+        signal: SignalGraphicsItem,
         signal_points: list[tuple[float, float]],
         visibility: bool,
     ) -> None:
         """
-        Changes the graphical signal to have the positions given as argument.
+        Set the positions and visibility of a graphical signal. Helper method
+        for loading a layout from file.
+
+        Parameters
+        ----------
+        signal : SignalGraphicsItem
+            Graphical signal to modify.
+        signal_points : list[tuple[float, float]]
+            List of points for visually drawing the signal as line.
+        visibility : bool
+            Visibility of the signal. ``True`` to show it, ``False`` to hide it.
         """
         qpoints = []
         # Turn points -> QPointF
@@ -363,30 +375,58 @@ class CpuGraphicsScene(QGraphicsScene):
             qpoints.append(QPointF(point[0], point[1]))
 
         # Set the new points
-        graphic_signal.setPoints(qpoints)
-        graphic_signal.setVisible(visibility)
+        signal.setPoints(qpoints)
+        signal.setVisible(visibility)
 
-    def loadGraphicsModule(
+    def _load_module(
         self,
-        graphics_module: ModuleGraphicsItem,
-        graphics_module_x: float,
-        graphics_module_y: float,
+        module: ModuleGraphicsItem,
+        pos_x: float,
+        pos_y: float,
     ) -> None:
         """
-        Changes the positions of graphical modules to the ones given as argument.
+        Set the position of a graphical module in the scene. Helper method
+        for loading a layout from file.
+
+        Parameters
+        ----------
+        module : ModuleGraphicsItem
+            Graphical module of which to set the position.
+        pos_x : float
+            Position on the x-axis in the scene.
+        pos_y : float
+            Position on the y-axis in the scene.
         """
-        graphics_module.setX(graphics_module_x)
-        graphics_module.setY(graphics_module_y)
+        module.setX(pos_x)
+        module.setY(pos_y)
 
-    def loadPort(
+    def _load_port(
         self,
         port: PortGraphicsItem,
-        x: float,
-        y: float,
+        pos_x: float,
+        pos_y: float,
         orientation: Orientation,
         visibility: bool,
     ) -> None:
+        """
+        Set position, orientation and visibility of a port of a graphical
+        module. Helper method for loading a layout from file.
+
+        Parameters
+        ----------
+        port : PortGraphicsItem
+            Port to modify.
+        pos_x : float
+            Position on the x-axis in the scene.
+        pos_y : float
+            Position on the y-axis in the scene.
+        orientation : Orientation
+            Orientation of the port.
+        visibility : bool
+            Visibility of the port. ``True`` to show the port, ``False`` to
+            hide it.
+        """
         port.setOrientation(orientation)
-        port.setX(x)
-        port.setY(y)
+        port.setX(pos_x)
+        port.setY(pos_y)
         port.setVisible(visibility)

src/simudator/gui/gui.py

@@ -3,7 +3,7 @@ import sys
 
 from qtpy import QtCore, QtWidgets
 from qtpy.QtCore import Signal as pyqtSignal
-from qtpy.QtCore import Slot
+from qtpy.QtCore import Slot, ws
 from qtpy.QtWidgets import (
     QAction,
     QApplication,
@@ -45,7 +45,7 @@ class GUI(QMainWindow):
         self._processor_handler = ProcessorHandler(processor, parent=self)
 
         self.setWindowTitle("SimuDator")
-        self._graphics_scene = CpuGraphicsScene(processor)
+        self._graphics_scene = CpuGraphicsScene()
         self._graphics_view = View(self._graphics_scene)
         # self.graphics_view.setDragMode(True)
         self._module_actions: dict[str, QAction] = {}
@@ -202,7 +202,7 @@ class GUI(QMainWindow):
             module_state = module.get_state()
             module_state[state] = parsed_value
             module.set_state(module_state)
-            self._graphics_scene.updateGraphicsItems()
+            self._graphics_scene.update_modules()
 
     @Slot(str, str, str)
     def editMemoryContent(self, module_name: str, adress: str, value: str) -> None:
@@ -225,7 +225,7 @@ class GUI(QMainWindow):
             module_state = module.get_state()
             module_state['memory'][parsed_adress] = parsed_value
             module.set_state(module_state)
-            self._graphics_scene.updateGraphicsItems()
+            self._graphics_scene.update_modules()
 
     @Slot(str, str, str)
     def addStateBreakpoint(self, module_name: str, state: str, value: str) -> None:
@@ -291,7 +291,7 @@ class GUI(QMainWindow):
         Add a module graphics item to the graphics scene and connect its
         QT signals and slots.
         """
-        self._graphics_scene.addModuleGraphicsItem(item)
+        self._graphics_scene.add_module(item)
         self.connectModuleActions(item.getActionSignals())
         self._processor_handler.changed.connect(item.update)
 
@@ -300,7 +300,7 @@ class GUI(QMainWindow):
         Add visual representations of all processor signals between all modules
         added to the GUI.
         """
-        self._graphics_scene.addAllSignals()
+        self._graphics_scene.add_all_signals()
 
     def load_layout(self, file_path: str) -> None:
         """Load a processor layout from file.
@@ -311,7 +311,7 @@ class GUI(QMainWindow):
             Path to the file containing a processor layout to load.
 
         """
-        self._graphics_scene.loadLayoutFromFile(file_path)
+        self._graphics_scene.load_layout_from_file(file_path)
 
 
 if __name__ == '__main__':