Skip to content

Trade cleanup#4971

Merged
aMannus merged 11 commits intoHarbourMasters:developfrom
Pepe20129:trade_cleanup
Feb 25, 2025
Merged

Trade cleanup#4971
aMannus merged 11 commits intoHarbourMasters:developfrom
Pepe20129:trade_cleanup

Conversation

@Pepe20129
Copy link
Contributor

@Pepe20129 Pepe20129 commented Jan 29, 2025

Move adult trade items to use randomizer inf flags when in rando instead of their own special system.
Also move child trade items to flags when in rando, fixing the "deleting weird egg/chicken" issue.

Build Artifacts

@briaguya0
Copy link
Contributor

any chance this fixes #4957?

aMannus
aMannus previously requested changes Feb 3, 2025
Copy link
Contributor

@aMannus aMannus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I'm loving the reduction in complexity, definitely the right path forward.

That said, this does touch quite a lot of vanilla code. I know some of it already existed so I'm not going to force you to address those in this PR, but I'd like to atleast ask to hookify the new code.

Comment on lines -2449 to +2459
if (item >= ITEM_POCKET_EGG) {
gSaveContext.ship.quest.data.randomizer.adultTradeItems |= ADULT_TRADE_FLAG(item);
if (IS_RANDO) {
if (item >= ITEM_POCKET_EGG) {
Flags_SetRandomizerInf(item - ITEM_POCKET_EGG + RAND_INF_ADULT_TRADES_HAS_POCKET_EGG);
} else if (item == ITEM_LETTER_ZELDA) {
//don't care about zelda's letter if it's already been shown to the guard
if (!Flags_GetInfTable(INFTABLE_SHOWED_ZELDAS_LETTER_TO_GATE_GUARD)) {
Flags_SetRandomizerInf(RAND_INF_CHILD_TRADES_HAS_LETTER_ZELDA);
}
} else {
Flags_SetRandomizerInf(item - ITEM_WEIRD_EGG + RAND_INF_CHILD_TRADES_HAS_WEIRD_EGG);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really feel like I can force this on you in this PR because this is touching code that already existed, but if you're open for it, would you mind moving this to a hook as well? If not it'll have to be done with a wider cleanup effort in the future.

Comment on lines -336 to -341
INV_CONTENT(ITEM_TRADE_CHILD) <= ITEM_MASK_KEATON || INV_CONTENT(ITEM_TRADE_CHILD) > ITEM_MASK_TRUTH ?
ITEM_MASK_TRUTH :
INV_CONTENT(ITEM_TRADE_CHILD) - 1,
INV_CONTENT(ITEM_TRADE_CHILD) >= ITEM_MASK_TRUTH || INV_CONTENT(ITEM_TRADE_CHILD) < ITEM_MASK_KEATON ?
ITEM_MASK_KEATON :
INV_CONTENT(ITEM_TRADE_CHILD) + 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before, not going to force you to change this now as it touches existing code, but it'd be really nice to get all the trade item code in z_kaleido_item.c moved over to hooks as well.

@aMannus aMannus dismissed their stale review February 24, 2025 07:22

Changes made

@aMannus aMannus merged commit 0638706 into HarbourMasters:develop Feb 25, 2025
5 checks passed
@Pepe20129 Pepe20129 deleted the trade_cleanup branch February 28, 2025 19:35
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.

3 participants