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

Core: refactor some loading mechanisms #1753

Merged
merged 9 commits into from
Jun 19, 2023
Merged

Core: refactor some loading mechanisms #1753

merged 9 commits into from
Jun 19, 2023

Conversation

Berserker66
Copy link
Member

What is this fixing or adding?

on its own, not much

How was this tested?

together with a new component that comes seperate

If this makes graphical changes, please attach screenshots.

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.

This breaks the following use-case:
python Launcher.py Generate

@black-sliver
Copy link
Member

I think the new code handles all use-cases we now have, with one exception:
Launching it with an invalid argument would not open to GUI previously. I don't have a strong opinion, but printing a message would be nice (in addition to opening the GUI), I think

With the current refactor, I think identify can be changed to only return 2 values - the original code would allow launching everything from the AP folder iirc, but that's not required anymore.

@Berserker66
Copy link
Member Author

It is reduced to returning 2 values already, but the change in the branch is not showing up in this PR.

@Berserker66
Copy link
Member Author

well, merging main into the branch again seems to have refreshed github

@black-sliver
Copy link
Member

Do we want to log/print/show a message if the argument was ignored/not found? Once we decided that, I think this can be merged

Launcher.py Outdated Show resolved Hide resolved
Co-authored-by: black-sliver <[email protected]>
Launcher.py Outdated Show resolved Hide resolved
Co-authored-by: black-sliver <[email protected]>
@ThePhar ThePhar added is: refactor/cleanup Improvements to code/output readability or organizization. affects: core Issues/PRs that touch core and may need additional validation. labels May 31, 2023
@Berserker66 Berserker66 merged commit 61fc805 into main Jun 19, 2023
@Berserker66 Berserker66 deleted the load_refactor branch June 19, 2023 23:01
Berserker66 added a commit that referenced this pull request Jun 20, 2023
Witchybun pushed a commit to Witchybun/Archipelago that referenced this pull request Jun 26, 2023
FlySniper pushed a commit to FlySniper/Archipelago that referenced this pull request Nov 14, 2023
Jouramie pushed a commit to Jouramie/Archipelago that referenced this pull request Feb 28, 2024
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: refactor/cleanup Improvements to code/output readability or organizization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants