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

Rando: Master Sword Shuffle #2981

Merged
merged 46 commits into from
Oct 22, 2023

Conversation

stratomaster64
Copy link
Contributor

@stratomaster64 stratomaster64 commented Jun 11, 2023

Adds the Master Sword to the item pool, requiring Adult Link to be more creative in his offensive strategies until he finds it.
Because of this:

  • Sheik/Ganondorf will give you hints on where the Master Sword may be located.
  • Logic is updated to reflect new requirements.
  • If you manage to get to Ganon's boss fight without MS, the fight will not give it to you (which fixes a vanilla bug).
  • Swordless Link oddities have also been addressed.

Build Artifacts

@stratomaster64
Copy link
Contributor Author

stratomaster64 commented Jun 12, 2023

Known issues so far:

  • Spoiler log doesn't see Master Sword as an item required to beat the game (for Ganon) and still assume you obtain the Master Sword by turning adult.
  • Two "Master Sword" entries are placed into the world, expected only one.
    • Actually obtaining the Master Sword at these locations will present itself as a different item entirely (due to the GetItem enum for Master Sword). Removing the errant enum results in the "Master Sword" looking like a BGS (expected), but not actually giving the sword itself, instead giving bombs on the item slot.
  • Although starting as Adult Link correctly leaves you swordless until you obtain a sword, starting as child and turning adult still gives you the Master Sword.

stratomaster64 and others added 6 commits June 11, 2023 21:07
"Master Sword" Items now actually give MS
updated GIMESSAGE (should stop crashing on non-Windows);
re-added MS in item list
cihld -> adult no longer gives MS;
ToT Master Sword now gives correct item
Add Master Sword GI draw func based on ToT MS object
@stratomaster64 stratomaster64 marked this pull request as ready for review June 12, 2023 23:09
Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Looking good so far. I've skipped over the locacc_xxx logic for now, hoping others can chime in there.

Something else that stands out, you most likely need to make changes in z_boss_ganon2 to prevent the player from getting a free MS sword. Could probably "kill" the spawned sword, or something.

soh/soh/Enhancements/randomizer/3drando/logic.cpp Outdated Show resolved Hide resolved
soh/soh/Enhancements/randomizer/3drando/item_pool.cpp Outdated Show resolved Hide resolved
soh/src/code/z_parameter.c Outdated Show resolved Hide resolved
soh/src/code/z_parameter.c Outdated Show resolved Hide resolved
soh/src/code/z_sram.c Outdated Show resolved Hide resolved
@garrettjoecox garrettjoecox added this to the MacReady milestone Jun 13, 2023
Check Tracker changes;
Gave RAND_INF and ice trap logic to ToT MS check;
Fixed swordless behavior for HBA/fishing
@inspectredc
Copy link
Contributor

We might want something along the lines of

if (gSaveContext.n64ddFlag && Randomizer_GetSettingValue(RSK_SHUFFLE_MASTER_SWORD) && gSaveContext.equips.buttonItems[0] == ITEM_NONE) {
    gSaveContext.equips.equipment = 0x1120;
} else {
    gSaveContext.equips.equipment = 0x1122;
}

where this section is

gSaveContext.equips.equipment = 0x1122;

I think what this does is stops the master sword slot from being highlighted in the pause menu but please double check

@stratomaster64
Copy link
Contributor Author

We might want something along the lines of

if (gSaveContext.n64ddFlag && Randomizer_GetSettingValue(RSK_SHUFFLE_MASTER_SWORD) && gSaveContext.equips.buttonItems[0] == ITEM_NONE) {
    gSaveContext.equips.equipment = 0x1120;
} else {
    gSaveContext.equips.equipment = 0x1122;
}

where this section is

gSaveContext.equips.equipment = 0x1122;

I think what this does is stops the master sword slot from being highlighted in the pause menu but please double check

Works as expected, will include in the next commit. Thanks!

Visual bug where box hovers over non-existent MS gone;
Fixed RAND_INF check with ToT MS pedestal;
Ganon no longer gives free MS
@stratomaster64
Copy link
Contributor Author

@inspectredc Could you also look into the location access logic files to confirm that everything makes sense there? I mostly copied off 3d rando changes for their MS shuffle, except where you already modified logic for tricks.

@inspectredc
Copy link
Contributor

@inspectredc Could you also look into the location access logic files to confirm that everything makes sense there? I mostly copied off 3d rando changes for their MS shuffle, except where you already modified logic for tricks.

Yeah will do that today 👍🏼

Copy link
Contributor

@inspectredc inspectredc left a comment

Choose a reason for hiding this comment

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

Few changes here, I realised when suggesting the hammer gold skull changes that it's definitely outside the scope of this PR to address those so feel free to not change them.

Mostly just adding some kokiri sword as adult checks here and there. If not hopefully I've left enough comments or it's clear enough as is

stratomaster64 and others added 2 commits June 15, 2023 17:41
Co-authored-by: inspectredc <78732756+inspectredc@users.noreply.github.com>
getting master sword no longer highlights box
Copy link
Contributor

@Pepe20129 Pepe20129 left a comment

Choose a reason for hiding this comment

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

Did a few randos and everything was functional; the only things I'd say are that when turning into adult, link visually pulls out the master sword and then stashes it away even if you don't have it, if it's possible I think it'd be better if either the sword was invisible or the animation was changed to another one (standing for example) and that in the ganon fight something similar happens where link visually takes out the sword even if you don't have it before ganon knocks it away

Those are just minor nitpicks tho and I think this is perfectly fine to merge

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Looking good so far, very close to the finish line! I have provided a few more suggestions/fixes below.

Other than that, the only thing that still has me uneasy is all the swordless management, especially knowing there is still some issues with it regarding Epona. At the moment, I don't really have an idea on how to address that, so may just need to think on that for a bit.

soh/src/overlays/actors/ovl_player_actor/z_player.c Outdated Show resolved Hide resolved
soh/src/code/z_play.c Outdated Show resolved Hide resolved
EventAccess(&NutPot, {[]{return NutPot || ((LogicSpiritTrialHookshot || CanUse(HOOKSHOT)) && HasBombchus && Bow && (MirrorShield || (SunlightArrows && CanUse(LIGHT_ARROWS))) && IsAdult);}}),
EventAccess(&SpiritTrialClear, {[]{return CanUse(LIGHT_ARROWS) && (MirrorShield || SunlightArrows) && HasBombchus && (LogicSpiritTrialHookshot || CanUse(HOOKSHOT));}}),
EventAccess(&NutPot, {[]{return NutPot || (((LogicSpiritTrialHookshot && CanJumpslash) || CanUse(HOOKSHOT)) && HasBombchus && CanUse(BOW) && (CanUse(MIRROR_SHIELD) || (SunlightArrows && CanUse(LIGHT_ARROWS))));}}),
EventAccess(&SpiritTrialClear, {[]{return CanUse(LIGHT_ARROWS) && (CanUse(MIRROR_SHIELD) || SunlightArrows) && HasBombchus && ((LogicSpiritTrialHookshot && CanJumpslash) || CanUse(HOOKSHOT));}}),
}, {
//Locations
LocationAccess(GANONS_CASTLE_SPIRIT_TRIAL_CRYSTAL_SWITCH_CHEST, {[]{return LogicSpiritTrialHookshot || CanUse(HOOKSHOT);}}),
Copy link
Contributor

@Malkierian Malkierian Oct 11, 2023

Choose a reason for hiding this comment

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

So unless you can hit this thing's crystal switch with the hookshot, you might need to include logic conditions for having a sword as adult. I think it assumes right now that being able to use hookshot also means you have a sword, since you'd be adult, and a spin attack will hit it through the wall.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah the line would need to be
LocationAccess(GANONS_CASTLE_SPIRIT_TRIAL_CRYSTAL_SWITCH_CHEST, {[]{return (LogicSpiritTrialHookshot || CanUse(HOOKSHOT)) && CanJumpslash;}}),
I imagine

stratomaster64 and others added 3 commits October 11, 2023 19:24
* better swordless handling with temp B

* prevent auto save in fishing pond

* prevent auto save during bombchu bowling
@Archez
Copy link
Contributor

Archez commented Oct 14, 2023

Copying description from stratomaster64#11 for visibility/history


This PR includes improvements for swordless state handling during minigames/epona by hijacking tempB with a "marker" value to allow us to restore the B button as empty without "disabling" the B button in the HUD. This should resolve all remaining issues with the HUD.
This also resolves child not having slingshot on B during child archery when swordless.

Additionally, an old swordless hack for child fishing can be removed in favor of the new handling, which should remove any possible cases that a kokiri sword is accidentally given to the player. This required preventing auto save in the fishing pond to avoid the fishing rod on B being saved to file.

A similar auto save restriction for bombchu bowling was needed. Shooting gallery did not have issues with auto save though.

The following scenarios have been tested:

  • Riding Epona without a sword or bow
  • Riding Epona with a sword but no bow
  • Riding Epona without a sword but with a bow
  • Pausing while on Epona
  • Saving while on Epona
  • Fishing as child or adult without a sword
  • Pausing while fishing
  • Saving while fishing
  • Game over while fishing
  • Playing shooting gallery as child or adult without a sword
  • Game over during shooting gallery
  • Playing bombchu bowling without a sword
  • Game over during bomchu bowling

Behind the scenes, rather than storing ITEM_NONE which is the same as BTN_DISABLED (255), I've elected to store ITEM_NONE_FE (254) so that we have a marker to know that we were swordless before and it doesn't dim/disabled the B button like (255) does.

Copy link
Contributor

@garrettjoecox garrettjoecox left a comment

Choose a reason for hiding this comment

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

Bravo. I know this one sat in the oven for a while but it's turned out great and playtested quite a lot.

As discussed in the discord we need to swap the instances of 0x72 for 0xE0, but assuming that is complete dropping an approval 👍

Copy link
Contributor

@Archez Archez left a comment

Choose a reason for hiding this comment

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

Beautiful! So excited to have this in. Thanks for working through the feedback.

If you get a chance, it would be nice to have something in the PR description listing roughly what was done for historical purposes, but not a blocker for merge 👍

@garrettjoecox garrettjoecox merged commit 2eaed8d into HarbourMasters:develop Oct 22, 2023
8 checks passed
@stratomaster64 stratomaster64 deleted the mister-sword branch November 8, 2023 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking This PR contains code that will be needed for another feature or bugfix to be PR'd later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants