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

OOT: Adjust the Logic Trick Keys to be an ordered object #2736

Merged
merged 11 commits into from
Jan 18, 2024

Conversation

Bicoloursnake
Copy link
Contributor

Hey wait a minute, this isn't (directly) a documentation PR, who let me touch actual code?

What is this fixing or adding?

In current main, the logic tricks in OOT end up as frozenset object, so they don't maintain the desired order. This PR instead has them as a tuple, in order to maintain the immutability of frozenset, while keeping the desired order of objects

How was this tested?

Several local generations and spot checking the webhost, in addition to the standard unit tests.

If this makes graphical changes, please attach screenshots.

Before:
image

After:
image

Bicoloursnake and others added 11 commits April 20, 2023 21:01
A few hours ago, I tested whether the client would run successfully on macOS (Send/Receive items, load maps, download maps, etc.). After the successful testing, I thought adding some documentation would be nice for those who want to play Archipelago on a macOS system.
Turns out you don't need sudo to do the download_data, oops.
Added an alternate option to simply terminal navigation
@Bicoloursnake Bicoloursnake changed the title OOT: Adjust the Logic Tricks to be an ordered object OOT: Adjust the Logic Trick Keys to be an ordered object Jan 17, 2024
@espeon65536
Copy link
Collaborator

Looks good to me, although it would be nice if you cleaned up the commit history of this branch

@Bicoloursnake
Copy link
Contributor Author

Sorry about the commit history. Is there a resource you can point me to for cleaning up commit history? I haven't had much luck using Github's sync fork feature to do it, and I'm not quite sure where else to look.

Copy link
Member

@Berserker66 Berserker66 left a comment

Choose a reason for hiding this comment

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

I'll just squash the history down

@Berserker66 Berserker66 merged commit ac7b707 into ArchipelagoMW:main Jan 18, 2024
12 checks passed
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants