Skip to content

Add tableId to getItemEntry #3064

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 4, 2023
Merged

Add tableId to getItemEntry #3064

merged 1 commit into from
Dec 4, 2023

Conversation

garrettjoecox
Copy link
Contributor

@garrettjoecox garrettjoecox commented Jul 3, 2023 •
edited by github-actions bot
Loading

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?

@garrettjoecox garrettjoecox force-pushed the addTableIdToGetItemEntry branch from ce7ebf8 to fde5c3d Compare August 16, 2023 01:27
@garrettjoecox garrettjoecox force-pushed the addTableIdToGetItemEntry branch from fde5c3d to c9ad725 Compare September 3, 2023 13:01
briaguya-ai
briaguya-ai reviewed Sep 5, 2023
View reviewed changes
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

@garrettjoecox garrettjoecox force-pushed the addTableIdToGetItemEntry branch from c9ad725 to 1273abd Compare September 18, 2023 20:54
@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 force-pushed the addTableIdToGetItemEntry branch from 24be50e to 2b88620 Compare December 2, 2023 20:47
@garrettjoecox garrettjoecox force-pushed the addTableIdToGetItemEntry branch from 2b88620 to 87165b1 Compare December 4, 2023 13:27
@garrettjoecox garrettjoecox merged commit 365afe7 into HarbourMasters:develop Dec 4, 2023
@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
@garrettjoecox @jamieklassen
Add tableId to getItemEntry (HarbourMasters#3064)
18b1e12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@briaguya-ai briaguya-ai briaguya-ai approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@garrettjoecox @Malkierian @briaguya-ai