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

Fix: Own Dungeon items using vanilla GI in suffled boss rooms #2859

Conversation

Archez
Copy link
Contributor

@Archez Archez commented May 8, 2023

When Own Dungeon items are set, we use vanilla GI values instead of the rando equivalent since the vanilla item give behavior can be used. This breaks when boss rooms are shuffled, causing keys/maps/compasses assigned to boss heart containers to apply to the wrong dungeon.

To address this, I've added a check on Own Dungeon item to also check that boss rooms aren't shuffled. Otherwise the rando GI will be used instead. e.g. vanilla = if Own Dungeon && Boss Shuffle off

Build Artifacts

@leggettc18
Copy link
Contributor

I'm willing to approve this for now, but part of me is wondering if we shouldn't just always return the keysanity keys/maps/compasses. Seems like it's creating a lot of extra work for not much gain. I feel like there was a reason we didn't do that but I don't recall what it was at this point.

@aMannus
Copy link
Contributor

aMannus commented May 8, 2023

I'm willing to approve this for now, but part of me is wondering if we shouldn't just always return the keysanity keys/maps/compasses. Seems like it's creating a lot of extra work for not much gain. I feel like there was a reason we didn't do that but I don't recall what it was at this point.

Yeah we discussed it briefly back when we introduced the coloured keys, and decided to keep it vanilla as there was some concern some people might prefer the vanilla text when it isn't needed and that it might feel strange to some that keys that are functionally equivalent to vanilla would suddenly be colored.

I'd be OK with just always using the randomizer specific keys, but I don't know how players feel about it.

That said, I'd say it's good to get this in and PR the switch later if we desire, just so we have this fixed asap.

CC @briaguya because I'm relatively certain he was part of the discussion back then as well

@briaguya-ai
Copy link
Contributor

briaguya-ai commented May 8, 2023

Yeah, I prefer vanilla when not shuffled. "You got the forest temple boss key" when in the forest temple and not having them shuffled outside of the forest temple just feels odd to me. Same with "ordinary vs masterful" when already in the dungeon and not shuffled outside. Same with colored keys when in the dungeon and not shuffled outside.

I feel like an ideal architecture for this would be using the keysanity versions but having logic in one spot to conditionally display the extra text and use the colored models etc.

@leggettc18
Copy link
Contributor

OK, my memory has been jogged then. I'll merge this one for now since there seems to be more discussion needed on any other potential fixes.

@leggettc18 leggettc18 merged commit caf8f1b into HarbourMasters:develop-spock May 8, 2023
7 checks passed
@Archez Archez deleted the fix-own-dungeon-gi-boss-shuffle branch May 8, 2023 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants