Simplifying Item_counts #156
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tial future usages
…s and thus in generation those would never be in logic
…ok modification later than create_items
src/__init__.py
Outdated
|
|
||
| item_counts = {} | ||
| item_counts: dict[int, Counter[str]] = {} | ||
| progression_counts: dict[int, Counter[str]] = {} |
There was a problem hiding this comment.
As part of this PR, it looks like all calls to world.get_item_counts() were changed to do only progression, and I don't see any other references to world.item_counts that use it in a non-prog way. (And pseudo-count
keywords only do state, so only prog.) So I have two questions:
- What's the use case for having a non-prog
item counts? What uses it or could use it? - If prog is the default usage, shouldn't
get_item_counts()default to prog_only?
There was a problem hiding this comment.
- I wanted to leave a way to get the original all item counts for potential hooks that used those counts (if there was any)
- similar to 1 prior to this pr the default call would return all item so I made it a choice so it wouldnt break any hooks that would use this.
if we dont care/worry about backward compat for created hooks then I can change it to default to progs and even remove the non prog support if thats what we wants
There was a problem hiding this comment.
I guess there's some potential value to being able to get overall counts at the end of create_items, even if we don't use it. Thanks for clarifying. 😄
FuzzyGamesOn
requested changes
Jun 30, 2025
…reset arg based on fuzzy's feedback
…into item_count_rework
FuzzyGamesOn
approved these changes
Jun 30, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rework get_item_counts to only create the item counts once(1 time) at the end of create_items
this should once and for all fix the counts by using the pool straight out of create_items instead of trying to recreate the item pool later from multiworld and precollected
I also made rules get the count of progression items by default
butsince I remembered State doesnt count non-progression items while generating meaning any All! would potentially never be in logic while generatingI also included a ! suffix if people want to use the old logic of All being every copy of an item not just the ones with progressions in their classification
the ! suffix is a something that can and probably should be something else is I just dont know what, It could even be removed entirely
This was tested in 2 tests using the same test manual with 3 copies of an item with 1 in game.json starting_items