Fix multiworld path hints: last location edition #2077
Merged
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.
Fixes #1733 (again)
See #2021 for context. That same class of issue was present in the "Make sure this wasn't the last hintable location for other goals" section, which overrides the previously selected goal with a new one.
I previously changed the main path hint generation block to ensure it gives an appropriate goal+world combination, but this block of code overrides that goal with a different one, without changing the world. That reintroduces the possibility of the goal and world being incompatible. If you're lucky, it's obviously wrong with a "path of Link's pocket"; if you're unlucky, it's just an incorrect hint.
The fix here is in the same vein as the previous one: stop discarding the portion of the required_locations tuple which lists the worlds that require this location's item for the same goal. Instead, keep goal+world together as a tuple, and use them to override the previously selected goal and world together.
I also tweaked this to gather all candidate goal+world combinations, and randomly choose from among them; the old code was biased to prefer goals ordered later in the category (though that would be a very minor bias considering that this codepath may not kick in at all for a given seed).
I went ahead and did some tidying along with the main change:
Soap box: it feels like the structure of goals + required_locations is the source of most of the complexity here, and that it would be more natural to have each per-world goal actually reference its own required locations. If W1's Deku Tree Map Chest has a hookshot needed for W2's Water Temple, that would intuitively be listed as required under W2's Water Temple goal. Right now, that requirement gets put on whichever W1 goal happens to align with W2 Water Temple; if W2 Water gives the Light Medallion, then W1's Light Medallion will have a required_location that says "W2 needs a hookshot to get their Light Medallion".
Seems like the other structuring would be more intuitive, and it would also allow more reasonable behavior for one_hint_per_goal. Right now, one_hint_per_goal is purely goal based, instead of considering different goal+world pairs distinct. So if W1 has already generated a hint for W2's Light Medallion, then it won't want to generate a hint for W3's Light medallion, even though they have basically nothing to do with each other. Similarly, for the purposes of this override code, if W2 and W3 each have a Light Medallion with 1 remaining hintable location from W1, and W1 has selected one of locations items to be hinted for a different goal, then this "last hintable location" logic won't kick in and override because W1's Light Medallion goal will have 2 separate required_locations entries for W2 and W3. It's the last chance for the location to be used to hint one of the specific goal+world pairs, but not the last chance to hint somebody's Light Medallion, so no override. I might be missing something, but my read of it is that this piece of code was primarily thought through in the context of single-world, and then imperfectly extended to multi-world.