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

Fix incorrect CellInventory field types #363

Merged
merged 1 commit into from
Jan 13, 2024
Merged

Conversation

NotMyWing
Copy link
Member

Fixes additional data losses not covered in #327. Enables natural support for larger cells and prevents them from overflowing past the int limit. Cells with int tags seamlessly migrate to long without regression.

The only minor concern is the change in signature for BasicCellInventory#loadCellItem(NBTTagCompound, int), which now accepts longs instead. This is not being overridden by mainstream mods such as EC2 or Thaumic Energistics, however.

Mostly on par with GTNewHorizons#234, except for native larger cells.

@NotMyWing NotMyWing added the bug Something isn't working label Jan 12, 2024
@NotMyWing NotMyWing requested a review from serenibyss January 12, 2024 07:06
@NotMyWing NotMyWing self-assigned this Jan 12, 2024
@serenibyss
Copy link
Member

This seems like a good start, though I'm sure there'll be more places to address in the future. What about

final long size = Math.min(Integer.MAX_VALUE, request.getStackSize());
?

@NotMyWing
Copy link
Member Author

This is most likely a safeguard for int-sized item/fluid stacks.

@NotMyWing NotMyWing merged commit f7aa4c9 into master Jan 13, 2024
@NotMyWing NotMyWing deleted the fix-cell-data-loss-again branch January 13, 2024 05:56
AE2-Enthusiast pushed a commit to AE2-Enthusiast/AE2-UEL that referenced this pull request Apr 14, 2024
Migrates the total item count and serialized stack sizes to `long`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants