-
Notifications
You must be signed in to change notification settings - Fork 704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Launcher: explicitly handle cli arguments to be passed to the Component #3714
Changes from 2 commits
20628b8
d886aa9
771512a
535e9c6
2ad254b
30d1edb
8a75f16
fac462e
1ca4bf9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -322,15 +322,28 @@ def main(args: Optional[Union[argparse.Namespace, dict]] = None): | |
init_logging('Launcher') | ||
Utils.freeze_support() | ||
multiprocessing.set_start_method("spawn") # if launched process uses kivy, fork won't work | ||
parser = argparse.ArgumentParser(description='Archipelago Launcher') | ||
parser = argparse.ArgumentParser( | ||
description='Archipelago Launcher', | ||
usage="[-h] [--update_settings] [Patch|Game|Component] [-- component args here]" | ||
) | ||
run_group = parser.add_argument_group("Run") | ||
run_group.add_argument("--update_settings", action="store_true", | ||
help="Update host.yaml and exit.") | ||
run_group.add_argument("Patch|Game|Component", type=str, nargs="?", | ||
help="Pass either a patch file, a generated game or the name of a component to run.") | ||
run_group.add_argument("args", nargs="*", | ||
help="Arguments to pass to component.") | ||
main(parser.parse_args()) | ||
run_group.add_argument("--", | ||
help="Arguments to pass to component.", | ||
dest="args", default=[]) | ||
args = sys.argv[1:] | ||
if "--" in args: | ||
i = args.index("--") | ||
passthrough_args = args[i+1:] | ||
args = args[:i] | ||
args = parser.parse_args(args) | ||
args.args = passthrough_args | ||
else: | ||
args = parser.parse_args() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this part necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think i figured out what the issue with this was (after reverting the
this is not a hill i'm willing to die on though, tested the rollback on these lines (and the 'args' arg) in source and frozen and the only scenarios that didn't work as expected were negative tests that just had invalid args ignored ex. will commit if you confirm this is good enough:tm: and a worthy downside to get rid of the manual parsing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. got the confirmation in discord |
||
main(args) | ||
|
||
from worlds.LauncherComponents import processes | ||
for process in processes: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when i was testing this locally
args
should work just fine here since if no args are passed it's an empty tuple. unless this is different between versions, since i only tested on 3.11.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it didn't work when i tried but i can retest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, dropping to just parse_args(args) does not work when using
python CommonClient.py args
potentially we can pass something into run_as_textclient() but the things i tested all didn't pass None into parse_args() correctly