-
Notifications
You must be signed in to change notification settings - Fork 723
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
Core: add panic_method setting #3261
Conversation
If this PR isn't already doing so, I recommend making it so that tests use the panic mode by default, to rid ourselves of these test failures. |
They default to swap. |
Can we be confident that this PR's solution won't trigger on cases where there is a reasonably solvable world bug causing the failure? Otherwise, changing default test behavior may suppress bugs that it shouldn't. |
Could there be a way for a game that knows they get a random swap failure under default settings to specifically choose to have their tests run with panic mode? |
Hmm. How do we feel about "Do safe swap only, then if that fails, throw the item into starting inventory"? Not even trying swap at all seems a bit overkill for some applications of this that I could think of |
This is what I thought. |
Some form of this would be good. I have, however, seen it go into a swap loop for a very long time. As in one time I went to sleep while it was swapping and woke up 8 hours later and it was still trying to swap items. Some way of deciding it has taken too long and to just give up would be nice |
There are some games with some options that make swapping very likely. Super Metroid / SMMR with no early morph, for example. Any games with an extremely small sphere one where it's easy to fill it with the wrong items. |
Not entirely sure why this discussion is on this PR, but my 2 cents on the topic is that we should automate early items; in such a way that it builds a state with starting state + already declared early items, identify worlds with let's say <3% of reachable locations, then add some random items to state that fix this problem, then declare those as early items. |
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.
Other than a minor style-preference nitpick, this works as advertised and was instrumental in big async gens.
Any further discussion of enhancements that can be added on top of this can be made in a different discussion/PR, in my opinion. Otherwise, LGTM.
Co-authored-by: Zach Parks <[email protected]>
What is this fixing or adding?
adds a Setting:
How was this tested?
https://discord.com/channels/731205301247803413/1214608557077700720/1236015401494773812
verified that swap gives the same result as it currently does, raise gives a fillerror and start_inventory seems to be doing what it is supposed to, by adding 2 items to precollected for this seed.
If this makes graphical changes, please attach screenshots.