Skip to content
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

VBify #4255

Merged
merged 51 commits into from
Oct 7, 2024
Merged

VBify #4255

merged 51 commits into from
Oct 7, 2024

Conversation

Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Jul 26, 2024

This PR moves some parts of randomizer and boss rush to VB.
It also got a little bit out of hand so it also moves some stuff from random places into their respective hooks (as an example there was a bit of rando code that run right before the OnSceneInit hook, so I've moved it to the OnSceneInit hook itself).
I recommend looking at this PR commit by commit as it's easier to understand (specially in BossRush.cpp).

Build Artifacts


f32 triforcePieceScale;

void RandomizerOnPlayerUpdateHandler() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should likely split this into two separate hook handlers, both of them only being registered when their respective options are on (triforce hunt & the randomized swimming)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that it might be better to have only one hook handler for each hook in randomizer and split if off inside the hook to prevent registering too many hooks, if it's preferred to use separate hooks, I'll change it to that.


extern "C" void func_8099485C(DoorGerudo* gerudoDoor, PlayState* play);

void RandomizerOnActorUpdateHandler(void* refActor) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is by far the most expensive hook we can register, we need to be very careful and precise about how and when this is registered. Each of the different target actors should be specified by the RegisterGameHookForID hook, and these hooks should only be registered when their respective options are on

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eg

onFileLoad(() {
  // unregister skeletonKeyDoorUpdate

  if (skeletonKeyEnabled) {
    // Because we're doing RegisterGameHookForID this will only ever be called for door updates
    GameInteractor::Instance->RegisterGameHookForID<GameInteractor::OnActorUpdate>(ACTOR_EN_DOOR, skeletonKeyDoorUpdate);
  }
})

}
}

void RandomizerOnGameFrameUpdateHandler() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, we should only register this hook handler when the infinite upgrades option is enabled in the current save

Copy link
Contributor

@Pepper0ni Pepper0ni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some bugs i found

gPlayState->transitionTrigger != TRANS_TRIGGER_START &&
(1 << 0 & gSaveContext.inventory.dungeonItems[SCENE_GANONS_TOWER]) == 0
) {
GiveItemEntryWithoutActor(gPlayState, ItemTableManager::Instance->RetrieveItemEntry(MOD_RANDOMIZER, RG_GANONS_CASTLE_BOSS_KEY));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be working, I'm not getting the GCBK after reloading in after the credits sequence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I missed this, yeah this should be migrated to just doing a flag set and letting the queue handle the give like everything else

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that it's not a check so we can't use randomizerQueuedChecks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be scope creep, but this made me think of the "ganon boss key on LACS" situation, and how seperating the conditional GCBK gives from LACS would be a good idea. Making a new type of GCBK give would solve both that and this.

soh/soh/Enhancements/randomizer/hook_handlers.cpp Outdated Show resolved Hide resolved
soh/src/overlays/actors/ovl_Door_Warp1/z_door_warp1.c Outdated Show resolved Hide resolved
@Pepe20129 Pepe20129 added code cleanup rando v3 Issues specifically targeting develop-rando labels Sep 10, 2024
@@ -533,11 +533,15 @@ void DoorWarp1_ChildWarpOut(DoorWarp1* this, PlayState* play) {
if (GameInteractor_Should(VB_GIVE_ITEM_FROM_BLUE_WARP, true, ITEM_GORON_RUBY)) {
Item_Give(play, ITEM_GORON_RUBY);
}
play->nextEntranceIndex = ENTR_DEATH_MOUNTAIN_TRAIL_0;
gSaveContext.nextCutsceneIndex = 0xFFF1;
if (GameInteractor_Should(VB_BLUE_WARP_APPLY_ENTRANCE_AND_CUTSCENE, true, this)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon second look at all the changes here in this file, I don't think we need any of them. You should be able to override the destination of the blue warp the same way we are for rando & cutscene skips with the VB_PLAY_TRANSITION_CS

Copy link
Contributor

@Pepper0ni Pepper0ni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On checking develop-rando, the GCBK issue exists there too it seems, so can be fixed elsewhere

@Malkierian Malkierian merged commit 2822dfc into HarbourMasters:develop-rando Oct 7, 2024
5 checks passed
@Pepe20129 Pepe20129 deleted the vbify branch October 8, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup rando v3 Issues specifically targeting develop-rando
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants