Skip to content

Fix trade limit - #40

Merged
cubefury merged 4 commits into
masterfrom
fix-trade-limit
Nov 15, 2025
Merged

Fix trade limit#40
cubefury merged 4 commits into
masterfrom
fix-trade-limit

Conversation

@cubefury

@cubefury cubefury commented Nov 6, 2025

Copy link
Copy Markdown
Collaborator

Fixes trade limit resetting on server restart.

@cubefury
cubefury requested a review from a team November 6, 2025 09:36
Comment on lines +31 to +34
nbt.setTag(
"displayItem",
(displayItem != null ? displayItem : new BigItemStack(ItemPlaceholder.placeholder))
.writeToNBT(new NBTTagCompound()));

@Nikolay-Sitnikov Nikolay-Sitnikov Nov 6, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If displayItem is null, shouldn't this just not add a tag? The loading logic says "if there's no tag, displayItem is null". This logic says "If displayItem is null, the tag is a placeholder." These two functions are no longer inverses of each other.

Is this intentional? Maybe the readFromNBT was also supposed to have placeholder logic?

(On first load, displayItem is null. On second load, displayItem is a placeholder.)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

you're right. it technically should never be null, but i just wrote that to be safe in case readFromNBT somehow makes it a null value. I'll remove this additional logic here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll add the null check at the assignment for displayItem instead.

@Dream-Master
Dream-Master requested review from a team and Nikolay-Sitnikov November 6, 2025 12:30
@Dream-Master Dream-Master added the 🚧 Testing on Zeta Do not merge yet, testing this PR on Zeta (experimental build) label Nov 14, 2025
@cubefury
cubefury merged commit 608f50a into master Nov 15, 2025
1 check passed
@cubefury
cubefury deleted the fix-trade-limit branch November 15, 2025 10:22
@Dream-Master Dream-Master removed the 🚧 Testing on Zeta Do not merge yet, testing this PR on Zeta (experimental build) label Dec 13, 2025
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.

3 participants