Skip to content

Add tableId to getItemEntry#3064

Merged
garrettjoecox merged 1 commit intoHarbourMasters:developfrom
garrettjoecox:addTableIdToGetItemEntry
Dec 4, 2023
Merged

Add tableId to getItemEntry#3064
garrettjoecox merged 1 commit intoHarbourMasters:developfrom
garrettjoecox:addTableIdToGetItemEntry

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

Malkierian commented Jul 14, 2023

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
briaguya0
briaguya0 reviewed Sep 5, 2023
View reviewed changes
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.

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

Malkierian commented Sep 30, 2023

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

garrettjoecox commented Oct 1, 2023

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
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

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