Add option to start with bunny hood#1915
Add option to start with bunny hood#1915garrettjoecox wants to merge 2 commits intoHarbourMasters:developfrom
Conversation
|
Overall I think this is a great change. Making it less binary makes it much more fluent in gameplay, and allows us to tweak things like starting with bunny hood with less restrictions, so that's awesome. I still have to try it out ingame, so I'll see if I can get some time to do that soon as well. One question:
I imagine you mean that you can start with a mask now, so getting another trade item would overwrite the mask and thus making it impossible to go back to it without Mask Select. But as far as I can see you're not handling Weird Egg, so wouldn't this still happen when, for example, starting with Bunny Hood and then receiving Weird Egg from somewhere? |
I actually forgot about child weird egg + chicken :D Weird egg was simple enough but there was no flag determining if the egg had hatched so I added one in sohStats. Tested and verified all is working as expected with swapping between other items and those |
|
Report from @RaelCappra
Also after talking with @aMannus
|
|
I just realised, I'm pretty sure this resolves #1686. |
b01e7e8 to
df8a975
Compare
|
Alright, this should be good now.
|
60dc44f to
d7779b2
Compare
060b5ce to
3426fef
Compare
|
Pushed up an iteration here that works, but going to think through this for a bit longer |
3426fef to
f6f41f9
Compare
bdfc2bf to
10d40a6
Compare
10d40a6 to
3544ff1
Compare
|
Any particular reason why this still isn't ready for merge? I have been playing with this on for weeks and haven't run into any issues. |
Is this ready for review @garrettjoecox ? Last message was that you wanted to think it through a bit? |
|
Hmm, some one experienced an issue with it a few weeks back, I can’t remember the exact context or issue but I told myself I wanted to do another round of testing and cleanup on it. |
|
i know my issue was this PR always enabling mask select when rando'd, but it seems that has been changed |
05bd4a3 to
ba13e6b
Compare
Indeed it has. Had to jog my memory a bit, but it seems like the decision I came to was, you can now always cycle through child trade items in rando, but only your most recently obtained mask will be in that cycle unless mask select is on. Additionally, when mask select is on, you can only cycle between masks that you have borrowed before. (When you have Complete Mask Quest on, all masks are considered borrowed at the beginning) So for example: Another example: Kind of complex to explain with all the different combinations of options but I think gameplay wise this clears some stuff up and is more intuitive. I will be getting this rebased and hopefully we can get it merged in soon 👍 |
|
We don't have shuffle child trade though, so Keaton would replace letter right? So basically the first mask shop slot in the menu cycle would be egg or chicken or letter or Keaton, then the rest of the slots would be different masks |
3544ff1 to
e04db4c
Compare
Personally I kinda like the separation of logic between the egg/chicken/letter and the masks tracks, but I'm not hard set on that, would be a simple adjustment to make |
|
I'm just saying logically you shouldn't be able to have both a mask and a letter. I get that we're breaking that with early bunny hood, but once you get Keaton from the mask shop going back to letter doesn't make sense |
yea that's fair. I'll make that adjustment |
8210d3a to
5923d24
Compare
|
Made that adjustment and replaced all the flag checks to use the macros/methods This is ready for review again 👍 |
inspectredc
left a comment
There was a problem hiding this comment.
Everything functionally looks good to me! Just wanted to clean up how the selection option looks on the randomizer window
02fa7f1 to
c09eb8e
Compare
c09eb8e to
fdb879c
Compare
|
Did another cleanup pass on this, now using randomizer_inf for the two flags and implemented cc @briaguya-ai |
a593587 to
3644575
Compare
briaguya0
left a comment
There was a problem hiding this comment.
one last question before i give this a ✔️, one other comment/question that won't stop me from saying 
| return 0; | ||
| } | ||
|
|
||
| return INV_CONTENT(ITEM_TRADE_CHILD) != GetNextChildTradeItem(0); |
There was a problem hiding this comment.
so we'll get here in rando without mask select, in the case that we've sold the mask would this return true since "SOLD OUT" isn't the same as the mask, or does activeMaskItemId change to reflect "SOLD OUT"?
There was a problem hiding this comment.
It would behave similar to how zelda's letter would after you open the Kak gate, it will return true and if that was the only thing you had access to it just gets replaced with "ITEM_NONE"
| // Special case for bunny hood, if we want to start with it we don't care about happy mask shop status | ||
| } else if (Randomizer_GetSettingValue(RSK_STARTING_BUNNY_HOOD)) { | ||
| possibleItems.push_back(ITEM_MASK_BUNNY); | ||
| } |
There was a problem hiding this comment.
maybe out of scope for this PR, but since start with bunny hood requires mask select to work we should probably either force it on (did we ever figure out a good pattern for forcing cvar options on based on rando settings?) or update the current cvar checks to use a macro or something that checks both the cvar and the RSK
There was a problem hiding this comment.
Starting with bunny hood doesn't require mask select, you could think of it like it just pretends you borrowed the bunny hood first.
If you borrow another mask, it will replace the bunny hood
|
did a little playtesting: start with bunny hood on, mask select off, complete mask quest off, skip child zelda off, gate closed
it feels a little odd to start with the ability to enter mask select, and then lose it (effectively along with the bunny hood) after borrowing the first mask from the shop start with bunny hood off, mask select off, complete mask quest off, skip child zelda off, gate closed
this feels broken, we really shouldn't allow entering mask select at all if we don't start with bunny hood and have mask select turned off |
|
i think it's probably worth it to define expected behavior a bit (this is all for rando)
i know that this is different than what is described in the top level PR comment. i'm assuming that's to have the items consumed upon trade for a future shuffle child trade impementation, but i think it makes sense to only do that in the case of shuffle child trade, not always in rando. edit: for how to handle sold a mask with start with bunny, i think i'm leaning "swap between SOLD OUT and bunny hood" to stay consistent with the behavior of pre-mask child trade items when starting with bunny hood |
While I agree that the second point is a bit worse, and can probably be fixed, it's worth pointing out that there are two features here. Mask select != swapping child trade sequence items Mask select is "If you have a mask, you can swap between the other masks you have access to" Swapping child trade sequence is "You can swap between the trade sequence items you might still need to use" |
|
it seems the only complex situation is mask select off when starting with bunny hood, i think in that scenario we should
that way, by not treating the started with bunny hood as a child trade item, we can always have the current child trade item available as well as the bunny hood, meaning we won't need to lose it once entering the mask part of the child trade sequence edit: this is because if people are choosing to start with the bunny hood they'll be pissed if they lose it |
|
Going to redo this on rando v3 |
Summary of changes:
GetNextChildTradeItemandCanChangeChildTradeItemBuild Artifacts