Skip to content

fix: backpack init copy#28

Open
EternalQ wants to merge 2 commits intoGTNewHorizons:masterfrom
cubeofint:master
Open

fix: backpack init copy#28
EternalQ wants to merge 2 commits intoGTNewHorizons:masterfrom
cubeofint:master

Conversation

@EternalQ
Copy link
Copy Markdown

Fixes problem, when new backpack created with existed UUID.

Copy link
Copy Markdown

@Luca-Guettinger Luca-Guettinger left a comment

Choose a reason for hiding this comment

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

This change adds a UUID collision check that isn't needed. Java's UUID.randomUUID() has 122 bits of randomness, 122 bits of SecureRandom entropy specifically. You need ~2.7 * 10^18 UUIDs for a 50% collision probability. No Minecraft server will ever have that many backpacks. Collisions are not a practical concern.

If there's a real bug where two backpacks end up with the same UID, the cause is NBT data being copied between ItemStacks without clearing the UID tag (creative mode, crafting, /give, etc.). The right fix would address that root cause, not add a filesystem check in a loop for a collision that won't happen from randomUUID().

Also worth noting that backpackSaveExists hits the filesystem (HashMap lookup + File.exists()) on every call, so this adds disk I/O in a loop for something that won't happen.

No reproduction steps or linked issue provided. What scenario actually triggers "new backpack created with existed UUID"?

@EternalQ
Copy link
Copy Markdown
Author

This PR exists only because it happened on the server twice.
And that's just what I know. I'm sure there are people who simply won't talk about it.
In any case, the fix is ​​already live on the server, and I just want to prevent others from running into the same problem.

New backpacks are not created so often that it becomes an I/O problem.

No reproduction steps or linked issue provided, because that is just new Backpack with existing inventory.

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.

2 participants