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: make state.prog_items a Dict[int, Counter[str]] #2407

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

alwaysintreble
Copy link
Collaborator

What is this fixing or adding?

Changes CollectionState.prog_items from a Counter[Tuple[str, int]] to a Dict[int, Counter[str]], offering more speed.

How was this tested?

Generated 1000 world messenger multiworld 10 times before and after and saw a ~9% speed increase. Could use more testing and benchmark numbers, but this should increase generation speed overall.

@el-u
Copy link
Collaborator

el-u commented Oct 31, 2023

Stardew Valley overwrites prog_items here:
https://github.com/alwaysintreble/Archipelago/blob/ada70d32963db68ad023811778d23e750c6b5017/worlds/stardew_valley/test/TestRules.py#L27
I don't know why it does that, but if you change it to overwrite it with a new dict-of-counter it should start working.

@alwaysintreble
Copy link
Collaborator Author

alwaysintreble commented Oct 31, 2023

I don't know why it does that

I think it's testing that the starting inventory is properly required? But yeah it'd be a lot better to just test that with a clean state instead of telling the state to do things that will never happen in normal circumstances.

@ScootyPuffJr1 ScootyPuffJr1 added is: refactor/cleanup Improvements to code/output readability or organizization. affects: core Issues/PRs that touch core and may need additional validation. labels Oct 31, 2023
Copy link
Member

@LegendaryLinux LegendaryLinux left a comment

Choose a reason for hiding this comment

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

Tested a generation and connected with ArchipIDLE, so I approve the IDLE change.

@Jarno458
Copy link
Collaborator

Jarno458 commented Nov 1, 2023

Tested it, it does seem to work, and does make debugging simpler

@Berserker66 Berserker66 merged commit 5669579 into ArchipelagoMW:main Nov 2, 2023
12 checks passed
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.

6 participants