fix: resolve several bugs in the tkinter ui

src/floors/tkinter_floor.py

@@ -2,9 +2,7 @@
 Module for the TkinterFloor class.
 """
 
-import threading
-import time
-from typing import List, cast
+from typing import List, Optional, cast
 
 from floors.floor import Floor
 from rooms.room import Room
@@ -30,7 +28,7 @@ def __init__(self, height: int, width: int, canvas, name: str, seed: str):
         self._canvas = canvas
         self._first_draw = True
         self.name = name
-        self._is_drawing = False
+        self._after_id: Optional[str] = None
 
     def add_room(self, x: int, y: int, room_type=RoomType.NORMAL_ROOM):
         self.add_to_floor_grid(x, y)
@@ -84,29 +82,34 @@ def draw(self) -> None:
         for room in self._tkinter_rooms():
             room.draw(self._canvas)
 
-    def draw_thread(self) -> None:
-        """
-        Starts a new thread to draw the floor step by step.
-        """
-        thread = threading.Thread(target=self.draw_step_by_step)
-        thread.start()
-
     def stop_drawing(self) -> None:
         """
-        Stops the drawing of the floor.
+        Stops the step by step drawing of the floor.
         """
-        self._is_drawing = False
+        if self._after_id is not None:
+            self._canvas.after_cancel(self._after_id)
+            self._after_id = None
 
-    def draw_step_by_step(self) -> None:
+    def draw_step_by_step(self, delay_ms: int = 500) -> None:
         """
-        Draws the floor step by step.
+        Draws the floor room by room using the tkinter event loop.
+        @param delay_ms: delay between two rooms in milliseconds
         """
+        self.stop_drawing()
         self._canvas.delete("all")
         sorted_rooms = sorted(self._tkinter_rooms(), key=lambda r: r.get_id())
-        self._is_drawing = True
-        for room in sorted_rooms:
-            if not self._is_drawing:
-                sorted_rooms.clear()
-                return
-            room.draw(self._canvas)
-            time.sleep(0.5)
+        self._draw_next_room(sorted_rooms, delay_ms)
+
+    def _draw_next_room(self, rooms: List[TkinterRoom], delay_ms: int) -> None:
+        """
+        Draws the first of the given rooms and schedules the remaining rooms.
+        @param rooms: rooms that still have to be drawn
+        @param delay_ms: delay between two rooms in milliseconds
+        """
+        self._after_id = None
+        if len(rooms) == 0:
+            return
+        rooms[0].draw(self._canvas)
+        self._after_id = self._canvas.after(
+            delay_ms, self._draw_next_room, rooms[1:], delay_ms
+        )

src/generators/floor_manager.py

@@ -98,9 +98,3 @@ def increase_floor(self) -> None:
             current_floor.draw()
             self.current_floor_name.set(current_floor.name)
             self.current_floor_seed_var.set(self.get_current_floor().seed)
-
-    def set_next_seed(self) -> None:
-        """
-        Sets the next seed to the current seed.
-        """
-        self.current_floor_seed_var.set(self.next_seed.get())

src/generators/tkinter_generator.py

@@ -41,7 +41,7 @@ def __init__(
         hex_color = util_functions.rgb2hex(color[0], color[1], color[2])
         self._canvas = tk.Canvas(
             height=my_globals.FLOOR_HEIGHT * my_globals.ROOM_HEIGHT,
-            width=my_globals.ROOM_WIDTH * my_globals.ROOM_WIDTH,
+            width=my_globals.FLOOR_WIDTH * my_globals.ROOM_WIDTH,
             background=hex_color,
         )
         self._path = tk.StringVar()
@@ -69,11 +69,12 @@ def _generate_and_draw_floor(self) -> None:
         """
         Cals the generate method and the draw method of the floor.
         """
+        self._floor.stop_drawing()
         self.generate(self._floor_manager.next_seed.get())
         self._floor_manager.next_seed.set(self._floor_manager.next_seed.get())
         self._floor_manager.next_seed.set(secrets.token_hex(16))
         # self._floors[self._current_floor].draw(tk)
-        self._floor_manager.get_current_floor().draw_thread()
+        self._floor_manager.get_current_floor().draw_step_by_step()
         self._floor = self._floor_manager.get_current_floor()
 
     def open(self) -> None:
@@ -219,17 +220,18 @@ def add_buttons(self) -> None:
         button_next = ttk.Button(button_frame, text="->", command=self._increase_floor)
         button_next.pack(side=tk.LEFT, padx=10)
         # Right arrow to increase
-        self._tk.bind("<Right>", lambda event: self._floor_manager.increase_floor())
+        self._tk.bind("<Right>", lambda event: self._increase_floor())
         # Next seed entry
         next_seed_label = ttk.Label(button_frame, text="Next seed: ")
         next_seed_label.pack(side=tk.LEFT, padx=10)
-        next_seed_entry = ttk.Entry(
+        self._next_seed_entry = ttk.Entry(
             button_frame, textvariable=self._floor_manager.next_seed
         )
-        next_seed_entry.bind(
-            "<Return>", lambda event: self._floor_manager.set_next_seed()
+        # Enter generates a floor with the entered seed
+        self._next_seed_entry.bind(
+            "<Return>", lambda event: self._generate_and_draw_floor()
         )
-        next_seed_entry.pack(side=tk.LEFT, padx=10)
+        self._next_seed_entry.pack(side=tk.LEFT, padx=10)
 
     def create_menu_bar(self) -> None:
         """
@@ -241,9 +243,7 @@ def create_menu_bar(self) -> None:
         menu_bar.add_command(label="Open", command=self.open, accelerator="Ctrl+o")
         menu_bar.add_command(
             label="Save",
-            command=lambda: self.save(
-                os.path.join(self._output_file_path, self._output_file_name)
-            ),
+            command=self._save_current_floor,
             accelerator="Ctrl+s",
         )
         menu_bar.add_command(
@@ -258,3 +258,15 @@ def create_menu_bar(self) -> None:
             label="Legend", command=legend_window.display_legend, accelerator="F1"
         )
         menu_bar.add_command(label="Exit", command=self.quit, accelerator="Ctrl+q")
+        self._tk.bind("<Control-o>", lambda event: self.open())
+        self._tk.bind("<Control-s>", lambda event: self._save_current_floor())
+        self._tk.bind("<Control-S>", lambda event: self.save())
+        self._tk.bind("<Control-t>", lambda event: self._theme_handler.switch_theme())
+        self._tk.bind("<F1>", lambda event: legend_window.display_legend())
+        self._tk.bind("<Control-q>", lambda event: self.quit())
+
+    def _save_current_floor(self) -> None:
+        """
+        Saves the current floor to the current output file.
+        """
+        self.save(os.path.join(self._output_file_path, self._output_file_name))

test/tkinter_generator_test.py

@@ -0,0 +1,150 @@
+"""Unit tests for the TkinterGenerator class."""
+
+import importlib.util
+import tempfile
+import time
+import unittest
+from unittest import mock
+
+from utils import globals as my_globals
+
+_HAS_TKINTER = importlib.util.find_spec("tkinter") is not None
+
+
+def _has_display() -> bool:
+    if not _HAS_TKINTER:
+        return False
+    # pylint: disable=import-outside-toplevel
+    import tkinter
+
+    try:
+        tkinter.Tk().destroy()
+    except tkinter.TclError:
+        return False
+    return True
+
+
+@unittest.skipUnless(_has_display(), "tkinter or a display is not available")
+class TkinterGeneratorTest(unittest.TestCase):
+    def setUp(self) -> None:
+        # pylint: disable=import-outside-toplevel
+        from generators.tkinter_generator import TkinterGenerator
+
+        self._tmp_dir = tempfile.TemporaryDirectory()
+        self._generator = TkinterGenerator("1", "floor", self._tmp_dir.name, 2)
+        self._generator.generate()
+
+    def tearDown(self) -> None:
+        self._generator._tk.destroy()
+        self._tmp_dir.cleanup()
+
+    def test_canvas_fits_the_floor(self):
+        canvas = self._generator._canvas
+        self.assertEqual(
+            my_globals.FLOOR_WIDTH * my_globals.ROOM_WIDTH, int(canvas.cget("width"))
+        )
+        self.assertEqual(
+            my_globals.FLOOR_HEIGHT * my_globals.ROOM_HEIGHT, int(canvas.cget("height"))
+        )
+
+    def _press(self, key: str) -> None:
+        tk = self._generator._tk
+        tk.update()
+        tk.focus_force()
+        tk.event_generate(key, when="now")
+        tk.update()
+
+    def test_arrow_keys_switch_the_current_floor(self):
+        self._generator.add_buttons()
+        first = self._generator._floor
+        self._generator.generate("2")
+        second = self._generator._floor
+
+        self._press("<Left>")
+        self.assertIs(first, self._generator._floor)
+
+        self._press("<Right>")
+        self.assertIs(second, self._generator._floor)
+
+    def test_menu_shortcuts_are_bound(self):
+        generator = self._generator
+        generator.create_menu_bar()
+        shortcuts = {
+            "<Control-o>": "open",
+            "<Control-s>": "_save_current_floor",
+            "<Control-S>": "save",
+            "<Control-q>": "quit",
+        }
+        for key, method in shortcuts.items():
+            with self.subTest(key=key), mock.patch.object(generator, method) as m:
+                self._press(key)
+                m.assert_called_once()
+
+        with mock.patch.object(generator._theme_handler, "switch_theme") as m:
+            self._press("<Control-t>")
+            m.assert_called_once()
+
+        with mock.patch("rooms.tkinter.legend_window.display_legend") as m:
+            self._press("<F1>")
+            m.assert_called_once()
+
+    def test_enter_in_the_next_seed_entry_generates_a_floor_with_that_seed(self):
+        generator = self._generator
+        generator.add_buttons()
+        generator._floor_manager.next_seed.set("entered-seed")
+        entry = generator._next_seed_entry
+
+        with mock.patch("floors.tkinter_floor.TkinterFloor.draw_step_by_step"):
+            generator._tk.update()
+            entry.focus_force()
+            entry.event_generate("<Return>", when="now")
+            generator._tk.update()
+
+        self.assertEqual("entered-seed", generator._floor.seed)
+        self.assertEqual(
+            "entered-seed", generator._floor_manager.current_floor_seed_var.get()
+        )
+
+    def _pump(self, milliseconds: int) -> None:
+        end = time.monotonic() + milliseconds / 1000
+        while time.monotonic() < end:
+            self._generator._tk.update()
+            time.sleep(0.005)
+
+    def _drawn_rooms(self) -> int:
+        canvas = self._generator._canvas
+        return len(
+            [item for item in canvas.find_all() if canvas.type(item) == "rectangle"]
+        )
+
+    def test_draw_step_by_step_draws_one_room_after_another(self):
+        floor = self._generator._floor
+
+        floor.draw_step_by_step(delay_ms=10)
+
+        self.assertEqual(1, self._drawn_rooms())
+        self._pump(10 * len(floor.get_rooms()) + 300)
+        self.assertEqual(len(floor.get_rooms()), self._drawn_rooms())
+
+    def test_stop_drawing_stops_the_step_by_step_drawing(self):
+        floor = self._generator._floor
+        floor.draw_step_by_step(delay_ms=10)
+
+        floor.stop_drawing()
+        self._pump(200)
+
+        self.assertEqual(1, self._drawn_rooms())
+
+    def test_generating_a_new_floor_stops_drawing_the_previous_one(self):
+        self._generator._floor.draw_step_by_step(delay_ms=10)
+
+        self._generator._generate_and_draw_floor()
+        self._pump(200)
+
+        # Only the first room of the new floor is drawn, the next one follows
+        # after 500 ms. Rooms of the previous floor must not be drawn anymore.
+        self.assertEqual(1, self._drawn_rooms())
+
+
+if __name__ == "__main__":
+    unittest.main()