Save block-NBT in structure module #1515
Conversation
cswhite2000
left a comment
cswhite2000
left a comment
There was a problem hiding this comment.
Commit's not signed off correctly.
Also, this should be configurable via XML. Should probably default this to false as well, to make this not a breaking change.
And we should explore if there is a more efficient way of doing this than serializing everything to NBT and then deserializing it.
| import org.bukkit.Nameable; | ||
| import org.bukkit.World; | ||
| import org.bukkit.WorldCreator; | ||
| import org.bukkit.*; |
There was a problem hiding this comment.
lets not use wildcard imports
| Region.Static staticRegion = region.getStatic(world); | ||
| worldEdit.placeBlocks(staticRegion, offset, update); | ||
| staticRegion.getBlockVectors().forEach(pos -> { | ||
| Block block = world.getBlockAt( |
There was a problem hiding this comment.
No need to make the block here, as most locations should have no nbt. So you're allocating lots of uneeded objects.
Probably better off moving the whole block creation into setBlockNBT
Something like setNBTat(World world, BlockVector vec, Object nbt)
| Region.Static staticRegion = region.getStatic(world); | ||
| staticRegion.getChunkPositions().forEach(cv -> this.saveSnapshot(cv, null)); | ||
| staticRegion.getBlockVectors().forEach(pos -> { | ||
| Block block = world.getBlockAt(pos.getBlockX(), pos.getBlockY(), pos.getBlockZ()); |
There was a problem hiding this comment.
Same here, move the block creation into the nms method.
|
|
||
| @Override | ||
| public Object getBlockNBT(Block block) { | ||
| ServerLevel level = ((CraftWorld) block.getWorld()).getHandle(); |
There was a problem hiding this comment.
We should try and see if there's an efficient way we can exit early if the location has no relevant data
|
|
||
| @Override | ||
| public Object getBlockNBT(Block block) { | ||
| TileEntity tile = ((CraftWorld) block.getWorld()) |
There was a problem hiding this comment.
Same here, this seems like an expensive way of checking to see if data is present
Pablete1234
left a comment
Pablete1234
left a comment
There was a problem hiding this comment.
I think you're severely underestimating the performance impact that this will have.
Try saving and restoring a big region (eg: bed rush map) and profile the timing and memory difference between the current implementation, which avoids allocations at every opportunity, and the proposed implementation.
If this is to land in pgm, it will have to be after some severe re-thinking of the performance implications
3 participants
This PR extends the functionality of the
<structures>module to allow any block-NBT data present within the structure region to be saved and restored. The implementation supports both 1.8 and 1.21.1 versions of PGM.