From 6bb1cce43f4f3dbbf489d46d7d3b6a14fd845b30 Mon Sep 17 00:00:00 2001
From: Doug Hoskisson <beauxq@users.noreply.github.com>
Date: Thu, 6 Jun 2024 11:36:14 -0700
Subject: [PATCH] Core: hot reload components from installed apworld (#3480)

* Core: hot reload components from installed apworld

* address PR reviews

`Launcher` widget members default to `None` so they can be defined in `build`

`Launcher._refresh_components` is not wrapped

loaded world goes into `world_sources` so we can check if it's already loaded.
(`WorldSource` can be ordered now without trying to compare `None` and `float`)
(don't load empty directories so we don't detect them as worlds)

* clarify that the installation is successful
---
 Launcher.py                    | 71 ++++++++++++++++++++++++----------
 typings/kivy/uix/boxlayout.pyi |  6 +++
 typings/kivy/uix/layout.pyi    |  8 +++-
 typings/schema/__init__.pyi    | 17 ++++++++
 worlds/LauncherComponents.py   | 23 ++++++++++-
 worlds/__init__.py             | 12 ++++--
 6 files changed, 110 insertions(+), 27 deletions(-)
 create mode 100644 typings/kivy/uix/boxlayout.pyi
 create mode 100644 typings/schema/__init__.pyi

diff --git a/Launcher.py b/Launcher.py
index e26e4afc..e4b65be9 100644
--- a/Launcher.py
+++ b/Launcher.py
@@ -19,7 +19,7 @@ import sys
 import webbrowser
 from os.path import isfile
 from shutil import which
-from typing import Sequence, Union, Optional
+from typing import Callable, Sequence, Union, Optional
 
 import Utils
 import settings
@@ -160,6 +160,9 @@ def launch(exe, in_terminal=False):
     subprocess.Popen(exe)
 
 
+refresh_components: Optional[Callable[[], None]] = None
+
+
 def run_gui():
     from kvui import App, ContainerLayout, GridLayout, Button, Label, ScrollBox, Widget
     from kivy.core.window import Window
@@ -170,11 +173,8 @@ def run_gui():
         base_title: str = "Archipelago Launcher"
         container: ContainerLayout
         grid: GridLayout
-
-        _tools = {c.display_name: c for c in components if c.type == Type.TOOL}
-        _clients = {c.display_name: c for c in components if c.type == Type.CLIENT}
-        _adjusters = {c.display_name: c for c in components if c.type == Type.ADJUSTER}
-        _miscs = {c.display_name: c for c in components if c.type == Type.MISC}
+        _tool_layout: Optional[ScrollBox] = None
+        _client_layout: Optional[ScrollBox] = None
 
         def __init__(self, ctx=None):
             self.title = self.base_title
@@ -182,18 +182,7 @@ def run_gui():
             self.icon = r"data/icon.png"
             super().__init__()
 
-        def build(self):
-            self.container = ContainerLayout()
-            self.grid = GridLayout(cols=2)
-            self.container.add_widget(self.grid)
-            self.grid.add_widget(Label(text="General", size_hint_y=None, height=40))
-            self.grid.add_widget(Label(text="Clients", size_hint_y=None, height=40))
-            tool_layout = ScrollBox()
-            tool_layout.layout.orientation = "vertical"
-            self.grid.add_widget(tool_layout)
-            client_layout = ScrollBox()
-            client_layout.layout.orientation = "vertical"
-            self.grid.add_widget(client_layout)
+        def _refresh_components(self) -> None:
 
             def build_button(component: Component) -> Widget:
                 """
@@ -218,14 +207,47 @@ def run_gui():
                     return box_layout
                 return button
 
+            # clear before repopulating
+            assert self._tool_layout and self._client_layout, "must call `build` first"
+            tool_children = reversed(self._tool_layout.layout.children)
+            for child in tool_children:
+                self._tool_layout.layout.remove_widget(child)
+            client_children = reversed(self._client_layout.layout.children)
+            for child in client_children:
+                self._client_layout.layout.remove_widget(child)
+
+            _tools = {c.display_name: c for c in components if c.type == Type.TOOL}
+            _clients = {c.display_name: c for c in components if c.type == Type.CLIENT}
+            _adjusters = {c.display_name: c for c in components if c.type == Type.ADJUSTER}
+            _miscs = {c.display_name: c for c in components if c.type == Type.MISC}
+
             for (tool, client) in itertools.zip_longest(itertools.chain(
-                    self._tools.items(), self._miscs.items(), self._adjusters.items()), self._clients.items()):
+                _tools.items(), _miscs.items(), _adjusters.items()
+            ), _clients.items()):
                 # column 1
                 if tool:
-                    tool_layout.layout.add_widget(build_button(tool[1]))
+                    self._tool_layout.layout.add_widget(build_button(tool[1]))
                 # column 2
                 if client:
-                    client_layout.layout.add_widget(build_button(client[1]))
+                    self._client_layout.layout.add_widget(build_button(client[1]))
+
+        def build(self):
+            self.container = ContainerLayout()
+            self.grid = GridLayout(cols=2)
+            self.container.add_widget(self.grid)
+            self.grid.add_widget(Label(text="General", size_hint_y=None, height=40))
+            self.grid.add_widget(Label(text="Clients", size_hint_y=None, height=40))
+            self._tool_layout = ScrollBox()
+            self._tool_layout.layout.orientation = "vertical"
+            self.grid.add_widget(self._tool_layout)
+            self._client_layout = ScrollBox()
+            self._client_layout.layout.orientation = "vertical"
+            self.grid.add_widget(self._client_layout)
+
+            self._refresh_components()
+
+            global refresh_components
+            refresh_components = self._refresh_components
 
             Window.bind(on_drop_file=self._on_drop_file)
 
@@ -254,10 +276,17 @@ def run_gui():
 
     Launcher().run()
 
+    # avoiding Launcher reference leak
+    # and don't try to do something with widgets after window closed
+    global refresh_components
+    refresh_components = None
+
 
 def run_component(component: Component, *args):
     if component.func:
         component.func(*args)
+        if refresh_components:
+            refresh_components()
     elif component.script_name:
         subprocess.run([*get_exe(component.script_name), *args])
     else:
diff --git a/typings/kivy/uix/boxlayout.pyi b/typings/kivy/uix/boxlayout.pyi
new file mode 100644
index 00000000..c63d691d
--- /dev/null
+++ b/typings/kivy/uix/boxlayout.pyi
@@ -0,0 +1,6 @@
+from typing import Literal
+from .layout import Layout
+
+
+class BoxLayout(Layout):
+    orientation: Literal['horizontal', 'vertical']
diff --git a/typings/kivy/uix/layout.pyi b/typings/kivy/uix/layout.pyi
index 2a418a1d..c27f8908 100644
--- a/typings/kivy/uix/layout.pyi
+++ b/typings/kivy/uix/layout.pyi
@@ -1,8 +1,14 @@
-from typing import Any
+from typing import Any, Sequence
+
 from .widget import Widget
 
 
 class Layout(Widget):
+    @property
+    def children(self) -> Sequence[Widget]: ...
+
     def add_widget(self, widget: Widget) -> None: ...
 
+    def remove_widget(self, widget: Widget) -> None: ...
+
     def do_layout(self, *largs: Any, **kwargs: Any) -> None: ...
diff --git a/typings/schema/__init__.pyi b/typings/schema/__init__.pyi
new file mode 100644
index 00000000..d993ec22
--- /dev/null
+++ b/typings/schema/__init__.pyi
@@ -0,0 +1,17 @@
+from typing import Any, Callable
+
+
+class And:
+    def __init__(self, __type: type, __func: Callable[[Any], bool]) -> None: ...
+
+
+class Or:
+    def __init__(self, *args: object) -> None: ...
+
+
+class Schema:
+    def __init__(self, __x: object) -> None: ...
+
+
+class Optional(Schema):
+    ...
diff --git a/worlds/LauncherComponents.py b/worlds/LauncherComponents.py
index 890b41aa..18c1a166 100644
--- a/worlds/LauncherComponents.py
+++ b/worlds/LauncherComponents.py
@@ -1,3 +1,4 @@
+import bisect
 import logging
 import pathlib
 import weakref
@@ -94,9 +95,10 @@ def _install_apworld(apworld_src: str = "") -> Optional[Tuple[pathlib.Path, path
 
     apworld_path = pathlib.Path(apworld_src)
 
+    module_name = pathlib.Path(apworld_path.name).stem
     try:
         import zipfile
-        zipfile.ZipFile(apworld_path).open(pathlib.Path(apworld_path.name).stem + "/__init__.py")
+        zipfile.ZipFile(apworld_path).open(module_name + "/__init__.py")
     except ValueError as e:
         raise Exception("Archive appears invalid or damaged.") from e
     except KeyError as e:
@@ -107,6 +109,9 @@ def _install_apworld(apworld_src: str = "") -> Optional[Tuple[pathlib.Path, path
         raise Exception("Custom Worlds directory appears to not be writable.")
     for world_source in worlds.world_sources:
         if apworld_path.samefile(world_source.resolved_path):
+            # Note that this doesn't check if the same world is already installed.
+            # It only checks if the user is trying to install the apworld file
+            # that comes from the installation location (worlds or custom_worlds)
             raise Exception(f"APWorld is already installed at {world_source.resolved_path}.")
 
     # TODO: run generic test suite over the apworld.
@@ -116,6 +121,22 @@ def _install_apworld(apworld_src: str = "") -> Optional[Tuple[pathlib.Path, path
     import shutil
     shutil.copyfile(apworld_path, target)
 
+    # If a module with this name is already loaded, then we can't load it now.
+    # TODO: We need to be able to unload a world module,
+    # so the user can update a world without restarting the application.
+    found_already_loaded = False
+    for loaded_world in worlds.world_sources:
+        loaded_name = pathlib.Path(loaded_world.path).stem
+        if module_name == loaded_name:
+            found_already_loaded = True
+            break
+    if found_already_loaded:
+        raise Exception(f"Installed APWorld successfully, but '{module_name}' is already loaded,\n"
+                        "so a Launcher restart is required to use the new installation.")
+    world_source = worlds.WorldSource(str(target), is_zip=True)
+    bisect.insort(worlds.world_sources, world_source)
+    world_source.load()
+
     return apworld_path, target
 
 
diff --git a/worlds/__init__.py b/worlds/__init__.py
index 4da9d8e8..83ee9613 100644
--- a/worlds/__init__.py
+++ b/worlds/__init__.py
@@ -1,11 +1,12 @@
 import importlib
+import logging
 import os
 import sys
 import warnings
 import zipimport
 import time
 import dataclasses
-from typing import Dict, List, TypedDict, Optional
+from typing import Dict, List, TypedDict
 
 from Utils import local_path, user_path
 
@@ -48,7 +49,7 @@ class WorldSource:
     path: str  # typically relative path from this module
     is_zip: bool = False
     relative: bool = True  # relative to regular world import folder
-    time_taken: Optional[float] = None
+    time_taken: float = -1.0
 
     def __repr__(self) -> str:
         return f"{self.__class__.__name__}({self.path}, is_zip={self.is_zip}, relative={self.relative})"
@@ -92,7 +93,6 @@ class WorldSource:
             print(f"Could not load world {self}:", file=file_like)
             traceback.print_exc(file=file_like)
             file_like.seek(0)
-            import logging
             logging.exception(file_like.read())
             failed_world_loads.append(os.path.basename(self.path).rsplit(".", 1)[0])
             return False
@@ -107,7 +107,11 @@ for folder in (folder for folder in (user_folder, local_folder) if folder):
         if not entry.name.startswith(("_", ".")):
             file_name = entry.name if relative else os.path.join(folder, entry.name)
             if entry.is_dir():
-                world_sources.append(WorldSource(file_name, relative=relative))
+                init_file_path = os.path.join(entry.path, '__init__.py')
+                if os.path.isfile(init_file_path):
+                    world_sources.append(WorldSource(file_name, relative=relative))
+                else:
+                    logging.warning(f"excluding {entry.name} from world sources because it has no __init__.py")
             elif entry.is_file() and entry.name.endswith(".apworld"):
                 world_sources.append(WorldSource(file_name, is_zip=True, relative=relative))