-
Couldn't load subscription status.
- Fork 16
Itemvalue misc fixes #104
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
Itemvalue misc fixes #104
Conversation
|
I was waiting for some ItemValue bug report before undrafting this PR but it seems that report wont come so ill mark this as ready |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor typo, everything else seems good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't believe nico dismissed my previous review, smh my head
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

I told myself I should stop making PR past midnight but here's one.
this is #101 but the diff work this time
og. lowercase commit was split into #103
includes multiple small fixes to ItemValue functionality:
b21f576 Renamed world.item_values_cache to world.itemvalue_rule_cache to stop potential confusion with world.item_values
88854ef Split/changed the force reset parameter of the cache of get_items_with_value to be a skip instead. Mainly so it work more like the ItemValue rule function. I made some helper.py functions to replace the reset usage
5150bbc Made both datavalidation of ItemValue (prefill and assert) only check for region with locations in it and their parents using some recursion tech
to explain the how/why of 5150bbc
my fix for https://discord.com/channels/1097532591650910289/1307802587839594617
WHY:
Hooks and Options can remove locations.
I could easily add a check to only test for region with locations but its still possible for a region without to be logically required since parent region can be empty.
HOW:
it work basicaly the same for the PreFill test and the Assert one
the only difference is the way to find the parents
Assert: Since we only have the basic json, I build a dict(child,list(Parent)) that way I can do the recursion belowRemovedin both case:
2.1 add the parents to an "already checked" list so it's skipped in future recursion