search

ArchipelagoMW / Archipelago

Archipelago Multi-Game Randomizer and Server
https://archipelago.gg
Other
480 stars 638 forks source link

BizHawkClient: Avoid error launching BizHawkClient via Launcher CLI #3554

Closed remyjette closed 2 weeks ago

remyjette commented 3 months ago

What is this fixing or adding?

Today the BizHawk Client calls argparse.ArgumentParser.parse_args with no arguments, so it parses the entirety of sys.argv. If BizHawkClient is invoked via the Launcher's command line (python3 Launcher.py BizHawkClient), this means the user will see an error on launch:

NotImplementedError: No Handler for BizHawkClient found.

If the user tries to pass a patch file (for example python3 Launcher.py BizHawkClient patch.apemerald) it won't start at all, failing to start with

usage: Launcher.py [-h] [--connect CONNECT] [--password PASSWORD] [--nogui] [patch_file]
Launcher.py: error: unrecognized arguments: patch.apemerald

The reason I noticed this I have a shortcut to the .AppImage with BizHawkClient as an argument (since there's no equivalent to ArchipelagoBizHawkClient.exe using the image), and I saw the above NotImplementedError every time I launched. It didn't prevent gameplay (this is not a blocker) but it would be nicer to not have the error.

The fix:

The Launcher already passes the args to the component's registered func; the BizHawkClient's launch and launch_client functions just needed to forward those args along and use those instead of parsing from sys.argv[1:] (the default behavior of parse_args).

How was this tested?

Tested both before and after my change, testing Launcher.py BizHawkClient, Launcher.py patch.emerald, Launcher.py BizHawkClient patch.apemerald, BizHawkClient.py, BizHawkClient.py patch.apemerald, and using the "Open Patch" button in the launcher.

remyjette commented 2 months ago

I agree. I'll keep this PR for the BizHawkClient changes but revert the LauncherComponents.py changes in favor of #3714.

remyjette commented 3 weeks ago

Re-tested everything after merging in main now that #3714 merged - this issue with BizHawkClient is still in main, and this PR still fixes the issue. It's now ready for review.