Skip to content

Save block-NBT in structure module #1515

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.

Sign up for GitHub

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

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

Save block-NBT in structure module #1515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions core/src/main/java/tc/oc/pgm/snapshot/WorldSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.HashMap;
import java.util.Map;
import org.bukkit.Bukkit;
import org.bukkit.ChunkSnapshot;
import org.bukkit.World;
import org.bukkit.block.Block;
Expand All @@ -14,10 +15,12 @@
import tc.oc.pgm.util.chunk.ChunkVector;
import tc.oc.pgm.util.material.BlockMaterialData;
import tc.oc.pgm.util.material.MaterialData;
import tc.oc.pgm.util.nms.NMSHacks;

public class WorldSnapshot {
private final World world;
private final Map<ChunkVector, ChunkSnapshot> chunkSnapshots = new HashMap<>();
private final Map<BlockVector, Object> savedNBT = new HashMap<>();
private final BudgetWorldEdit worldEdit;

public WorldSnapshot(World world) {
Expand Down Expand Up @@ -80,11 +83,36 @@ public void saveSnapshot(ChunkVector cv, @Nullable BlockState oldState) {
}

public void saveRegion(Region region) {
region.getChunkPositions().forEach(cv -> this.saveSnapshot(cv, null));
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());
Copy link
Member

@cswhite2000 cswhite2000 Jun 4, 2025

Choose a reason for hiding this comment

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

Same here, move the block creation into the nms method.

try {
Object tag = NMSHacks.NMS_HACKS.getBlockNBT(block);
if (tag != null) savedNBT.put(pos, tag);
} catch (Throwable t) {
Bukkit.getLogger().info("Failed to save NBT for block at " + pos + ": " + t.getMessage());
}
});
}

public void placeBlocks(Region region, BlockVector offset, boolean update) {
worldEdit.placeBlocks(region, offset, update);
Region.Static staticRegion = region.getStatic(world);
worldEdit.placeBlocks(staticRegion, offset, update);
staticRegion.getBlockVectors().forEach(pos -> {
Block block = world.getBlockAt(
Copy link
Member

@cswhite2000 cswhite2000 Jun 4, 2025

Choose a reason for hiding this comment

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

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)

pos.getBlockX() + offset.getBlockX(),
pos.getBlockY() + offset.getBlockY(),
pos.getBlockZ() + offset.getBlockZ());
Object tag = savedNBT.get(pos);
if (tag != null) {
try {
NMSHacks.NMS_HACKS.setBlockNBT(block, tag);
} catch (Throwable t) {
Bukkit.getLogger().info("Failed to restore NBT at " + pos + ": " + t.getMessage());
}
}
});
}

public void removeBlocks(Region region, BlockVector offset, boolean update) {
Expand Down
49 changes: 39 additions & 10 deletions platform/platform-modern/src/main/java/tc/oc/pgm/platform/modern/impl/ModernNMSHacks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,11 @@
import java.util.UUID;
import java.util.logging.Level;
import net.kyori.adventure.text.Component;
import net.minecraft.core.BlockPos;
import net.minecraft.core.HolderLookup;
import net.minecraft.core.RegistryAccess;
import net.minecraft.core.registries.Registries;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.nbt.NbtException;
import net.minecraft.nbt.ReportedNbtException;
import net.minecraft.resources.ResourceKey;
Expand All @@ -31,19 +34,16 @@
import net.minecraft.world.level.CustomSpawner;
import net.minecraft.world.level.GameRules;
import net.minecraft.world.level.biome.BiomeManager;
import net.minecraft.world.level.block.entity.BlockEntity;
import net.minecraft.world.level.block.state.BlockState;
import net.minecraft.world.level.chunk.LevelChunkSection;
import net.minecraft.world.level.chunk.status.ChunkStatus;
import net.minecraft.world.level.dimension.LevelStem;
import net.minecraft.world.level.storage.LevelDataAndDimensions;
import net.minecraft.world.level.storage.LevelStorageSource;
import net.minecraft.world.level.storage.PrimaryLevelData;
import net.minecraft.world.level.validation.ContentValidationException;
import org.bukkit.Bukkit;
import org.bukkit.Chunk;
import org.bukkit.Material;
import org.bukkit.Nameable;
import org.bukkit.World;
import org.bukkit.WorldCreator;
import org.bukkit.*;
Copy link
Member

@cswhite2000 cswhite2000 Jun 4, 2025

Choose a reason for hiding this comment

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

lets not use wildcard imports

import org.bukkit.block.Block;
import org.bukkit.craftbukkit.CraftChunk;
import org.bukkit.craftbukkit.CraftServer;
Expand All @@ -52,10 +52,7 @@
import org.bukkit.craftbukkit.entity.CraftFirework;
import org.bukkit.craftbukkit.generator.CraftWorldInfo;
import org.bukkit.craftbukkit.util.CraftMagicNumbers;
import org.bukkit.entity.Entity;
import org.bukkit.entity.Fireball;
import org.bukkit.entity.Firework;
import org.bukkit.entity.Player;
import org.bukkit.entity.*;
import org.bukkit.event.player.PlayerPickupArrowEvent;
import org.bukkit.event.player.PlayerPickupItemEvent;
import org.bukkit.event.world.WorldLoadEvent;
Expand Down Expand Up @@ -393,4 +390,36 @@ public int getMaxWorldSize(World world) {
public int allocateEntityId() {
return Bukkit.getUnsafe().nextEntityId();
}

@Override
public Object getBlockNBT(Block block) {
ServerLevel level = ((CraftWorld) block.getWorld()).getHandle();
Copy link
Member

@cswhite2000 cswhite2000 Jun 4, 2025

Choose a reason for hiding this comment

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

We should try and see if there's an efficient way we can exit early if the location has no relevant data

BlockPos pos = new BlockPos(block.getX(), block.getY(), block.getZ());
BlockEntity be = level.getBlockEntity(pos);

if (be == null) return null;

CompoundTag tag = be.saveWithFullMetadata(level.registryAccess());
return tag;
}

@Override
public void setBlockNBT(Block block, Object tag) {
if (!(tag instanceof CompoundTag nbt)) return;

ServerLevel level = ((CraftWorld) block.getWorld()).getHandle();
BlockPos pos = new BlockPos(block.getX(), block.getY(), block.getZ());
BlockState state = level.getBlockState(pos);

BlockEntity existing = level.getBlockEntity(pos);
if (existing != null) {
level.removeBlockEntity(pos);
}

HolderLookup.Provider registryLookup = level.registryAccess();
BlockEntity loaded = BlockEntity.loadStatic(pos, state, nbt, registryLookup);
if (loaded != null) {
level.setBlockEntity(loaded);
}
}
}
56 changes: 43 additions & 13 deletions ...form/platform-sportpaper/src/main/java/tc/oc/pgm/platform/sportpaper/impl/SpNMSHacks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,11 @@
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import net.minecraft.server.v1_8_R3.ChunkSection;
import net.minecraft.server.v1_8_R3.EntityArrow;
import net.minecraft.server.v1_8_R3.EntityFireball;
import net.minecraft.server.v1_8_R3.EntityFireworks;
import net.minecraft.server.v1_8_R3.IBlockData;
import net.minecraft.server.v1_8_R3.IDataManager;
import net.minecraft.server.v1_8_R3.NBTTagCompound;
import net.minecraft.server.v1_8_R3.ServerNBTManager;
import net.minecraft.server.v1_8_R3.WorldData;
import net.minecraft.server.v1_8_R3.WorldServer;
import org.bukkit.Bukkit;
import net.minecraft.server.v1_8_R3.*;
import org.bukkit.*;
import org.bukkit.Chunk;
import org.bukkit.Material;
import org.bukkit.World;
import org.bukkit.WorldCreator;
import org.bukkit.block.Block;
import org.bukkit.craftbukkit.v1_8_R3.CraftChunk;
import org.bukkit.craftbukkit.v1_8_R3.CraftWorld;
Expand Down Expand Up @@ -98,7 +88,6 @@ public void resumeServer() {
@Override
public Inventory createFakeInventory(Player viewer, Inventory realInventory) {
if (realInventory.hasCustomName()) {
//noinspection deprecation
return realInventory instanceof DoubleChestInventory
? Bukkit.createInventory(viewer, realInventory.getSize(), realInventory.getName())
: Bukkit.createInventory(viewer, realInventory.getType(), realInventory.getName());
Expand Down Expand Up @@ -229,4 +218,45 @@ public int getMaxWorldSize(World world) {
public int allocateEntityId() {
return Bukkit.allocateEntityId();
}

@Override
public Object getBlockNBT(Block block) {
TileEntity tile = ((CraftWorld) block.getWorld())
Copy link
Member

@cswhite2000 cswhite2000 Jun 5, 2025

Choose a reason for hiding this comment

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

Same here, this seems like an expensive way of checking to see if data is present

.getHandle()
.getTileEntity(new BlockPosition(block.getX(), block.getY(), block.getZ()));
if (tile == null) return null;

NBTTagCompound tag = new NBTTagCompound();
tile.b(tag); // Save
return tag;
}

@Override
public void setBlockNBT(Block block, Object tag) {
if (!(tag instanceof NBTTagCompound)) return;

WorldServer world = ((CraftWorld) block.getWorld()).getHandle();
BlockPosition pos = new BlockPosition(block.getX(), block.getY(), block.getZ());
NBTTagCompound compound = (NBTTagCompound) tag;

compound.setInt("x", pos.getX());
compound.setInt("y", pos.getY());
compound.setInt("z", pos.getZ());

TileEntity tile = world.getTileEntity(pos);
if (tile == null) {
// Force block update to trigger tile entity re-creation
IBlockData blockData = world.getType(pos);
world.setTypeAndData(pos, Blocks.AIR.getBlockData(), 0); // Clear block
world.setTypeAndData(pos, blockData, 3); // Restore block

tile = world.getTileEntity(pos);
if (tile == null) {
return;
}
}

tile.a(compound); // Load NBT
tile.update();
}
}
4 changes: 4 additions & 0 deletions util/src/main/java/tc/oc/pgm/util/nms/NMSHacks.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,8 @@ public interface NMSHacks {
int getMaxWorldSize(World world);

int allocateEntityId();

Object getBlockNBT(Block block);

void setBlockNBT(Block block, Object tag);
}