Skip to content
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_Give and OnReceiveItem updates #2580

Merged
merged 11 commits into from
Mar 12, 2023
Merged

Conversation

Malkierian
Copy link
Contributor

@Malkierian Malkierian commented Mar 4, 2023

OnReceiveItem now takes a GetItemEntry parameter to allow access to modIndex (futureproofing for mods).

All access to that hook now feeds in the appropriate parameter.

Randomizer_Item_Give now triggers OnReceiveItem.

Shop item giving now also implements mod ID tracking to properly feed into OnReceiveItem after pending sales go through.

Build Artifacts

…ollowing changes:

Reworked the references to it in `z_parameter` to call a single function with the item return and hook call in it for ease of editing.

Modified the pendingSale functionality to set and pass a modIndex value as well, as mod items (like randomizer) in shops still use the vanilla sale path.
Also added Randomizer_Item_Give to the OnReceiveItem system.
@briaguya-ai briaguya-ai added the do not merge Not ready or not valid changes label Mar 4, 2023
As a side effect, item autosave doesn't work for shop/scrub/merchant transactions, requires new OnSaleEnded hook that can also call the autosave.
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.

Doing a first pass through, main thing that stands out to me is a couple places where we can avoid a call to ItemTable_ReceiveItem

soh/src/overlays/actors/ovl_En_GirlA/z_en_girla.c Outdated Show resolved Hide resolved
soh/src/overlays/actors/ovl_En_Si/z_en_si.c Outdated Show resolved Hide resolved
soh/src/overlays/actors/ovl_En_Si/z_en_si.c Outdated Show resolved Hide resolved
soh/src/code/z_sram.c Outdated Show resolved Hide resolved
soh/src/code/z_sram.c Outdated Show resolved Hide resolved
soh/src/overlays/actors/ovl_player_actor/z_player.c Outdated Show resolved Hide resolved
soh/src/overlays/actors/ovl_player_actor/z_player.c Outdated Show resolved Hide resolved
@Malkierian
Copy link
Contributor Author

Yeah, I feel pretty silly about all those unnecessary RetrieveEntry calls now, lol. No, there's no reason for them.

…ry properties were being used to call it. General code cleanup.
…ale.

Migrated AutoSave to its own function, registered AutoSave function to OnReceiveItem and OnSaleEnd hooks to help with autsaving after buying items.

Some futureproofing for AutoSave function with parameters for skipping autosave, for when transition end is migrated to AutoSave function (whether through direct call or through a hook).
@Malkierian
Copy link
Contributor Author

Should be ready for merging pending another review.

@briaguya-ai briaguya-ai removed the do not merge Not ready or not valid changes label Mar 6, 2023
Copy link
Contributor

@dcvz dcvz left a comment

Choose a reason for hiding this comment

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

Looking good, just some small comments. I also don't see where you trigger the sale ended hooks, do we still need that?

soh/soh/Enhancements/mods.cpp Outdated Show resolved Hide resolved
soh/soh/Enhancements/game-interactor/GameInteractor.h Outdated Show resolved Hide resolved
soh/soh/Enhancements/mods.cpp Outdated Show resolved Hide resolved
@Malkierian
Copy link
Contributor Author

The call to OnSaleEnd happens on line 6198 of z_parameter.c, where the call used to be to OnReceiveItem. But yeah, the param name change makes sense.

@Malkierian
Copy link
Contributor Author

Oh, and as a clarification, sale Item_Gives happen earlier than OnSaleEnd in other areas of code, but still utilize autosave limitations to prevent autosaving before rupee deduction.

Refactored all references to OnReceiveItem to OnItemReceive to mirror upcoming full hook refactor for name ordering conventions.

Up-to-date with develop.
@dcvz dcvz merged commit 7305261 into HarbourMasters:develop Mar 12, 2023
@Malkierian Malkierian deleted the item-give branch March 12, 2023 07:54
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.

None yet

4 participants