-
Notifications
You must be signed in to change notification settings - Fork 632
Item cycling improvements #3456
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
Item cycling improvements #3456
Conversation
This was previously done with `Inventory_ReplaceItem` but that led to problems when another slot had the same item as the one that's being cycled.
briaguya0
left a comment
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.
tested the mask select aspect of it locally and it all seems to be working as expected
![]()
Archez
left a comment
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.
Beautiful cleanup! Thank you for finishing this up, I was going to do something similar after #2368 but you bet me to it 🙂
Just left a couple non-blocking suggestions below.
| return gCurrentItemCyclingSlot != -1; | ||
| } | ||
|
|
||
| void KaleidoScope_ResetTradeSelect() { |
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.
Could probably rename this to KaleidoScope_ResetItemCycling() so the name matches the rest of the refactors.
| } | ||
|
|
||
| void KaleidoScope_HandleItemCycles(PlayState* play) { | ||
| KaleidoScope_HandleItemCycleExtras( |
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.
It would be nice to have a comment per each handle/draw pairs calling out which it is for (technically the SLOT_ enum sorta explains it, but a comment wouldn't hurt).
| true | ||
| ); | ||
|
|
||
| gSlotAgeReqs[SLOT_TRADE_CHILD] = |
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.
A comment for the req changes would also be nice.
This PR improves several aspects of the item cycling implementation making it easier to modify (or add new item cycles) in code.
It also makes items for the wrong age grayed out when they're the previous or next item in the cycle.
Build Artifacts