ArchipelagoMW / Archipelago

Archipelago Multi-Game Randomizer and Server
https://archipelago.gg
Other
502 stars 657 forks source link

OOT: Fix missing region accessibility updates after update_reachable_regions() #3712

Closed Mysteryem closed 1 month ago

Mysteryem commented 3 months ago

What is this fixing or adding?

CollectionState.update_reachable_regions() un-stales the state for the player, but when checking OOTRegion.can_reach(), it would only update OOT's age region accessibility when the state was stale, so if the state was always un-staled by update_reachable_regions() immediately before OOTRegion.can_reach(), OOT's age region accessibility would never update.

This patch fixes the issue by replacing use of CollectionState.stale with a separate stale state dictionary specific to OOT that is only un-staled by _oot_update_age_reachable_regions().

OOT's collect() and remove() implementations have been updated to stale the new OOT-specific state.

How was this tested?

I think this currently causes no issues at all, because CollectionState.update_reachable_regions() would only be called for an OOT player from Region.can_reach() with an OOT Region, but OOTRegion.can_reach() overrides this method to call its own _oot_update_age_reachable_regions(). If core code were to be updated to call CollectionState.update_reachable_regions() elsewhere in the future, then issues could occur. If this is deemed to not be an issue, this PR can be discarded.

The issue only appeared because of unrelated, abandoned changes I was making to CollectionState.sweep_for_events() to call CollectionState.update_reachable_regions() before checking for accessible locations, which resulted in OOT's pre-fill raising a FillError due to running out of locations to place items.

Generation was run before and after the patch with fixed seeds and OOT and other worlds known to be deterministic. The locations of all progression items were seen to be the same.