-
Notifications
You must be signed in to change notification settings - Fork 104
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
[Enhancement] EasyMaskEquip #755
base: develop
Are you sure you want to change the base?
Conversation
I think this is already in a pretty good working state so I'm just gonna go ahead and change this to ready for review. I don't foresee this having to many issues, but please do report any bugs you may find. |
… plus moved CHECK_GIVEN_MASK_ON_MOON to macros.h
5a50f24
to
4f08dac
Compare
…r is busy with an action
… any mask in the air when wearing a transformation mask
- Streamlined mask selection logic with helper functions - Added grayscale indicators for unavailable masks in the UI - Optimized vertex handling for mask border rendering - Improved game hook integrations for reliable mask equipping - Enhanced code readability and maintainability
…kinian into easymaskequip
…kinian into easymaskequip
I realize ShouldEquipMask may not account for all scenarios, but its a good start. I would need help identifying what else we may need to add. |
…kinian into easymaskequip
Co-Authored-By: Garrett Cox <garrettjcox@gmail.com>
63 commits later and I think I've gotten this to a state I'm happy with. I don't plan to make any more changes unless the devs request otherwise. Hope yall like it! |
Previously, Easy Mask Equip supported both regular and transformation masks, which forced disabling some vanilla behaviors and risked breaking existing logic. To avoid discarding all my work, this update refocuses the feature solely on transformation masks as an extension of Fast Transformation. This change simplifies the implementation via a clear state machine for handling pending actions and frees up the C and D pad buttons for other items, preserving the intended quality-of-life improvements. |
…equip" This reverts commit c4a7b5b.
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.
Though I know the interface is similar to the persistent bunny hood, the user having to unpause to engage the transformation just feels awkward, I'm not really sure how I feel about this one.
|
||
// Hook for controlling the display of the selected item textbox. | ||
COND_VB_SHOULD(VB_KALEIDO_DISPLAY_ITEM_TEXT, CVAR, { | ||
*should = !isTransformationMask(static_cast<ItemId>(gPlayState->pauseCtx.cursorItem[PAUSE_MASK])); |
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.
This appears to be causing an issue with persistent bunny hood, causing it to show the text when toggling it.
Instead I think you want
*should = !isTransformationMask(static_cast<ItemId>(gPlayState->pauseCtx.cursorItem[PAUSE_MASK])); | |
if (isTransformationMask(static_cast<ItemId>(gPlayState->pauseCtx.cursorItem[PAUSE_MASK]))) { | |
*should = false; | |
} |
|
||
/// Returns true if the given mask is a transformation mask. | ||
bool isTransformationMask(ItemId mask) { | ||
return std::binary_search(kTransformationMasks.begin(), kTransformationMasks.end(), mask); |
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 are so little masks here it's probably more simple to just do a direct comparison inline than store them in an array and iterate over it.
for (s16 slot = 0; slot < MASK_NUM_SLOTS; ++slot) { | ||
if (items[slot + ITEM_NUM_SLOTS] == target) | ||
return slot; | ||
} |
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.
Using the SLOT() macro here should get you what you want (I think?)
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.
The gItemSlots array maps itemIDs to their slots, but looks like the mapping is only applicable for regular item maybe? I couldn't get it to work with the masks.
I pushed an update that not only addresses some of you feedback but also goes a bit overboard with a pretty extensive refactor. The changes include streamlining state management with a scoped enum and a dedicated state tracker, introducing a new VertexBuffer for more robust vertex handling, and encapsulating input and state logic into a new MaskEquipManager. The interface has got a upgrade as well and should hopefully address that awkwardness you mentioned. Fair warning: it's a pretty hefty update, so you might want to grab a coffee before diving in lol. Let me know if you have any questions or need further tweaks! |
EasyMaskEquip is a feature that allows players to quickly equip masks without needing to assign them to C-buttons or D-pad slots. It simplifies the process by letting players equip masks directly from the pause menu. The enhancement also provides visual feedback by highlighting the currently equipped mask and graying out masks that can't be equipped in certain situations (e.g., underwater or mid-transformation).
This PR introduces the EasyMaskEquip enhancement, allowing players to equip masks directly from the pause menu by pressing the A button.A new OnKaleidoClose hook was added to handle mask equipping when the pause menu is closed. This allows players to change their mask selection if they accidentally select the wrong one, ensuring the mask is only equipped once the pause menu is closed.
This enhancement also utilizes Proxy's BeforeKaleidoDrawPage hook from PR #537 to allocate and update vertex data for the mask equip border.Build Artifacts