Skip to content
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: Allow opening patches for clients without an exe #2176

Merged
merged 2 commits into from
Oct 2, 2023

Conversation

Zunawe
Copy link
Collaborator

@Zunawe Zunawe commented Sep 14, 2023

What is this fixing or adding?

Right now the "Open Patch" button in the launcher is restricted to only recognize file extensions associated with root-level clients. So the only way for an apworld with a bespoke internal client to open a patch file is to open it directly with the launcher.

This removes the exe requirement, and if no exe is found after the file is identified, the file is given directly to the launcher, which should identify the patch file and run component.func in run_component.

How was this tested?

Dropped a Pokemon Emerald apworld into worlds/ and was able to open an apemerald patch file to launch the Emerald client that's inside the apworld. Also verified that an apz5 file still opened and patched correctly. All tested on source only.

@black-sliver
Copy link
Member

hm, the isfile(get_exe(c)[-1]) was because you can skip installation of clients during the setup. Those won't show up in the open_patch then - this is probably still relevant for clients that live outside of their worlds?

@Zunawe
Copy link
Collaborator Author

Zunawe commented Sep 15, 2023

Ahh, right. Maybe something like

        if c.type == Type.CLIENT and \
                isinstance(c.file_identifier, SuffixIdentifier) and \
                (c.script_name is None or isfile(get_exe(c)[-1])):
            suffixes += c.file_identifier.suffixes

If you have a "script name", assume that means you require a root level client to be installed. Otherwise, assume you won't have a root level client and need to whitelist your suffixes here so the launcher can handle them.

@black-sliver
Copy link
Member

Yeah, something like that should work

@Zunawe
Copy link
Collaborator Author

Zunawe commented Sep 16, 2023

Updated. Tested a frozen 3.11 build this time and everything worked as expected. Deleting a couple root-level clients resulted in their suffixes being removed from the Open Patch window.

Copy link
Member

@black-sliver black-sliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@black-sliver black-sliver merged commit 24403eb into ArchipelagoMW:main Oct 2, 2023
12 checks passed
@ThePhar ThePhar added is: enhancement Issues requesting new features or pull requests implementing new features. affects: core Issues/PRs that touch core and may need additional validation. labels Oct 16, 2023
@Zunawe Zunawe deleted the open-patch branch October 29, 2023 00:47
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
…goMW#2176)

* Launcher: Allow opening patches for clients without an exe

* Launcher: Restore behavior for not showing patch suffixes for clients that aren't installed
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
…goMW#2176)

* Launcher: Allow opening patches for clients without an exe

* Launcher: Restore behavior for not showing patch suffixes for clients that aren't installed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: core Issues/PRs that touch core and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants