CommonClient: Explicitly parse url arg as an archipelago:// url

[qwint]

What is this fixing or adding?

stops CommonClient from parsing the url arg when the value is not of format archipelago://localhost:port including fixing a current issue where the component name could override values in the --connect arg

## What is this fixing or adding?
Launcher "Text Client" --connect archipelago.gg:38281
should work, it doesn't, this fixes that

I don't particularly love the contents of the fix because I believe the "args" argument in the launcher parser is supposed to do this job, but I got no other input on discord so I'm hoping to get better input here

## How was this tested?
ran Python Launcher.py "Text Client" --connect archipelago.gg:38281 and saw it open the text client, connect to the address, and ask for slot name
also tested with rooms i had open using slotname:password@server:port and it automatically connected without issue

## If this makes graphical changes, please attach screenshots.

[ScipioWright]

Changes are small and function properly. The change in Launcher.py does not cause errors in args to go unnoticed, since the errors are caught somewhere else.

[gaithern]

Tested some scenarios with this change on an instance on my end. Everything looks good:

python Launcher.py "Text Client" --connect archipelago.gg:60157

• Correctly opens the client, connects, and prompts for slot name

python Launcher.py "Text Client" --connect somejunk

• Correctly opens the text client and informs user that the client failed to find a multiworld server

python Launcher.py "Text Client" --some_junk_arg

• Correctly handles the bad argument, giving the message "Launcher.py: error: unrecognized arguments: --some_junk_arg"

python Launcher.py "Text Client" --connect 127.0.0.1:38281

• Correctly opens the client, connects, and prompts user for slot name

[NewSoupVi]

Appears to be superceded by #3714. Let me know if this was a mistake.

[qwint]

this part is still needed, if that's rolled into a different PR or this PR is reopened removing the Launcher.py changes idc

[NewSoupVi]

I'll reopen this and look at it soon, what exactly does this part do on its own?

[qwint]

currently argparse grabs the first positional arg and throws it into url, that's to be expected but does mean that launcher "text client" puts the component name in the url block, this causes issues because the highlighted code block only checks if there is a url arg before overriding things

the updated code just confirms that the scheme of the url is correct (i.e. archipelago://blah@blah) before applying any of the available values to the ctx to stop mishandling of any potential bad data

[NewSoupVi]

Hmmm wait, so then wouldn't this break python CommonClient.py archipelago.gg:38281 --name Player1?

[NewSoupVi]

Oh I'm stupid

[NewSoupVi]

Oh I see. That just doesn't even work at all. I thought that was something that was supposed to work, but I guess not

[NewSoupVi]

Ok, well, then this change is correct, but now I'm wondering if launching CommonClient is super broken rn

[NewSoupVi]

Ok, after some conversation on Discord, while this doesn't make anything worse or anything, I'm not quite sure whether this is the correct approach or whether it is complete.

In my opinion, python CommonClient.py archipelago.gg:38281 should work. I know that technically it should be //archipelago.gg:38281, but in my opinion, the first arg should just be interpreted as the url and the archipelago:// scheme case should be a special case

I would basically add an

if url.scheme == "archipelago":
    ...
else:
    # Just use whatever the user put in as the connect url directly
    args.connect = args.url

at the end here.

If we really think that python CommonClient.py archipelago.gg:38281 shouldn't work, then it should at least output a warning I think. As a user, I'd absolutely expect this to work, and if it doesn't, I would not know why unless being told.

If there are cases where "supported uses" of other components (such as Launcher) pass bad values to CommonClient that shouldn't be interpreted as a url, then we should fix those cases. If it's user error (i.e. they used Launcher args in an incorrect way, which caused something weird to be passed to CommonClient as the url), the most I would do is add warnings.