From 0d38b415404bca2b5bcb7bca4ef4c2c7ca43d0b6 Mon Sep 17 00:00:00 2001 From: Bryce Wilson Date: Thu, 23 Nov 2023 06:00:46 -0800 Subject: [PATCH] BizHawkClient: Add support for multiple concurrent instances (#2475) This allows multiple client/connector pairs to run at the same time. It also includes a few other miscellaneous small changes that accumulated as I went. They can be split if desired - Whatever the `client_socket:send` line (~440) was doing with that missing operator, it's no longer doing. Don't ask me how it was working before. Lua is witchcraft. - Removed the `settimeout(2)` which causes the infamous emulator freeze (and replaced it with a `settimeout(0)` when the server socket is created). It appears to be unnecessary to set a timeout for discovering a client. Maybe at some point in time it was useful to keep the success rate for connecting high, but it seems to not be a problem if the timeout is 0 instead. - Also updated the Emerald setup to remove mention of the freezing. - Connector script now picks the first port that's not in use in a range of 5 ports. - To summarize why I was previously under the impression that multiple running scripts would not detect when a port was in use: 1. Calling `socket.bind` in the existing script will first create an ipv6 socket. 2. A second concurrent script trying to bind to the same port would I think fail to create an ipv6 socket but then succeed in creating an ipv4 socket on the same port. 3. That second socket could never communicate with a client; extra clients would just bounce off the first script. 4. The third concurrent script will then fail on both and actually give an `address already in use` error. - I'm not _really_ sure what's going on there. But forcing one or the other by calling `socket.tcp4()` or `socket.tcp6()` means that only one script will believe it has the port while any others will give `address already in use` as you'd expect. - As a side note, our `socket.lua` is much wonkier than I had previously thought. I understand some parts were added for LADX and when BizHawk 2.9 came out, but as far back as the file's history in this repo, it has provided a strange, modified interface as compared to the file it was originally derived from, to no benefit as far as I can tell. - The connector script closes `server` once it finds a client and opens a new one if the connection drops. I'm not sure this ultimately has an effect, but it seems more proper. - If the connector script's main function returns because of some error or refusal to proceed, the script no longer tries to resume the coroutine it was part of, which would flood the log with irrelevant errors. - Creating `SyncError`s in `guarded_read` and `guarded_write` would raise its own error because the wrong variable was being used in its message. - A call to `_bizhawk.connect` can take a while as the client tries the possible ports. There's a modification that will wait on either the `connect` or the exit event. And if the exit event fires while still looking for a connector script, this cancels the `connect` so the window can close. - Related: It takes 2-3 seconds for a call to `asyncio.open_connection` to come back with any sort of response on my machine, which can be significant now that we're trying multiple ports in sequence. I guess it could fire off 5 tasks at once. Might cause some weirdness if there exist multiple scripts and multiple clients looking for each other at the same time. - Also related: The first time a client attempts to connect to a script, they accept each other and start communicating as expected. The second client to try that port seems to believe it connects and will then time out on the first message. And then all subsequent attempts to connect to that port by any client will be refused (as expected) until the script shuts down or restarts. I haven't been able to explain this behavior. It adds more time to a client's search for a script, but doesn't ultimately cause problems. --- data/lua/connector_bizhawk_generic.lua | 99 ++++++++++++++++--------- worlds/_bizhawk/__init__.py | 36 ++++++--- worlds/_bizhawk/context.py | 13 +++- worlds/pokemon_emerald/docs/setup_en.md | 4 +- 4 files changed, 104 insertions(+), 48 deletions(-) diff --git a/data/lua/connector_bizhawk_generic.lua b/data/lua/connector_bizhawk_generic.lua index b0b06de4..c4e72930 100644 --- a/data/lua/connector_bizhawk_generic.lua +++ b/data/lua/connector_bizhawk_generic.lua @@ -249,6 +249,24 @@ Response: - `err` (`string`): A description of the problem ]] +local bizhawk_version = client.getversion() +local bizhawk_major, bizhawk_minor, bizhawk_patch = bizhawk_version:match("(%d+)%.(%d+)%.?(%d*)") +bizhawk_major = tonumber(bizhawk_major) +bizhawk_minor = tonumber(bizhawk_minor) +if bizhawk_patch == "" then + bizhawk_patch = 0 +else + bizhawk_patch = tonumber(bizhawk_patch) +end + +local lua_major, lua_minor = _VERSION:match("Lua (%d+)%.(%d+)") +lua_major = tonumber(lua_major) +lua_minor = tonumber(lua_minor) + +if lua_major > 5 or (lua_major == 5 and lua_minor >= 3) then + require("lua_5_3_compat") +end + local base64 = require("base64") local socket = require("socket") local json = require("json") @@ -257,7 +275,9 @@ local json = require("json") -- Will cause lag due to large console output local DEBUG = false -local SOCKET_PORT = 43055 +local SOCKET_PORT_FIRST = 43055 +local SOCKET_PORT_RANGE_SIZE = 5 +local SOCKET_PORT_LAST = SOCKET_PORT_FIRST + SOCKET_PORT_RANGE_SIZE local STATE_NOT_CONNECTED = 0 local STATE_CONNECTED = 1 @@ -277,24 +297,6 @@ local locked = false local rom_hash = nil -local lua_major, lua_minor = _VERSION:match("Lua (%d+)%.(%d+)") -lua_major = tonumber(lua_major) -lua_minor = tonumber(lua_minor) - -if lua_major > 5 or (lua_major == 5 and lua_minor >= 3) then - require("lua_5_3_compat") -end - -local bizhawk_version = client.getversion() -local bizhawk_major, bizhawk_minor, bizhawk_patch = bizhawk_version:match("(%d+)%.(%d+)%.?(%d*)") -bizhawk_major = tonumber(bizhawk_major) -bizhawk_minor = tonumber(bizhawk_minor) -if bizhawk_patch == "" then - bizhawk_patch = 0 -else - bizhawk_patch = tonumber(bizhawk_patch) -end - function queue_push (self, value) self[self.right] = value self.right = self.right + 1 @@ -435,7 +437,7 @@ function send_receive () end if message == "VERSION" then - local result, err client_socket:send(tostring(SCRIPT_VERSION).."\n") + client_socket:send(tostring(SCRIPT_VERSION).."\n") else local res = {} local data = json.decode(message) @@ -463,14 +465,45 @@ function send_receive () end end -function main () - server, err = socket.bind("localhost", SOCKET_PORT) +function initialize_server () + local err + local port = SOCKET_PORT_FIRST + local res = nil + + server, err = socket.socket.tcp4() + while res == nil and port <= SOCKET_PORT_LAST do + res, err = server:bind("localhost", port) + if res == nil and err ~= "address already in use" then + print(err) + return + end + + if res == nil then + port = port + 1 + end + end + + if port > SOCKET_PORT_LAST then + print("Too many instances of connector script already running. Exiting.") + return + end + + res, err = server:listen(0) + if err ~= nil then print(err) return end + server:settimeout(0) +end + +function main () while true do + if server == nil then + initialize_server() + end + current_time = socket.socket.gettime() timeout_timer = timeout_timer - (current_time - prev_time) message_timer = message_timer - (current_time - prev_time) @@ -482,16 +515,16 @@ function main () end if current_state == STATE_NOT_CONNECTED then - if emu.framecount() % 60 == 0 then - server:settimeout(2) + if emu.framecount() % 30 == 0 then + print("Looking for client...") local client, timeout = server:accept() if timeout == nil then print("Client connected") current_state = STATE_CONNECTED client_socket = client + server:close() + server = nil client_socket:settimeout(0) - else - print("No client found. Trying again...") end end else @@ -527,27 +560,27 @@ else emu.frameadvance() end end - + rom_hash = gameinfo.getromhash() - print("Waiting for client to connect. Emulation will freeze intermittently until a client is found.\n") + print("Waiting for client to connect. This may take longer the more instances of this script you have open at once.\n") local co = coroutine.create(main) function tick () local status, err = coroutine.resume(co) - - if not status then + + if not status and err ~= "cannot resume dead coroutine" then print("\nERROR: "..err) print("Consider reporting this crash.\n") if server ~= nil then server:close() end - + co = coroutine.create(main) end end - + -- Gambatte has a setting which can cause script execution to become -- misaligned, so for GB and GBC we explicitly set the callback on -- vblank instead. @@ -557,7 +590,7 @@ else else event.onframeend(tick) end - + while true do emu.frameadvance() end diff --git a/worlds/_bizhawk/__init__.py b/worlds/_bizhawk/__init__.py index 34039908..c3314e18 100644 --- a/worlds/_bizhawk/__init__.py +++ b/worlds/_bizhawk/__init__.py @@ -12,7 +12,8 @@ import json import typing -BIZHAWK_SOCKET_PORT = 43055 +BIZHAWK_SOCKET_PORT_RANGE_START = 43055 +BIZHAWK_SOCKET_PORT_RANGE_SIZE = 5 class ConnectionStatus(enum.IntEnum): @@ -45,11 +46,13 @@ class BizHawkContext: streams: typing.Optional[typing.Tuple[asyncio.StreamReader, asyncio.StreamWriter]] connection_status: ConnectionStatus _lock: asyncio.Lock + _port: typing.Optional[int] def __init__(self) -> None: self.streams = None self.connection_status = ConnectionStatus.NOT_CONNECTED self._lock = asyncio.Lock() + self._port = None async def _send_message(self, message: str): async with self._lock: @@ -86,15 +89,24 @@ class BizHawkContext: async def connect(ctx: BizHawkContext) -> bool: - """Attempts to establish a connection with the connector script. Returns True if successful.""" - try: - ctx.streams = await asyncio.open_connection("localhost", BIZHAWK_SOCKET_PORT) - ctx.connection_status = ConnectionStatus.TENTATIVE - return True - except (TimeoutError, ConnectionRefusedError): - ctx.streams = None - ctx.connection_status = ConnectionStatus.NOT_CONNECTED - return False + """Attempts to establish a connection with a connector script. Returns True if successful.""" + rotation_steps = 0 if ctx._port is None else ctx._port - BIZHAWK_SOCKET_PORT_RANGE_START + ports = [*range(BIZHAWK_SOCKET_PORT_RANGE_START, BIZHAWK_SOCKET_PORT_RANGE_START + BIZHAWK_SOCKET_PORT_RANGE_SIZE)] + ports = ports[rotation_steps:] + ports[:rotation_steps] + + for port in ports: + try: + ctx.streams = await asyncio.open_connection("localhost", port) + ctx.connection_status = ConnectionStatus.TENTATIVE + ctx._port = port + return True + except (TimeoutError, ConnectionRefusedError): + continue + + # No ports worked + ctx.streams = None + ctx.connection_status = ConnectionStatus.NOT_CONNECTED + return False def disconnect(ctx: BizHawkContext) -> None: @@ -233,7 +245,7 @@ async def guarded_read(ctx: BizHawkContext, read_list: typing.List[typing.Tuple[ return None else: if item["type"] != "READ_RESPONSE": - raise SyncError(f"Expected response of type READ_RESPONSE or GUARD_RESPONSE but got {res['type']}") + raise SyncError(f"Expected response of type READ_RESPONSE or GUARD_RESPONSE but got {item['type']}") ret.append(base64.b64decode(item["value"])) @@ -285,7 +297,7 @@ async def guarded_write(ctx: BizHawkContext, write_list: typing.List[typing.Tupl return False else: if item["type"] != "WRITE_RESPONSE": - raise SyncError(f"Expected response of type WRITE_RESPONSE or GUARD_RESPONSE but got {res['type']}") + raise SyncError(f"Expected response of type WRITE_RESPONSE or GUARD_RESPONSE but got {item['type']}") return True diff --git a/worlds/_bizhawk/context.py b/worlds/_bizhawk/context.py index ccf747f1..2699b0f5 100644 --- a/worlds/_bizhawk/context.py +++ b/worlds/_bizhawk/context.py @@ -130,7 +130,18 @@ async def _game_watcher(ctx: BizHawkClientContext): logger.info("Waiting to connect to BizHawk...") showed_connecting_message = True - if not await connect(ctx.bizhawk_ctx): + # Since a call to `connect` can take a while to return, this will cancel connecting + # if the user has decided to close the client. + connect_task = asyncio.create_task(connect(ctx.bizhawk_ctx), name="BizHawkConnect") + exit_task = asyncio.create_task(ctx.exit_event.wait(), name="ExitWait") + await asyncio.wait([connect_task, exit_task], return_when=asyncio.FIRST_COMPLETED) + + if exit_task.done(): + connect_task.cancel() + return + + if not connect_task.result(): + # Failed to connect continue showed_no_handler_message = False diff --git a/worlds/pokemon_emerald/docs/setup_en.md b/worlds/pokemon_emerald/docs/setup_en.md index 6a1df8e5..3c5c8c19 100644 --- a/worlds/pokemon_emerald/docs/setup_en.md +++ b/worlds/pokemon_emerald/docs/setup_en.md @@ -52,8 +52,8 @@ you can re-open it from the launcher. 3. In EmuHawk, go to `Tools > Lua Console`. This window must stay open while playing. 4. In the Lua Console window, go to `Script > Open Script…`. 5. Navigate to your Archipelago install folder and open `data/lua/connector_bizhawk_generic.lua`. -6. The emulator may freeze every few seconds until it manages to connect to the client. This is expected. The BizHawk -Client window should indicate that it connected and recognized Pokemon Emerald. +6. The emulator and client will eventually connect to each other. The BizHawk Client window should indicate that it +connected and recognized Pokemon Emerald. 7. To connect the client to the server, enter your room's address and port (e.g. `archipelago.gg:38281`) into the top text field of the client and click Connect.