Bypass some events for world validation checks, or something

[r0bd0g]

This fixes the most serious issue, but at the same time it barely fixes anything, at the cost of adding some redundancy into the logic. I don't know that it will ever be possible to fix it properly because it's so difficult to be sure that you're not breaking anything. I have gone through and moved some settings out of events so that world validation checks can pass through those entrances, since validation checks aren't allowed to collect anything, including those events. This PR applies some changes to the Kakariko Gate and to any passthroughs involving beans. I felt that especially the Kak gate issue was pretty serious (for example, it wasn't allowing child on only spawn ER to spawn in Death Mountain area), and the planted beans issue was also kind of rough, so I felt that something needed to be done to address those particular issues sooner than later.

Except for the check that confirms no-item access to Kokiriko, all validation checks (ToD, ToT, and Poe access) allow for the use of starting items. An obvious example of a problem that couldn't be addressed here might be starting with Hammer, if the grotto being opened is permanent, it will use an event and so the validation checks can't pass through, whereas a temporary grotto could have been entered. So for a specific example, if you were on mixed pools interiors and grottoes starting with a Hammer, you might find the Poe trader in the near Kak grotto, but never in the near Market grotto, which is of course pretty silly. It's unsustainable(?) to go around to every passthrough that uses an event that contains items that you might start with, and make non-event duplicates of that logic to allow the current age to pass through, so I opted not to deal with starting items at all. If you have to start with any item for any change to matter for the validation checks, I didn't do it, even though a lot of the time those items weren't even checked for inside of the problematic event, but were needed either to reach the event or to complete the passthrough after the event.

(I found some additional settings-only passthroughs that I opted not to address since there are a lot of items that could have been involved, but also they're not yet relevant on main branch without some currently work-in-progress features. If King Dodongo shortcuts and BONKO both enabled, you need no items to pass through that boss room. If the right tricks are enabled, no items are needed for adult to pass through the Morpha or Bongo boss rooms. There's also passing through the enemies in the entrance of MQ Ganon's, which requires no items as adult, with there being an open PR currently to shuffle the tower entrance. So you might want to add is_adult as an alternative to the large here() for clearing the enemies in the first room, if that gets accepted, idk.)

This resistance to using starting items ever puts some starting items in a bit of a weird spot. Starting with unshuffled Zelda's Letter is more of a setting than it is a starting item, but I still opted not to handle that. If your gate is set to open on Zelda and you start with unshuffled Zelda's Letter, the gate is just as open immediately as with open gate, but you know what that's your problem just pick open gate. Unshuffled Gerudo Card on open Gerudo Fortress is in a similar spot, which again I feel is more of a setting than a starting item. (I don't think the fact that it's a 'skipped location' matters or not for whether it can be used as a starting item?) The Card would matter for the Gerudo Gate, entering GTG, and also just general navigation around Fortress and Hideout. (You'd have to be careful about only checking the relevant settings in place of Gerudo Card, because that could allow you to enter GTG, which requires the rupees you might need from first finding Kokiriko. A bit hack-y, but also checking for Time Travel on a route intended to let the validation checks pass through the GTG entrance would allow all except the Kokiriko check to pass through, since all of those collect Time Travel.) And quickly pointing out another piece of jank -- because of the keysy implementation detail of logically starting with all of the keys, unlocked doors can't be used for the Kokiriko access check despite starting items not practically being involved.

Remote events, including many named events and also at()s can't usually be worked around in the manner that I used in this PR, since you don't know how the region with the event might be accessed. For example it's possible that Defeat Morpha and thus Water Temple Clear could be collected without needing any items at all as adult, with the right tricks and settings enabled, which would allow access into the fishing pond. But of course if your access to that event is not as the age that you want to use the event as...

The only way to solve these issues completely would be to allow events (and more?) to be collected during some world validations checks. But that is likely not going to be safe to allow in some instances, and I'm not entirely sure what all those instances are. You could imagine starting as adult and needing to Hammer open a grotto so that child could return to ToT later. Obviously that would break the point of the ToT access check, which confirms that the age you don't start as can return to the Temple of Time without potentially needing the age you do start as to contribute in some way. I think that, the way that validation check is currently written, merely adding in allowing events to be collected would likely break the check in the manner I've just described.

There's also the question of whether unshuffled or plando'd items might be safe to collect. (I'm not seeing either whether the ToD access check is doing anything special to prevent ToD-specific entrances from being used, so it seems likely none of the other checks are allowed to do anything ToD related either?) How deep does the rabbit hole go on what's safe to collect and what isn't? Honestly no idea. (Also the thought of having to think about how any changes here might interact with the shared Spirit Temple areas scares me a little.)

If you really want me to, I could be more rigorous and actually duplicate the logic for more obscure settings combos and starting items. (For example, I could replace all instances of "here(can_blast_or_smash)" on passthroughs with "(here(can_blast_or_smash) or can_blast_or_smash)", or even add an alternative to Water Temple Clear that confirms the necessary items to beat Morpha and that all relevant ERs that could move that boss somewhere else are also disabled.) It's all stuff that wouldn't break any of the validation checks when written correctly, but still would miss out on being able to handle several cases. And it would be a lot of code duplication and quite difficult to maintain. Something needs to be done to fix that Kak gate, but this rabbit hole is pretty deep.

[fenhl]

I think these changes should be commented, since they're really just workarounds for a bigger issue.

Ultimately, I believe it would make sense to replace these supplementary searches with ways to conditionally require equivalent access for some things, which would allow us to relax the restrictions on them. For example, we could say that access to the guard house is only required to use bottles.

In the meantime, I wonder if it would make sense to adjust the definition of here so it doesn't just generate an anonymous event but also checks the condition directly.

[r0bd0g]

Unwrapping here() like that, where it checks both for the event and for the condition directly, would maybe fix that specific event type. (Though if any here() events are safe to collect, doing that still wouldn't allow the event to actually be collected. I think probably here() events are probably never safe b/c they're generally used for cross-age communication, which breaks at least the ToT access check, at least the way that check is currently written.) I worry about negatively impacting execution time when double-checking all the logic like that.

Part of what bothers me about all this is the extent to which minute implementation details currently matter. Open Kak gate was written in a pretty similar way to what this PR does not too long ago. And we'd considered whether to check plant_beans inside the helper or not. There are probably cases in the logic where a setting could have been put inside an event but wasn't. Should those be commented as well? Maybe somewhere there should be a general warning about using events, I dunno.

[r0bd0g]

Thinking some about the various access checks. No idea if anything I've said here makes any sense. I'm not happy with the idea to only handle here() events by checking for their conditions as an alternative to collecting the event (which is functionally the same as only allowing the event to be collected as the age that intends to use it), because many at() and named events are okay to use as well, again as long as they can only be used by the same age that collects them.

The Kok/Kak access check is actually safe to collect anything, as long as you don't use rupees, or any item that requires refills. Maybe it could be possible to go through and somehow disallow those things for the purposes of this check. It's also possible, it might even be a good idea, to replace this access check with robust handling of drop logic for rupees and refills. Besides the obvious downside of being a bit of a nightmare to keep everything working correctly (I could probably handle it, but it'd probably be hard for others to keep track of it all), this would negatively affect the other access checks a bit b/c they wouldn't be allowed to collect the drops, taking the use of those starting items or using rupees off the table, unless further changes are made.

You might think you could apply the above idea to the poe trader access check, as in, you need to access the poe trader to be able to use bottles, but this can't work. It doesn't jive with the fact that vanilla OoT requires the use of bottles to be able to reach the poe trader (you must use a fish to enter Jabu to get Song of Time, etc). Banning bottles until you can get adult and find the poe trader is obviously too heavy-handed and can't be the right choice. (I wonder if the best outcome here might be to somehow allow big poe bottles to be emptied. But I think about the prospect of adding that and it just doesn't sound like it'd be easy to implement or communicate exactly how it works to to player? So it's clearly not in the cards.)

I had an idea where, what if, regardless of the seed's starting age, you pretend that you've started as adult, and you're allowed to collect whatever adult can collect (probably meaning the time travel item is banned?). And you would ban bottles from being used as adult before the poe trader can be reached. (No idea what kind of technical difficulties might be encountered with trying to implement all that.) But problems arise when Spirit Temple gets involved. Here comes a contrived scenario. Maybe child needs a bottle to get to Spirit because you have to go through the Jabu entrance or something. You spend your only spirit key as child but you miss whatever item it is in shared Spirit that adult needs to get to be able to reach the poe trader without ever going back child. You fill that bottle with a big poe. Now you can't spend that key to get into Spirit as adult and you can't get back to Spirit as child, so you're stuck. (The only part about this that requires poor choices from the player is when they bottle a big poe and save it without yet having access to the trader.)

The fact that this kind of sticking point exists makes me wary of any scheme that collects anything ever in these validation checks. I wonder if all of the validation checks could be rewritten to work like what described, where you pretend you start as the age that the validation check is concerned with, and maybe you have ban time travel. Collect whatever you can, and see if you can reach whatever goals. But they all run into non-repeatable access problems like that Spirit thing. It we actually move forward with allowing things to be collected in these checks, we'd have to be exceptionally careful if we're going to avoid this type of dead end. I guess this would mainly be done by somehow (perhaps this means just only allowing events to be collected, if there are no problematic events possible in Spirit) making sure nothing of importance can be collected using the shared age routes in Spirit... And beyond that just generally hoping that we haven't forgotten anything important -- the issues might be a lot worse than this and I'm just not thinking about it the right way yet idk it's all too confusing.

The more I think about it, the more I think that little patches like what you see in this PR might be the best that we can realistically muster... Is there anything I missed in this PR that might be worth handling with some janky duplicate logic???

[r0bd0g]

As dumb and as weird as this PR is you probably should merge it soon before s7 races really pick up.

[aofengen]

Please do not merge this until race mods understand what in God's name is going on, because no one here has explained anything

[fenhl]

Here's a copy of a high-level summary of this PR that I posted in #dev-race-mods-maintainers:

okay so the randomizer does something called supplementary searches, which are part of the world validation. They ensure invariants that are not handled by the main logic, namely access to time of day changes, access to ToT as the non-starting age, access to the guard house as adult, and access to refills

due to how these are implemented and used, they cannot pick up items. Events are implemented as items so they will also not trigger events. One example is opening the Kakariko gate

#1983 fixed a bug where the logic would require child access to Kakariko to pass through the gate even if the gate was already open from the start of the game. @r0bd0g was unaware of how supplementary searches work at the time, so the fix uses an event. This had the unintended side effect of making it impossible to spawn in DMT even with the gate open

#2093 adjusts the logic for the gate again to preserve the bugfix but also allow the spawn in DMT. It also makes a similar change to allow pre-planted beans to be used for the supplementary searches. That's all the PR does

the reason there's so much discussion around this is that this fix is considered a hack, and there are many other similar situations which aren't being fixed, but a proper fix would amount to a complete rewrite of the randomizer's algorithms which is not happening any time soon. So the discussion is about how far to extend this hack in the meantime

[r0bd0g]

Ah I got beat to it. Probably theirs was an easier explanation anyway. I'm gonna delete my attempt to explain it lol. Let me add two things:

If you see any other changes other than Kak Gate or Planted Beans, it's all minor edits that do nothing. I was originally wondering if I was going to need to make larger changes, and decided not to. But you see some minor edits across the file that change nothing as a result of my noodling as I scanned over the file.

Unresolved question about what to do about a potential comment explaining the situation. It's a hack but, kind of so is everything right now??? So I haven't added a comment b/c I don't know what to say or where to say it... How should people modifying the logic be made aware of the subtleties of events? But I would disagree if you think the current lack of a comment should hold up the fix? I didn't write the original ER implementation which is where this issue originates. And it's gone uncommented anywhere in the logic files for years already.

[fenhl]

This is what I mean regarding comments. Just explain the hack where it's used to prevent anyone from removing this seemingly redundant code until a proper fix is implemented, if that ever happens.

[r0bd0g]

Alternatively, the settings could be removed from the gate event and the plant beans helper entirely, and the setting checked an additional time on every instance where the event/helper are used.

Those comments also don't necessarily stop anybody from making the same mistake that I did with some other event.

Maintainers feel free to commit those suggestions if I haven't gotten to it yet. (I'm not sure actually how to merge the whole of them at once...)

[fenhl]

Those comments also don't necessarily stop anybody from making the same mistake that I did with some other event.

That's not the point of the comments.