-
Notifications
You must be signed in to change notification settings - Fork 631
Adds a new Hook for adding locations to pools #5945
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
base: develop
Are you sure you want to change the base?
Adds a new Hook for adding locations to pools #5945
Conversation
It is set up similarly to Vanilla Behavior Override hooks.
Had to make ShuffleTradeItems.c into a cpp file and add some extern "C" boilerplate stuff to make it happen.
Still needs conversion of everything else about it to hooks.
|
|
||
| // Rando Generation | ||
| DEFINE_HOOK(OnGenerationCompletion, ()); | ||
| DEFINE_HOOK(ShouldAddLocationToPool, (Rando::Location * location, bool* result)); |
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.
| DEFINE_HOOK(ShouldAddLocationToPool, (Rando::Location * location, bool* result)); | |
| DEFINE_HOOK(ShouldAddLocationToPool, (Rando::Location* location, bool* result)); |
surprised clang-format let this by
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 had it like that before, clang-format was the one that changed it.
| } else { | ||
| if (location->IsOverworld()) { | ||
| *should = RAND_GET_OPTION(RSK_SHUFFLE_CRATES) == RO_SHUFFLE_CRATES_OVERWORLD; | ||
| } else { | ||
| *should = RAND_GET_OPTION(RSK_SHUFFLE_CRATES) == RO_SHUFFLE_CRATES_DUNGEONS; | ||
| } | ||
| } |
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 appreciate why you didn't make this one long else-if chain, but the extra level of indentation still irks me
ofc I'm degenerate enough to use ?: here
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.
There's a few of the earlier ones I converted that I may go back and clean up a bit
| switch (location->GetRandomizerCheck()) { | ||
| case RC_LW_DEKU_SCRUB_GROTTO_FRONT: | ||
| case RC_LW_DEKU_SCRUB_NEAR_BRIDGE: | ||
| case RC_HF_DEKU_SCRUB_GROTTO: |
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.
| case RC_HF_DEKU_SCRUB_GROTTO: | |
| case RC_HF_DEKU_SCRUB_GROTTO: | |
| *should = true; |
need this no? I'm confused what not setting *should does since everything is wrapped in checking RCTYPE first, where if RCTYPE mismatch happens we don't touch *should so I'd expect default is false
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.
*should defaults to true, you only need to change it if it should be false. I did it that way on purpose so if you try to do a new shuffle, it's true and all the basic checks that we always shuffle don't need any hooks. Some of the ones I converted first set *should to a boolean expression, it's just that setting it to true is effectively "doing nothing" in those cases. These are what I meant when I said I might go back and clean some of those up a bit more.
|
This doesn't immediately come off to me as a problem that *should (pun intended) be solved with hooks, though I don't have much familiarity with the shuffling side of 1ship's rando. Would it not make more sense to have the function that actually registers the checks conditionally register them based on if they should be shuffled? Eg for trees, |
|
Can't really do that because all the "static data" (like names and such) for locations need to be registered all the time for spoiler parsing reasons. This is constructing the location pool for the specific seed, which doesn't happen until I hit generate. I realize this is a bit different from the way we use most of our hooks, but all the ways I could think of to do what I wanted to do would've ended up just being like hooks without actually using our existing hook system, so I just used the existing hook system. |
|
Collecting the eyeball frog in a seed while on this test build gave me all of the trade items from odd mushroom on |
This PR adds a new hook for checking whether a check should be shuffled or not, so that logic can now exist in the cpp file for that shuffle rather than in some other location.
The usage of this hook somewhat mirrors the Vanilla Behavior Override hooks, where you have a default result and then your hook changes that result if it needs to. I've implemented it such that the default is "true", so while you're developing a new shuffle it's checks are shuffled by default, and then later you add the hook with logic to set it to false when they shouldn't be shuffled.
This PR also comes with a few new files for some older shuffles. Currently they only contain the
SHOULD_SHUFFLE_LOCATIONhook definition for those shuffles but they do serve as a good place to put any other hooks and/or actor action funcs that we currently have either in authentic source or in hook_handlers or something later.I haven't played any full seeds with this yet but I have generated several seeds with different settings to make sure the spoiler logs have the locations I expected them to have.
Build Artifacts