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

Add tableId to getItemEntry #3064

Merged

Conversation

garrettjoecox
Copy link
Contributor

@garrettjoecox garrettjoecox commented Jul 3, 2023

I don't love it, but these comments explain why:

uint16_t modIndex; // Primarily used for determining whether to use Item_Give or Randomizer_Item_Give
uint16_t tableId; // GetItemEntry table this entry is in (usually the same as modIndex, but not always)

My use case is I need to serialize a getItemEntry and send it across the wire, and sending over the getItemID + modIndex does not suffice because the modIndex is not 1:1 with the table the entry is stored in.

Build Artifacts

@Malkierian
Copy link
Contributor

So if I understand these changes correctly, with the new tableId, the normal GET_ITEM macro uses the modIndex, but the new macro allows setting both separately? Otherwise the functionality is all the same?

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

left a comment with an idea to possibly make it easier to read the GET_ITEM_CUSTOM_TABLE usages, not sure if it would make things overall better or worse though

soh/soh/Enhancements/randomizer/randomizer.cpp Outdated Show resolved Hide resolved
@Malkierian
Copy link
Contributor

So I guess whether we merge this would depend on whether we wanted to maintain the rando item table or not going forward.

@garrettjoecox
Copy link
Contributor Author

So I guess whether we merge this would depend on whether we wanted to maintain the rando item table or not going forward.

The rando item table is staying (at least I think?), it's the weird "in between" table that is the issue, it's stored in the randomzier table but has a vanilla mod ID

@garrettjoecox garrettjoecox force-pushed the addTableIdToGetItemEntry branch 2 times, most recently from e134c12 to 24be50e Compare November 15, 2023 04:29
@garrettjoecox garrettjoecox merged commit 365afe7 into HarbourMasters:develop Dec 4, 2023
8 checks passed
@garrettjoecox garrettjoecox deleted the addTableIdToGetItemEntry branch March 4, 2024 15:33
jamieklassen pushed a commit to jamieklassen/Shipwright that referenced this pull request Mar 10, 2024
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

3 participants