Temper and Harden Text Client (#52)

* Provide config path so OpenGOAL can use mod-specific saves and settings.

* Add versioning to MemoryReader. Harden the client against user errors.

* Updated comments.

* Add Deathlink as a "statement of intent" to the YAML. Small updates to client.

* Revert deathlink changes.

* Update error message.

* Added color markup to log messages printed in text client.

* Separate loggers by agent, write markup to GUI and non-markup to disk simultaneously.

* Refactor MemoryReader callbacks from main_tick to constructor.

* Make callback names more... informative.

* Give users explicit instructions in error messages.

worlds/jakanddaxter/Client.py

@@ -1,5 +1,8 @@
+import logging
 import os
 import subprocess
+from logging import Logger
+
 import colorama
 
 import asyncio
@@ -12,7 +15,7 @@ from pymem.exception import ProcessNotFound
 
 import Utils
 from NetUtils import ClientStatus
-from CommonClient import ClientCommandProcessor, CommonContext, logger, server_loop, gui_enabled
+from CommonClient import ClientCommandProcessor, CommonContext, server_loop, gui_enabled
 from .Options import EnableOrbsanity
 
 from .GameID import jak1_name
@@ -23,6 +26,7 @@ import ModuleUpdate
 ModuleUpdate.update()
 
 
+logger = logging.getLogger("JakClient")
 all_tasks: Set[Task] = set()
 
 
@@ -51,7 +55,7 @@ class JakAndDaxterClientCommandProcessor(ClientCommandProcessor):
         - status : check internal status of the REPL."""
         if arguments:
             if arguments[0] == "connect":
-                logger.info("This may take a bit... Wait for the success audio cue before continuing!")
+                self.ctx.on_log_info(logger, "This may take a bit... Wait for the success audio cue before continuing!")
                 self.ctx.repl.initiated_connect = True
             if arguments[0] == "status":
                 create_task_log_exception(self.ctx.repl.print_status())
@@ -64,7 +68,7 @@ class JakAndDaxterClientCommandProcessor(ClientCommandProcessor):
             if arguments[0] == "connect":
                 self.ctx.memr.initiated_connect = True
             if arguments[0] == "status":
-                self.ctx.memr.print_status()
+                create_task_log_exception(self.ctx.memr.print_status())
 
 
 class JakAndDaxterContext(CommonContext):
@@ -85,8 +89,19 @@ class JakAndDaxterContext(CommonContext):
     memr_task: asyncio.Task
 
     def __init__(self, server_address: Optional[str], password: Optional[str]) -> None:
-        self.repl = JakAndDaxterReplClient()
-        self.memr = JakAndDaxterMemoryReader()
+        self.repl = JakAndDaxterReplClient(self.on_log_error,
+                                           self.on_log_warn,
+                                           self.on_log_success,
+                                           self.on_log_info)
+        self.memr = JakAndDaxterMemoryReader(self.on_location_check,
+                                             self.on_finish_check,
+                                             self.on_deathlink_check,
+                                             self.on_deathlink_toggle,
+                                             self.on_orb_trade,
+                                             self.on_log_error,
+                                             self.on_log_warn,
+                                             self.on_log_success,
+                                             self.on_log_info)
         # self.repl.load_data()
         # self.memr.load_data()
         super().__init__(server_address, password)
@@ -219,7 +234,7 @@ class JakAndDaxterContext(CommonContext):
             player = self.player_names[self.slot] if self.slot is not None else "Jak"
             death_text = self.memr.cause_of_death.replace("Jak", player)
             await self.send_death(death_text)
-            logger.info(death_text)
+            self.on_log_warn(logger, death_text)
 
         # Reset all flags, but leave the death count alone.
         self.memr.send_deathlink = False
@@ -246,6 +261,33 @@ class JakAndDaxterContext(CommonContext):
     def on_orb_trade(self, orbs_changed: int):
         create_task_log_exception(self.ap_inform_orb_trade(orbs_changed))
 
+    def on_log_error(self, lg: Logger, message: str):
+        lg.error(message)
+        if self.ui:
+            color = self.jsontotextparser.color_codes["red"]
+            self.ui.log_panels["Archipelago"].on_message_markup(f"[color={color}]{message}[/color]")
+            self.ui.log_panels["All"].on_message_markup(f"[color={color}]{message}[/color]")
+
+    def on_log_warn(self, lg: Logger, message: str):
+        lg.warning(message)
+        if self.ui:
+            color = self.jsontotextparser.color_codes["orange"]
+            self.ui.log_panels["Archipelago"].on_message_markup(f"[color={color}]{message}[/color]")
+            self.ui.log_panels["All"].on_message_markup(f"[color={color}]{message}[/color]")
+
+    def on_log_success(self, lg: Logger, message: str):
+        lg.info(message)
+        if self.ui:
+            color = self.jsontotextparser.color_codes["green"]
+            self.ui.log_panels["Archipelago"].on_message_markup(f"[color={color}]{message}[/color]")
+            self.ui.log_panels["All"].on_message_markup(f"[color={color}]{message}[/color]")
+
+    def on_log_info(self, lg: Logger, message: str):
+        lg.info(message)
+        if self.ui:
+            self.ui.log_panels["Archipelago"].on_message_markup(f"{message}")
+            self.ui.log_panels["All"].on_message_markup(f"{message}")
+
     async def run_repl_loop(self):
         while True:
             await self.repl.main_tick()
@@ -253,66 +295,125 @@ class JakAndDaxterContext(CommonContext):
 
     async def run_memr_loop(self):
         while True:
-            await self.memr.main_tick(self.on_location_check,
-                                      self.on_finish_check,
-                                      self.on_deathlink_check,
-                                      self.on_deathlink_toggle,
-                                      self.on_orb_trade)
+            await self.memr.main_tick()
             await asyncio.sleep(0.1)
 
 
 async def run_game(ctx: JakAndDaxterContext):
 
     # These may already be running. If they are not running, try to start them.
+    # TODO - Support other OS's. cmd for some reason does not work with goalc. Pymem is Windows-only.
     gk_running = False
     try:
         pymem.Pymem("gk.exe")  # The GOAL Kernel
         gk_running = True
     except ProcessNotFound:
-        logger.info("Game not running, attempting to start.")
+        ctx.on_log_warn(logger, "Game not running, attempting to start.")
 
     goalc_running = False
     try:
         pymem.Pymem("goalc.exe")  # The GOAL Compiler and REPL
         goalc_running = True
     except ProcessNotFound:
-        logger.info("Compiler not running, attempting to start.")
+        ctx.on_log_warn(logger, "Compiler not running, attempting to start.")
 
-    # Don't mind all the arguments, they are exactly what you get when you run "task boot-game" or "task repl".
-    # TODO - Support other OS's. cmd for some reason does not work with goalc. Pymem is Windows-only.
-    if not gk_running:
-        try:
-            gk_path = Utils.get_settings()["jakanddaxter_options"]["root_directory"]
-            gk_path = os.path.normpath(gk_path)
-            gk_path = os.path.join(gk_path, "gk.exe")
-        except AttributeError as e:
-            logger.error(f"Hosts.yaml does not contain {e.args[0]}, unable to locate game executables.")
+    try:
+        # Validate folder and file structures of the ArchipelaGOAL root directory.
+        root_path = Utils.get_settings()["jakanddaxter_options"]["root_directory"]
+
+        # Always trust your instincts.
+        if "/" not in root_path:
+            msg = (f"The ArchipelaGOAL root directory contains no path. (Are you missing forward slashes?)\n"
+                   f"Please check that the value of 'jakanddaxter_options > root_directory' in your host.yaml file "
+                   f"is a valid existing path, and all backslashes have been replaced with forward slashes.")
+            ctx.on_log_error(logger, msg)
             return
 
-        if gk_path:
-            # Prefixing ampersand and wrapping gk_path in quotes is necessary for paths with spaces in them.
+        # Start by checking the existence of the root directory provided in the host.yaml file.
+        root_path = os.path.normpath(root_path)
+        if not os.path.exists(root_path):
+            msg = (f"The ArchipelaGOAL root directory does not exist, unable to locate the Game and Compiler.\n"
+                   f"Please check that the value of 'jakanddaxter_options > root_directory' in your host.yaml file "
+                   f"is a valid existing path, and all backslashes have been replaced with forward slashes.")
+            ctx.on_log_error(logger, msg)
+            return
+
+        # Now double-check the existence of the two executables we need.
+        gk_path = os.path.join(root_path, "gk.exe")
+        goalc_path = os.path.join(root_path, "goalc.exe")
+        if not os.path.exists(gk_path) or not os.path.exists(goalc_path):
+            msg = (f"The Game and Compiler could not be found in the ArchipelaGOAL root directory.\n"
+                   f"Please check the value of 'jakanddaxter_options > root_directory' in your host.yaml file, "
+                   f"and ensure that path contains gk.exe, goalc.exe, and a data folder.")
+            ctx.on_log_error(logger, msg)
+            return
+
+        # IMPORTANT: Before we check the existence of the next piece, we must ask "Are you a developer?"
+        # The OpenGOAL Compiler checks the existence of the "data" folder to determine if you're running from source
+        # or from a built package. As a developer, your repository folder itself IS the data folder and the Compiler
+        # knows this. You would have created your "iso_data" folder here as well, so we can skip the "iso_data" check.
+        # HOWEVER, for everyone who is NOT a developer, we must ensure that they copied the "iso_data" folder INTO
+        # the "data" folder per the setup instructions.
+        data_path = os.path.join(root_path, "data")
+        if os.path.exists(data_path):
+
+            # NOW double-check the existence of the iso_data folder under <root directory>/data. This is necessary
+            # for the compiler to compile the game correctly.
+            # TODO - If the GOALC compiler is updated to take the iso_data folder as a runtime argument,
+            #  we may be able to remove this step.
+            iso_data_path = os.path.join(root_path, "data", "iso_data")
+            if not os.path.exists(iso_data_path):
+                msg = (f"The iso_data folder could not be found in the ArchipelaGOAL data directory.\n"
+                       f"Please follow these steps:\n"
+                       f"   Run the OpenGOAL Launcher, click Jak and Daxter > Advanced > Open Game Data Folder.\n"
+                       f"   Copy the iso_data folder from this location.\n"
+                       f"   Click Jak and Daxter > Features > Mods > ArchipelaGOAL > Advanced > Open Game Data Folder.\n"
+                       f"   Paste the iso_data folder in this location.\n"
+                       f"   Click Advanced > Compile. When this is done, click Continue.\n"
+                       f"   Close all launchers, games, clients, and Powershell windows, then restart Archipelago.\n"
+                       f"(See Setup Guide for more details.)")
+                ctx.on_log_error(logger, msg)
+                return
+
+        # Now we can FINALLY attempt to start the programs.
+        if not gk_running:
+            # Per-mod saves and settings are stored in a spot that is a little unusual to get to. We have to .. out of
+            # ArchipelaGOAL root folder, then traverse down to _settings/archipelagoal. Then we normalize this path
+            # and pass it in as an argument to gk. This folder will be created if it does not exist.
+            config_relative_path = "../_settings/archipelagoal"
+            config_path = os.path.normpath(
+                os.path.join(
+                    os.path.normpath(root_path),
+                    os.path.normpath(config_relative_path)))
+
+            # Prefixing ampersand and wrapping in quotes is necessary for paths with spaces in them.
             gk_process = subprocess.Popen(
-                ["powershell.exe", f"& \"{gk_path}\"", "--game jak1", "--", "-v", "-boot", "-fakeiso", "-debug"],
+                ["powershell.exe",
+                 f"& \"{gk_path}\"",
+                 f"--config-path \"{config_path}\"",
+                 "--game jak1",
+                 "--", "-v", "-boot", "-fakeiso", "-debug"],
                 creationflags=subprocess.CREATE_NEW_CONSOLE)  # These need to be new consoles for stability.
 
-    if not goalc_running:
-        try:
-            goalc_path = Utils.get_settings()["jakanddaxter_options"]["root_directory"]
-            goalc_path = os.path.normpath(goalc_path)
-            goalc_path = os.path.join(goalc_path, "goalc.exe")
-        except AttributeError as e:
-            logger.error(f"Hosts.yaml does not contain {e.args[0]}, unable to locate game executables.")
-            return
-
-        if goalc_path:
+        if not goalc_running:
             # Prefixing ampersand and wrapping goalc_path in quotes is necessary for paths with spaces in them.
             goalc_process = subprocess.Popen(
                 ["powershell.exe", f"& \"{goalc_path}\"", "--game jak1"],
                 creationflags=subprocess.CREATE_NEW_CONSOLE)  # These need to be new consoles for stability.
 
+    except AttributeError as e:
+        ctx.on_log_error(logger, f"Host.yaml does not contain {e.args[0]}, unable to locate game executables.")
+        return
+    except FileNotFoundError as e:
+        msg = (f"The ArchipelaGOAL root directory path is invalid.\n"
+               f"Please check that the value of 'jakanddaxter_options > root_directory' in your host.yaml file "
+               f"is a valid existing path, and all backslashes have been replaced with forward slashes.")
+        ctx.on_log_error(logger, msg)
+        return
+
     # Auto connect the repl and memr agents. Sleep 5 because goalc takes just a little bit of time to load,
     # and it's not something we can await.
-    logger.info("This may take a bit... Wait for the success audio cue before continuing!")
+    ctx.on_log_info(logger, "This may take a bit... Wait for the game's title sequence before continuing!")
     await asyncio.sleep(5)
     ctx.repl.initiated_connect = True
     ctx.memr.initiated_connect = True
@@ -331,7 +432,7 @@ async def main():
     ctx.run_cli()
 
     # Find and run the game (gk) and compiler/repl (goalc).
-    await run_game(ctx)
+    create_task_log_exception(run_game(ctx))
     await ctx.exit_event.wait()
     await ctx.shutdown()
 

worlds/jakanddaxter/Items.py

@@ -16,26 +16,26 @@ cell_item_table = {
     0:  "Power Cell",
 }
 
-# Scout flies are interchangeable within their respective sets of 7. Notice the abbreviated level name after each item.
+# Scout flies are interchangeable within their respective sets of 7. Notice the level name after each item.
 # Also, notice that their Item ID equals their respective Power Cell's Location ID. This is necessary for
 # game<->archipelago communication.
 scout_item_table = {
-    95: "Scout Fly - GR",
-    75: "Scout Fly - SV",
-    7:  "Scout Fly - FJ",
-    20: "Scout Fly - SB",
-    28: "Scout Fly - MI",
-    68: "Scout Fly - FC",
-    76: "Scout Fly - RV",
-    57: "Scout Fly - PB",
-    49: "Scout Fly - LPC",
-    43: "Scout Fly - BS",
-    88: "Scout Fly - MP",
-    77: "Scout Fly - VC",
-    85: "Scout Fly - SC",
-    65: "Scout Fly - SM",
-    90: "Scout Fly - LT",
-    91: "Scout Fly - GMC",
+    95: "Scout Fly - Geyser Rock",
+    75: "Scout Fly - Sandover Village",
+    7:  "Scout Fly - Sentinel Beach",
+    20: "Scout Fly - Forbidden Jungle",
+    28: "Scout Fly - Misty Island",
+    68: "Scout Fly - Fire Canyon",
+    76: "Scout Fly - Rock Village",
+    57: "Scout Fly - Lost Precursor City",
+    49: "Scout Fly - Boggy Swamp",
+    43: "Scout Fly - Precursor Basin",
+    88: "Scout Fly - Mountain Pass",
+    77: "Scout Fly - Volcanic Crater",
+    85: "Scout Fly - Snowy Mountain",
+    65: "Scout Fly - Spider Cave",
+    90: "Scout Fly - Lava Tube",
+    91: "Scout Fly - Citadel",  # Had to shorten, it was >32 characters.
 }
 
 # Orbs are also generic and interchangeable.

worlds/jakanddaxter/client/MemoryReader.py

@@ -1,19 +1,23 @@
+import logging
 import random
 import struct
-from typing import ByteString, List, Callable
+from typing import ByteString, List, Callable, Optional
 import json
 import pymem
 from pymem import pattern
 from pymem.exception import ProcessNotFound, ProcessError, MemoryReadError, WinAPIError
 from dataclasses import dataclass
 
-from CommonClient import logger
 from ..locs import (OrbLocations as Orbs,
                     CellLocations as Cells,
                     ScoutLocations as Flies,
                     SpecialLocations as Specials,
                     OrbCacheLocations as Caches)
 
+
+logger = logging.getLogger("MemoryReader")
+
+
 # Some helpful constants.
 sizeof_uint64 = 8
 sizeof_uint32 = 4
@@ -21,6 +25,12 @@ sizeof_uint8 = 1
 sizeof_float = 4
 
 
+# *****************************************************************************
+# **** This number must match (-> *ap-info-jak1* version) in ap-struct.gc! ****
+# *****************************************************************************
+expected_memory_version = 2
+
+
 # IMPORTANT: OpenGOAL memory structures are particular about the alignment, in memory, of member elements according to
 # their size in bits. The address for an N-bit field must be divisible by N. Use this class to define the memory offsets
 # of important values in the struct. It will also do the byte alignment properly for you.
@@ -89,6 +99,12 @@ their_item_owner_offset = offsets.define(sizeof_uint8, 32)
 my_item_name_offset = offsets.define(sizeof_uint8, 32)
 my_item_finder_offset = offsets.define(sizeof_uint8, 32)
 
+# Version of the memory struct, to cut down on mod/apworld version mismatches.
+memory_version_offset = offsets.define(sizeof_uint32)
+
+# Connection status to AP server (not the game!)
+server_connection_offset = offsets.define(sizeof_uint8)
+
 # The End.
 end_marker_offset = offsets.define(sizeof_uint8, 4)
 
@@ -161,61 +177,94 @@ class JakAndDaxterMemoryReader:
     orbsanity_enabled: bool = False
     orbs_paid: int = 0
 
-    def __init__(self, marker: ByteString = b'UnLiStEdStRaTs_JaK1\x00'):
-        self.marker = marker
-        self.connect()
+    # Game-related callbacks (inform the AP server of changes to game state)
+    inform_checked_location: Callable
+    inform_finished_game: Callable
+    inform_died: Callable
+    inform_toggled_deathlink: Callable
+    inform_traded_orbs: Callable
 
-    async def main_tick(self,
-                        location_callback: Callable,
-                        finish_callback: Callable,
-                        deathlink_callback: Callable,
-                        deathlink_toggle: Callable,
-                        paid_orbs_callback: Callable):
+    # Logging callbacks
+    # These will write to the provided logger, as well as the Client GUI with color markup.
+    log_error: Callable    # Red
+    log_warn: Callable     # Orange
+    log_success: Callable  # Green
+    log_info: Callable     # White (default)
+
+    def __init__(self,
+                 location_check_callback: Callable,
+                 finish_game_callback: Callable,
+                 send_deathlink_callback: Callable,
+                 toggle_deathlink_callback: Callable,
+                 orb_trade_callback: Callable,
+                 log_error_callback: Callable,
+                 log_warn_callback: Callable,
+                 log_success_callback: Callable,
+                 log_info_callback: Callable,
+                 marker: ByteString = b'UnLiStEdStRaTs_JaK1\x00'):
+        self.marker = marker
+
+        self.inform_checked_location = location_check_callback
+        self.inform_finished_game = finish_game_callback
+        self.inform_died = send_deathlink_callback
+        self.inform_toggled_deathlink = toggle_deathlink_callback
+        self.inform_traded_orbs = orb_trade_callback
+
+        self.log_error = log_error_callback
+        self.log_warn = log_warn_callback
+        self.log_success = log_success_callback
+        self.log_info = log_info_callback
+
+    async def main_tick(self):
         if self.initiated_connect:
             await self.connect()
+            await self.verify_memory_version()
             self.initiated_connect = False
 
         if self.connected:
             try:
                 self.gk_process.read_bool(self.gk_process.base_address)  # Ping to see if it's alive.
             except (ProcessError, MemoryReadError, WinAPIError):
-                logger.error("The gk process has died. Restart the game and run \"/memr connect\" again.")
+                self.log_error(logger, "The gk process has died. Restart the game and run \"/memr connect\" again.")
                 self.connected = False
         else:
             return
 
-        # Save some state variables temporarily.
-        old_deathlink_enabled = self.deathlink_enabled
+        # TODO - How drastic of a change is this, to wrap all of main_tick in a self.connected check?
+        if self.connected:
 
-        # Read the memory address to check the state of the game.
-        self.read_memory()
+            # Save some state variables temporarily.
+            old_deathlink_enabled = self.deathlink_enabled
 
-        # Checked Locations in game. Handle the entire outbox every tick until we're up to speed.
-        if len(self.location_outbox) > self.outbox_index:
-            location_callback(self.location_outbox)
-            self.save_data()
-            self.outbox_index += 1
+            # Read the memory address to check the state of the game.
+            self.read_memory()
 
-        if self.finished_game:
-            finish_callback()
+            # Checked Locations in game. Handle the entire outbox every tick until we're up to speed.
+            if len(self.location_outbox) > self.outbox_index:
+                self.inform_checked_location(self.location_outbox)
+                self.save_data()
+                self.outbox_index += 1
 
-        if old_deathlink_enabled != self.deathlink_enabled:
-            deathlink_toggle()
-            logger.debug("Toggled DeathLink " + ("ON" if self.deathlink_enabled else "OFF"))
+            if self.finished_game:
+                self.inform_finished_game()
 
-        if self.send_deathlink:
-            deathlink_callback()
+            if old_deathlink_enabled != self.deathlink_enabled:
+                self.inform_toggled_deathlink()
+                logger.debug("Toggled DeathLink " + ("ON" if self.deathlink_enabled else "OFF"))
 
-        if self.orbs_paid > 0:
-            paid_orbs_callback(self.orbs_paid)
-            self.orbs_paid = 0
+            if self.send_deathlink:
+                self.inform_died()
+
+            if self.orbs_paid > 0:
+                self.inform_traded_orbs(self.orbs_paid)
+                self.orbs_paid = 0
 
     async def connect(self):
         try:
             self.gk_process = pymem.Pymem("gk.exe")  # The GOAL Kernel
-            logger.info("Found the gk process: " + str(self.gk_process.process_id))
+            logger.debug("Found the gk process: " + str(self.gk_process.process_id))
         except ProcessNotFound:
-            logger.error("Could not find the gk process.")
+            self.log_error(logger, "Could not find the gk process.")
             self.connected = False
             return
 
@@ -230,21 +279,45 @@ class JakAndDaxterMemoryReader:
             self.goal_address = int.from_bytes(self.gk_process.read_bytes(goal_pointer, sizeof_uint64),
                                                byteorder="little",
                                                signed=False)
-            logger.info("Found the archipelago memory address: " + str(self.goal_address))
+            logger.debug("Found the archipelago memory address: " + str(self.goal_address))
             self.connected = True
         else:
-            logger.error("Could not find the archipelago memory address.")
+            self.log_error(logger, "Could not find the archipelago memory address!")
             self.connected = False
 
-        if self.connected:
-            logger.info("The Memory Reader is ready!")
+    async def verify_memory_version(self):
+        if not self.connected:
+            self.log_error(logger, "The Memory Reader is not connected!")
 
-    def print_status(self):
-        logger.info("Memory Reader Status:")
-        logger.info("   Game process ID: " + (str(self.gk_process.process_id) if self.gk_process else "None"))
-        logger.info("   Game state memory address: " + str(self.goal_address))
-        logger.info("   Last location checked: " + (str(self.location_outbox[self.outbox_index - 1])
-                                                    if self.outbox_index else "None"))
+        memory_version: Optional[int] = None
+        try:
+            memory_version = self.read_goal_address(memory_version_offset, sizeof_uint32)
+            if memory_version == expected_memory_version:
+                self.log_success(logger, "The Memory Reader is ready!")
+            else:
+                raise MemoryReadError(memory_version_offset, sizeof_uint32)
+        except (ProcessError, MemoryReadError, WinAPIError):
+            msg = (f"The OpenGOAL memory structure is incompatible with the current Archipelago client!\n"
+                   f"   Expected Version: {str(expected_memory_version)}\n"
+                   f"   Found Version: {str(memory_version)}\n"
+                   f"Please follow these steps:\n"
+                   f"   Run the OpenGOAL Launcher, click Jak and Daxter > Features > Mods > ArchipelaGOAL.\n"
+                   f"   Click Update (if one is available).\n"
+                   f"   Click Advanced > Compile. When this is done, click Continue.\n"
+                   f"   Click Versions and verify the latest version is marked 'Active'.\n"
+                   f"   Close all launchers, games, clients, and Powershell windows, then restart Archipelago.")
+            self.log_error(logger, msg)
+            self.connected = False
+
+    async def print_status(self):
+        proc_id = str(self.gk_process.process_id) if self.gk_process else "None"
+        last_loc = str(self.location_outbox[self.outbox_index - 1] if self.outbox_index else "None")
+        msg = (f"Memory Reader Status:\n"
+               f"   Game process ID: {proc_id}\n"
+               f"   Game state memory address: {str(self.goal_address)}\n"
+               f"   Last location checked: {last_loc}")
+        await self.verify_memory_version()
+        self.log_info(logger, msg)
 
     def read_memory(self) -> List[int]:
         try:
@@ -350,10 +423,10 @@ class JakAndDaxterMemoryReader:
             completed = self.read_goal_address(completed_offset, sizeof_uint8)
             if completed > 0 and not self.finished_game:
                 self.finished_game = True
-                logger.info("Congratulations! You finished the game!")
+                self.log_success(logger, "Congratulations! You finished the game!")
 
         except (ProcessError, MemoryReadError, WinAPIError):
-            logger.error("The gk process has died. Restart the game and run \"/memr connect\" again.")
+            self.log_error(logger, "The gk process has died. Restart the game and run \"/memr connect\" again.")
             self.connected = False
 
         return self.location_outbox

worlds/jakanddaxter/client/ReplClient.py

@@ -1,11 +1,12 @@
 import json
+import logging
 import queue
 import time
 import struct
 import random
 from dataclasses import dataclass
 from queue import Queue
-from typing import Dict, Optional
+from typing import Dict, Optional, Callable
 
 import pymem
 from pymem.exception import ProcessNotFound, ProcessError
@@ -13,7 +14,6 @@ from pymem.exception import ProcessNotFound, ProcessError
 import asyncio
 from asyncio import StreamReader, StreamWriter, Lock
 
-from CommonClient import logger
 from NetUtils import NetworkItem
 from ..GameID import jak1_id, jak1_max
 from ..Items import item_table
@@ -25,6 +25,9 @@ from ..locs import (
     OrbCacheLocations as Caches)
 
 
+logger = logging.getLogger("ReplClient")
+
+
 @dataclass
 class JsonMessageData:
     my_item_name: Optional[str] = None
@@ -53,11 +56,27 @@ class JakAndDaxterReplClient:
     inbox_index = 0
     json_message_queue: Queue[JsonMessageData] = queue.Queue()
 
-    def __init__(self, ip: str = "127.0.0.1", port: int = 8181):
+    # Logging callbacks
+    # These will write to the provided logger, as well as the Client GUI with color markup.
+    log_error: Callable    # Red
+    log_warn: Callable     # Orange
+    log_success: Callable  # Green
+    log_info: Callable     # White (default)
+
+    def __init__(self,
+                 log_error_callback: Callable,
+                 log_warn_callback: Callable,
+                 log_success_callback: Callable,
+                 log_info_callback: Callable,
+                 ip: str = "127.0.0.1",
+                 port: int = 8181):
         self.ip = ip
         self.port = port
         self.lock = asyncio.Lock()
-        self.connect()
+        self.log_error = log_error_callback
+        self.log_warn = log_warn_callback
+        self.log_success = log_success_callback
+        self.log_info = log_info_callback
 
     async def main_tick(self):
         if self.initiated_connect:
@@ -68,12 +87,13 @@ class JakAndDaxterReplClient:
             try:
                 self.gk_process.read_bool(self.gk_process.base_address)  # Ping to see if it's alive.
             except ProcessError:
-                logger.error("The gk process has died. Restart the game and run \"/repl connect\" again.")
+                self.log_error(logger, "The gk process has died. Restart the game and run \"/repl connect\" again.")
                 self.connected = False
             try:
                 self.goalc_process.read_bool(self.goalc_process.base_address)  # Ping to see if it's alive.
             except ProcessError:
-                logger.error("The goalc process has died. Restart the compiler and run \"/repl connect\" again.")
+                self.log_error(logger,
+                               "The goalc process has died. Restart the compiler and run \"/repl connect\" again.")
                 self.connected = False
         else:
             return
@@ -111,22 +131,22 @@ class JakAndDaxterReplClient:
                     logger.debug(response)
                 return True
             else:
-                logger.error(f"Unexpected response from REPL: {response}")
+                self.log_error(logger, f"Unexpected response from REPL: {response}")
                 return False
 
     async def connect(self):
         try:
             self.gk_process = pymem.Pymem("gk.exe")  # The GOAL Kernel
-            logger.info("Found the gk process: " + str(self.gk_process.process_id))
+            logger.debug("Found the gk process: " + str(self.gk_process.process_id))
         except ProcessNotFound:
-            logger.error("Could not find the gk process.")
+            self.log_error(logger, "Could not find the gk process.")
             return
 
         try:
             self.goalc_process = pymem.Pymem("goalc.exe")  # The GOAL Compiler and REPL
-            logger.info("Found the goalc process: " + str(self.goalc_process.process_id))
+            logger.debug("Found the goalc process: " + str(self.goalc_process.process_id))
         except ProcessNotFound:
-            logger.error("Could not find the goalc process.")
+            self.log_error(logger, "Could not find the goalc process.")
             return
 
         try:
@@ -139,9 +159,10 @@ class JakAndDaxterReplClient:
             if "Connected to OpenGOAL" and "nREPL!" in welcome_message:
                 logger.debug(welcome_message)
             else:
-                logger.error(f"Unable to connect to REPL websocket: unexpected welcome message \"{welcome_message}\"")
+                self.log_error(logger,
+                               f"Unable to connect to REPL websocket: unexpected welcome message \"{welcome_message}\"")
         except ConnectionRefusedError as e:
-            logger.error(f"Unable to connect to REPL websocket: {e.strerror}")
+            self.log_error(logger, f"Unable to connect to REPL websocket: {e.strerror}")
             return
 
         ok_count = 0
@@ -175,7 +196,7 @@ class JakAndDaxterReplClient:
             if await self.send_form("(set! *cheat-mode* #f)", print_ok=False):
                 ok_count += 1
 
-            # Run the retail game start sequence (while still in debug).
+            # Run the retail game start sequence (while still connected with REPL).
             if await self.send_form("(start \'play (get-continue-by-name *game-info* \"title-start\"))"):
                 ok_count += 1
 
@@ -186,25 +207,29 @@ class JakAndDaxterReplClient:
                 self.connected = False
 
         if self.connected:
-            logger.info("The REPL is ready!")
+            self.log_success(logger, "The REPL is ready!")
 
     async def print_status(self):
-        logger.info("REPL Status:")
-        logger.info("  REPL process ID: " + (str(self.goalc_process.process_id) if self.goalc_process else "None"))
-        logger.info("  Game process ID: " + (str(self.gk_process.process_id) if self.gk_process else "None"))
+        gc_proc_id = str(self.goalc_process.process_id) if self.goalc_process else "None"
+        gk_proc_id = str(self.gk_process.process_id) if self.gk_process else "None"
+        msg = (f"REPL Status:\n"
+               f"   REPL process ID: {gc_proc_id}\n"
+               f"   Game process ID: {gk_proc_id}\n")
         try:
             if self.reader and self.writer:
                 addr = self.writer.get_extra_info("peername")
-                logger.info("  Game websocket: " + (str(addr) if addr else "None"))
+                addr = str(addr) if addr else "None"
+                msg += f"   Game websocket: {addr}\n"
                 await self.send_form("(dotimes (i 1) "
                                      "(sound-play-by-name "
                                      "(static-sound-name \"menu-close\") "
                                      "(new-sound-id) 1024 0 0 (sound-group sfx) #t))", print_ok=False)
         except ConnectionResetError:
-            logger.warn("  Connection to the game was lost or reset!")
-        logger.info("  Did you hear the success audio cue?")
-        logger.info("  Last item received: " + (str(getattr(self.item_inbox[self.inbox_index], "item"))
-                                                if self.inbox_index else "None"))
+            msg += f"   Connection to the game was lost or reset!"
+        last_item = str(getattr(self.item_inbox[self.inbox_index], "item")) if self.inbox_index else "None"
+        msg += f"   Last item received: {last_item}\n"
+        msg += f"   Did you hear the success audio cue?"
+        self.log_info(logger, msg)
 
     # To properly display in-game text, it must be alphanumeric and uppercase.
     # I also only allotted 32 bytes to each string in OpenGOAL, so we must truncate.
@@ -255,7 +280,7 @@ class JakAndDaxterReplClient:
         elif ap_id == jak1_max:
             await self.receive_green_eco()  # Ponder why I chose to do ID's this way.
         else:
-            raise KeyError(f"Tried to receive item with unknown AP ID {ap_id}.")
+            self.log_error(logger, f"Tried to receive item with unknown AP ID {ap_id}!")
 
     async def receive_power_cell(self, ap_id: int) -> bool:
         cell_id = Cells.to_game_id(ap_id)
@@ -266,7 +291,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received a Power Cell!")
         else:
-            logger.error(f"Unable to receive a Power Cell!")
+            self.log_error(logger, f"Unable to receive a Power Cell!")
         return ok
 
     async def receive_scout_fly(self, ap_id: int) -> bool:
@@ -278,7 +303,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received a {item_table[ap_id]}!")
         else:
-            logger.error(f"Unable to receive a {item_table[ap_id]}!")
+            self.log_error(logger, f"Unable to receive a {item_table[ap_id]}!")
         return ok
 
     async def receive_special(self, ap_id: int) -> bool:
@@ -290,7 +315,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received special unlock {item_table[ap_id]}!")
         else:
-            logger.error(f"Unable to receive special unlock {item_table[ap_id]}!")
+            self.log_error(logger, f"Unable to receive special unlock {item_table[ap_id]}!")
         return ok
 
     async def receive_move(self, ap_id: int) -> bool:
@@ -302,7 +327,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received the ability to {item_table[ap_id]}!")
         else:
-            logger.error(f"Unable to receive the ability to {item_table[ap_id]}!")
+            self.log_error(logger, f"Unable to receive the ability to {item_table[ap_id]}!")
         return ok
 
     async def receive_precursor_orb(self, ap_id: int) -> bool:
@@ -314,7 +339,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received {orb_amount} Precursor Orbs!")
         else:
-            logger.error(f"Unable to receive {orb_amount} Precursor Orbs!")
+            self.log_error(logger, f"Unable to receive {orb_amount} Precursor Orbs!")
         return ok
 
     # Green eco pills are our filler item. Use the get-pickup event instead to handle being full health.
@@ -323,7 +348,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received a green eco pill!")
         else:
-            logger.error(f"Unable to receive a green eco pill!")
+            self.log_error(logger, f"Unable to receive a green eco pill!")
         return ok
 
     async def receive_deathlink(self) -> bool:
@@ -343,7 +368,7 @@ class JakAndDaxterReplClient:
         if ok:
             logger.debug(f"Received deathlink signal!")
         else:
-            logger.error(f"Unable to receive deathlink signal!")
+            self.log_error(logger, f"Unable to receive deathlink signal!")
         return ok
 
     async def subtract_traded_orbs(self, orb_count: int) -> bool:
@@ -357,11 +382,15 @@ class JakAndDaxterReplClient:
             if ok:
                 logger.debug(f"Subtracting {orb_count} traded orbs!")
             else:
-                logger.error(f"Unable to subtract {orb_count} traded orbs!")
+                self.log_error(logger, f"Unable to subtract {orb_count} traded orbs!")
             return ok
 
         return True
 
+    # OpenGOAL has a limit of 8 parameters per function. We've already hit this limit. We may have to split these
+    # options into two groups, both of which required to be sent successfully, in the future.
+    # TODO - Alternatively, define a new datatype in OpenGOAL that holds all these options, instantiate the type here,
+    #  and rewrite the ap-setup-options! function to take that instance as input.
     async def setup_options(self,
                             os_option: int, os_bundle: int,
                             fc_count: int, mp_count: int,
@@ -373,14 +402,23 @@ class JakAndDaxterReplClient:
                                   f"(the float {lt_count}) (the float {ct_amount}) "
                                   f"(the float {ot_amount}) (the uint {goal_id}))")
         message = (f"Setting options: \n"
-                   f"    Orbsanity Option {os_option}, Orbsanity Bundle {os_bundle}, \n"
-                   f"    FC Cell Count {fc_count}, MP Cell Count {mp_count}, \n"
-                   f"    LT Cell Count {lt_count}, Citizen Orb Amt {ct_amount}, \n"
-                   f"    Oracle Orb Amt {ot_amount}, Completion GOAL {goal_id}... ")
+                   f"   Orbsanity Option {os_option}, Orbsanity Bundle {os_bundle}, \n"
+                   f"   FC Cell Count {fc_count}, MP Cell Count {mp_count}, \n"
+                   f"   LT Cell Count {lt_count}, Citizen Orb Amt {ct_amount}, \n"
+                   f"   Oracle Orb Amt {ot_amount}, Completion GOAL {goal_id}... ")
         if ok:
             logger.debug(message + "Success!")
+            status = 1
         else:
-            logger.error(message + "Failed!")
+            self.log_error(logger, message + "Failed!")
+            status = 2
+
+        ok = await self.send_form(f"(ap-set-connection-status! (the uint {status}))")
+        if ok:
+            logger.debug(f"Connection Status {status} set!")
+        else:
+            self.log_error(logger, f"Connection Status {status} failed to set!")
+
         return ok
 
     async def save_data(self):