Skip to content

Conversation

@Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Nov 27, 2023

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

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.
Copy link
Contributor

@briaguya0 briaguya0 left a 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

:shipit:

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 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() {
Copy link
Contributor

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(
Copy link
Contributor

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] =
Copy link
Contributor

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.

@briaguya0 briaguya0 merged commit dc43472 into HarbourMasters:develop Dec 27, 2023
@Pepe20129 Pepe20129 deleted the item_cycling_streamline branch December 27, 2023 18:02
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.

4 participants