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

Use existing isPlayerPlaced value to track who placed a block #243

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jeck
Copy link

@Jeck Jeck commented Feb 16, 2023

What is this?

This is a proof-of-concept of using the isPlayerPlaced value to store an identifier for the player who placed the block.

The existing behavior is to set that value to "1" if placed by a player... and then do nothing with the actual integer stored.

Since we're already storing four bytes per player-placed block, why not use them?

Why make this change?

This change allows for the easy creation of a self_placed filter, which I'll include in a PR to the libreforge repo shortly.

What good is a self_placed filter?

  • A skill that levels up when breaking other player's stuff, but not your own.
  • Weapons that deal bonus damage when standing on home turf.
  • Tools that mine your own blocks instantly.
  • Explosives that don't damage your own blocks
  • More ideas as they come to me.

How it works

This change assigns each player a local ID, and then uses that ID instead of the value "1" for the isPlayerPlaced value.

Local ID is a single INT stored stored on the PlayerProfile. Storing it on the player profile makes it a proxy value for UUID.

Why not use UUID instead of a local ID?

If additional memory use isn't a concern, UUIDs are the most robust solution. However, using a UUID here would increase storage space used by isPlayerPlaced 4x.

Doing it with local IDs, as this PR does, allows us to use the existing value and thus has no increase in per-block storage use.

Given the size of a signed 4-byte integer, a server won't run out of local IDs until Minecraft sells 2 billion copies and all of those copies come to play on a specific server.

Why not rely on the anti-grief integration?

The anti-grief integration only tracks who has permission to modify a given block. Not who placed it in the first place. Even if we were to modify the integration, not every anti-grief plugin implements a "who placed this block" function.

And again, we already store a value here. This is just making use of it.

Performance Analysis

The existing behavior is to write to the chunk's profile each time a player places a block. This behavior is unchanged.

There is now an additional read from the player's profile when placing blocks, to determine the player's local ID. Depending on how that read occurs in the code (I haven't read deeply yet, but I'm assuming it's memoized somewhere so it's not hitting the DB/Disk every read) this should have negligable impact.

Backwards Compatibility

This change doesn't break any existing behavior.

Before this PR, worlds with blocks marked by isPlayerPlaced use the value 1 to denote their placement, and no value if they were not player placed. I've ensured that the local player IDs start at 2, so that there's no conflict on migrating versions.

There's also no conflict if the server downgrades from this version; the actual value set by isPlayerPlaced is ignored prior to this PR.

@Jeck
Copy link
Author

Jeck commented Feb 16, 2023

This PR exists to enable the self_placed filter, whose PR is here: Auxilor/libreforge#84

@WillFP
Copy link
Member

WillFP commented Feb 22, 2023

Hi - a couple things:
I don't think local ID is necessary, UUID is 16-byte and I don't think that the memory overhead is significant. Instead I'd store as a byte array with the player's UUID.

@Jeck
Copy link
Author

Jeck commented Feb 22, 2023

Excellent, that's the much more robust solution!

It does impose a per-player-placed block overhead beyond what eco currently has, which in a worst case scenario could get quite large- roughly 300kb per chunk, assuming players placed every block in a chunk. But in practice players touch a tiny tiny fraction of blocks per chunk, so that's hardly worth stressing over.

I'll go ahead and redo the implementation so that local ID is not necessary. Might be a few days, but it's an even simpler code change than this one is.

@WillFP WillFP closed this Jun 17, 2023
@WillFP WillFP reopened this Jun 17, 2023
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