[1.10.2] [SOLVED] Saving additional information to chunk data

[Bektor]

Hi,

 

I'm wondering how I can save additional information to a chunk like for example that every chunk has a different polution value

which is changed by machines. Like placing more machines in chunk x will result in a higher polution of chunk x than a chunk without

a machine has. Also machines will have different polution values depending of which machine it is, for example machine x adds 10

to the chunk polution while machine y adds 15.

 

- How can I save this additional information to every chunk?

- How can a block change this number? (make it larger when placed or the block does something, make it smaller when the player breaks the block)

 

Thx in advance.

Bektor

Edited June 19, 2017 by Bektor

[V0idWa1k3r]

Forge has chunk-data-related events. ChunkDataEvent.Save and ChunkDataEvent.Load. Both give you the NBT of the chunk. Handle the load event, load your data, pack it into your preferred way of storing data, put it into something like a map, sync it if needed and you are done. Handle the Save event and save your data. Handle the ChunkEvent.Unload to remove data from your map. Access the map through any means you like. Not the cleanest sollution as map accessis somewhat expensive but unfortunately you can't attach a capability to a chunk as chunks are not instances of ICapabilityProvider. At least not yet.

[Draco18s]

There are three events you need:

https://github.com/Draco18s/ReasonableRealism/blob/master/src/main/java/com/draco18s/flowers/FlowerEventHandler.java#L101-L118

One for reading the data, one for writing the data, and one from freeing the data from RAM.

[Bektor]

Ok.  So how do I save my data within those events now?

 

@Draco18s I see you've done it with some ChunkCoords stuff and NBTData and HashMaps etc. Also

for what are those ChunkCoords?

 

And when saving my data, the event is required to know the data to be saved, but from where should it

know the data to be saved? I mean the block which will be placed has somehow call the event and tell

it which data should be saved on top of the already existing data 

(for example existing data: 8.4f, data from new block: 1.1f -> new chunk data: 9.5f). So how am I able

to achieve this?

 

[V0idWa1k3r]

No, you store the data in a map, modify the data in the map when needed and in your save event handling you save the data from the map that corresponds to the chunk saved.

5 minutes ago, Bektor said:

Also

for what are those ChunkCoords

They are the key for the map. Those a coordinates of the chunk you use to access and store the data in your map. 

[Bektor]

Ok, so I've got now this code, thought I don't know if this will work the way I just implemented it.

And from where do I get the data to be saved? I mean, every block should set the data, thought I've got

no clue how I should let every block itself write the data to the chunk.

I have also no idea how the unload stuff should be implemented.

 

public class ChunkEvents {
    
    @SubscribeEvent
    public void onChunkLoad(ChunkDataEvent.Load event) {
        if(event.getWorld().isRemote)
            return;
        
        PolutionData.readData(event.getWorld(), event.getChunk().xPosition, event.getChunk().zPosition, event.getData());
    }
    
    @SubscribeEvent
    public void onChunkUnload(ChunkEvent.Unload event) {
        if(event.getWorld().isRemote)
            return;
        
        PolutionData.freeData(event.getWorld(), event.getChunk().xPosition, event.getChunk().zPosition);
    }
    
    @SubscribeEvent
    public void onChunkSave(ChunkDataEvent.Save event) {
        if(event.getWorld().isRemote)
            return;
        
        PolutionData.saveData(event.getWorld(), event.getChunk().xPosition, event.getChunk().zPosition, event.getData());
    }
}

public class PolutionData {
    
    private static final String KEY = "chunkPolution";
    // ConcurrentHashMap to allow multiply access at the same time
    private static ConcurrentHashMap<ChunkPos, Integer> data = new ConcurrentHashMap<>();
    
    public static void readData(World world, int chunkX, int chunkZ, NBTTagCompound compound) {
        if(compound.hasKey(KEY)) {
            ChunkPos key = new ChunkPos(chunkX, chunkZ);
            data.put(key, compound.getInteger(KEY));
        } else {
            ChunkPos key = new ChunkPos(chunkX, chunkZ);
            data.put(key, 0);
        }
    }
    
    public static void freeData(World world, int chunkX, int chunkZ) {
        // no clue how this should work
    }
    
    public static void saveData(World world, int chunkX, int chunkZ, NBTTagCompound compound) {
        NBTTagCompound nbt = new NBTTagCompound();
        data.forEach((key, value) -> {
            nbt.setInteger(key.toString(), value);
        });
        
        compound.setTag(KEY, nbt);
    }
}

 

[V0idWa1k3r]

5 minutes ago, Bektor said:

I have also no idea how the unload stuff should be implemented.

 

Just remove your data that corresponds the chunk coordinates from a map. That's all. When a chunk is unloaded it is saved first so you are free to remove the data. 

7 minutes ago, Bektor said:

I mean, every block should set the data, thought I've got

no clue how I should let every block itself write the data to the chunk.

Upon placement of your block you get the data that corresponds to a chunk the block is placed in and do something with that data(increment the pollution for example). Upon your block being broken you do the same but you decrement the pollution. You do not need to write data to a chunk from a block, you already write the data(that is obtained from an underlying map) to a chunk every time the chunk is saved.

[mrAppleXZ]

Also, if you want to sync data with clients on the server, you can use ChunkWatchEvent.

public void onChunkWatch(ChunkWatchEvent.Watch e)
    {
        //send sync packet to e.getPlayer();
    }

    public void onChunkUnwatch(ChunkWatchEvent.UnWatch e)
    {
        //send desync packet to e.getPlayer();
    }

 

[Bektor]

On 6/5/2017 at 3:50 PM, V0idWa1k3r said:

Just remove your data that corresponds the chunk coordinates from a map. That's all. When a chunk is unloaded it is saved first so you are free to remove the data. 

Upon placement of your block you get the data that corresponds to a chunk the block is placed in and do something with that data(increment the pollution for example). Upon your block being broken you do the same but you decrement the pollution. You do not need to write data to a chunk from a block, you already write the data(that is obtained from an underlying map) to a chunk every time the chunk is saved.

Ok, but how do I get the chunkX and chunkZ positions within the onBlockAdded and breakBlock method?

So that I would be able to call something like an increment method within PolutionData which just gets the object at the given chunk position from my ConcurrentHashMap data 

and replaced the value of it with a new value.

 

On 6/5/2017 at 9:49 PM, mrAppleXZ said:

Also, if you want to sync data with clients on the server, you can use ChunkWatchEvent.

public void onChunkWatch(ChunkWatchEvent.Watch e)
    {
        //send sync packet to e.getPlayer();
    }

    public void onChunkUnwatch(ChunkWatchEvent.UnWatch e)
    {
        //send desync packet to e.getPlayer();
    }

 

Hm, interesting. Now I am wondering how I can send this data to the player when he looks at a specific block in the chunk or for example when he has an item in his inventory.

[Draco18s]

6 minutes ago, Bektor said:

Ok, but how do I get the chunkX and chunkZ positions within the onBlockAdded and breakBlock method?

How would you get the chunkX and chunkZ position from any blockpos?

[Bektor]

14 minutes ago, Draco18s said:

How would you get the chunkX and chunkZ position from any blockpos?

I guess *16.

So now I'm just wondering about the data sync. How can I sync this data when the player is looking at a specific block or has a specific item in his inventory.

 

Also with my current code is the data automatically created and saved for each new and existing chunk without having to place a block from my mod?

(existing chunk when loading an old world before my mod was installed).

Edited June 8, 2017 by Bektor

[Draco18s]

2 hours ago, Bektor said:

I guess *16.

Why would you multiply?

[Choonster]

You can also use the ChunkPos(BlockPos) constructor to convert a BlockPos to a ChunkPos.

[Bektor]

11 hours ago, Draco18s said:

Why would you multiply?

Well, I guess the idea came in mind because I worked some time with world generation where you would do chunkX * 16 stuff.

4 hours ago, Choonster said:

You can also use the ChunkPos(BlockPos) constructor to convert a BlockPos to a ChunkPos.

Ah, totally missed this constructor.

 

Also I'm getting a null pointer somewhere in my code:

java.util.concurrent.ExecutionException: java.lang.NullPointerException
	at java.util.concurrent.FutureTask.report(Unknown Source) ~[?:1.8.0_131]
	at java.util.concurrent.FutureTask.get(Unknown Source) ~[?:1.8.0_131]
	at net.minecraft.util.Util.runTask(Util.java:29) [Util.class:?]
	at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:743) [MinecraftServer.class:?]
	at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:688) [MinecraftServer.class:?]
	at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:156) [IntegratedServer.class:?]
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:537) [MinecraftServer.class:?]
	at java.lang.Thread.run(Unknown Source) [?:1.8.0_131]
Caused by: java.lang.NullPointerException
	at minecraftplaye.justanotherenergy.common.lib.PolutionData.change(PolutionData.java:19) ~[PolutionData.class:?]
	at minecraftplaye.justanotherenergy.common.blocks.BlockSolarPanel.onBlockAdded(BlockSolarPanel.java:39) ~[BlockSolarPanel.class:?]
	at net.minecraft.world.chunk.Chunk.setBlockState(Chunk.java:660) ~[Chunk.class:?]
	at net.minecraft.world.World.setBlockState(World.java:388) ~[World.class:?]
	at net.minecraft.item.ItemBlock.placeBlockAt(ItemBlock.java:184) ~[ItemBlock.class:?]
	at net.minecraft.item.ItemBlock.onItemUse(ItemBlock.java:60) ~[ItemBlock.class:?]
	at net.minecraftforge.common.ForgeHooks.onPlaceItemIntoWorld(ForgeHooks.java:780) ~[ForgeHooks.class:?]
	at net.minecraft.item.ItemStack.onItemUse(ItemStack.java:159) ~[ItemStack.class:?]
	at net.minecraft.server.management.PlayerInteractionManager.processRightClickBlock(PlayerInteractionManager.java:509) ~[PlayerInteractionManager.class:?]
	at net.minecraft.network.NetHandlerPlayServer.processTryUseItemOnBlock(NetHandlerPlayServer.java:706) ~[NetHandlerPlayServer.class:?]
	at net.minecraft.network.play.client.CPacketPlayerTryUseItemOnBlock.processPacket(CPacketPlayerTryUseItemOnBlock.java:68) ~[CPacketPlayerTryUseItemOnBlock.class:?]
	at net.minecraft.network.play.client.CPacketPlayerTryUseItemOnBlock.processPacket(CPacketPlayerTryUseItemOnBlock.java:13) ~[CPacketPlayerTryUseItemOnBlock.class:?]
	at net.minecraft.network.PacketThreadUtil$1.run(PacketThreadUtil.java:21) ~[PacketThreadUtil$1.class:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) ~[?:1.8.0_131]
	at java.util.concurrent.FutureTask.run(Unknown Source) ~[?:1.8.0_131]
	at net.minecraft.util.Util.runTask(Util.java:28) ~[Util.class:?]
	... 5 more

public class PolutionData { // line 11
    
    private static final String KEY = "chunkPolution";
    // ConcurrentHashMap to allow multiply access at the same time
    private static ConcurrentHashMap<ChunkPos, Integer> data = new ConcurrentHashMap<>();
    
    public static void change(BlockPos pos, int dataToChange) {
        ChunkPos chunkPos = new ChunkPos(pos);
        data.replace(chunkPos, data.get(chunkPos), (data.get(chunkPos) + dataToChange)); // line 19
    }
    
    public static int get(BlockPos pos) {
        return data.get(new ChunkPos(pos));
    }
    
    public static void readData(World world, int chunkX, int chunkZ, NBTTagCompound compound) {
        if(compound.hasKey(KEY)) {
            ChunkPos key = new ChunkPos(chunkX, chunkZ);
            data.put(key, compound.getInteger(KEY));
        } else {
            ChunkPos key = new ChunkPos(chunkX, chunkZ);
            data.put(key, 0);
        }
    }
    
    public static void freeData(World world, int chunkX, int chunkZ) {
        ChunkPos pos = new ChunkPos(chunkX, chunkZ);
        if(data.containsKey(pos))
            data.remove(pos);
    }
    
    public static void saveData(World world, int chunkX, int chunkZ, NBTTagCompound compound) {
        NBTTagCompound nbt = new NBTTagCompound();
        data.forEach((key, value) -> {
            nbt.setInteger(key.toString(), value);
        });
        
        compound.setTag(KEY, nbt);
    }
}
      
public class BlockSolarPanel extends Block {
	[...]
	@Override
	public void onBlockAdded(World worldIn, BlockPos pos, IBlockState state) {
	    super.onBlockAdded(worldIn, pos, state);
	    
	    PolutionData.change(pos, 32); // line 39
	}
	
	@Override
	public void breakBlock(World worldIn, BlockPos pos, IBlockState state) {
	    super.breakBlock(worldIn, pos, state);
	    
	    PolutionData.change(pos, -32); // line 46
	}
	
	@Override
	public void updateTick(World worldIn, BlockPos pos, IBlockState state, Random rand) {
	    // TODO Auto-generated method stub
	    super.updateTick(worldIn, pos, state, rand);
	    
	    System.out.println(PolutionData.get(pos)); // line 54
	}
  [...]
}

 

So as chunkPos isn't null and dataToChange is also not null and the list data can't be null as it get's initialized direclty on creation which leads to the assumption that the chunk I've placed the block in is not in the list. Thought it should be.

Edited June 9, 2017 by Bektor

[Draco18s]

1 hour ago, Bektor said:

Well, I guess the idea came in mind because I worked some time with world generation where you would do chunkX * 16 stuff.

That would be chunkpos -> blockpos, not blockpos -> chunkpos

[Bektor]

Anyone?

[Bektor]

17 minutes ago, diesieben07 said:

1. Use << 4 and >> 4 to convert between chunk coords and block coords. Division does not work properly with negative numbers (-15 / 16 is 0, but it should be -1 for chunk coords).

2. This is complete bullocks.

3. What even? I assume this is trying to be threadsafe in some way? It definitely isn't, if you want to be threadsafe here use the ConcurrentMap::compute method: map.compute(key, (k, v) -> v == null ? 1 : v + 1). But you should not need to be threadsafe here. Why are you trying to be?

4. You are getting a NPE because you are trying to add to the value in the map, but that value is null. You can't add to null.

To 2: Well, this is what I understood what it is doing: 

Quote

[...] supporting full concurrency of retrievals and adjustable expected concurrency for updates [...]

To 3: Well, currently it's all running in one thread so a normal HashMap should do fine, but I'm thinking of moving some of the logic which will require this data into a different thread, as far as it is possible to do such a thing within Minecraft. Also what is the difference between ConcurrentMap::compute and ConcurrentMap::replace?

To 4: Hm, it shouldn't be null, atleast from what I wanted to do. The plan was that every chunk has as a default value a polution of 0, so there wouldn't be a null.

[Bektor]

44 minutes ago, diesieben07 said:

Yes, but it has nothing to do with "multiple access". ConcurrentHashMap allows threads to read and modify the map concurrently.

 

First of all, read the Javadocs. But in brevity: replace checks if the value for a given key is as expected and if so, replaces it with the new value. It does all this atomically (as opposed to calling get and then put inside an if statement). This is important for thread-safety. compute applies the given function to whatever value is currently stored for the key and stores the result and again it does all this atomically. You can do everything that you can do with compute with replace, but you would need a while loop and it would be less efficient.

 

Well, you do this in readData.  readData will only be called for chunks loaded from disk, so a chunk that is freshly generated by the terrain generator will not have a default value. I suggest you just tread null in the map properly instead of trying to introduce the default value at every possible occasion a chunk might be created.

Ah, ok. Fixed this thing now. Just having some problems with saving and reading the data to/from NBT left.

Either the data is just always 0 or with changing a few lines of NBT saving stuff, Minecraft completly crashes (removing the containsKey in saveData for example results into an NPE)

    public static void readData(World world, int chunkX, int chunkZ, NBTTagCompound compound) {
        if(compound.hasKey(KEY)) {
            ChunkPos key = new ChunkPos(chunkX, chunkZ);
            data.put(key, compound.getInteger(KEY));
        }
    }
    
    public static void freeData(World world, int chunkX, int chunkZ) {
        ChunkPos pos = new ChunkPos(chunkX, chunkZ);
        if(data.containsKey(pos))
            data.remove(pos);
    }
    
    public static void saveData(World world, int chunkX, int chunkZ, NBTTagCompound compound) {
        ChunkPos key = new ChunkPos(chunkX, chunkZ);
        if(data.containsKey(key))
            compound.setInteger(KEY, data.get(key));
    }

 

[Bektor]

34 minutes ago, diesieben07 said:

Yes, of course it will crash, because if the map does not contain the key Map::get will return null. You then try to unbox that null Integer into an int, which obviously cannot work.

What makes you think the data is 0? Have you used the debugger?

By the way, why is all this stuff static? You can't just have a global map, there can be many chunks with the same coordinates, in different dimensions. You should probably use a world capability or WorldSavedData.

Well, this stuff is all static so because it gets called in the event methods itself, so the methods I created for ChunkDataEvent.Load, ChunkEvent.Unload and ChunkDataEvent.Save.

Also I haven't used the debugger to find out the data is 0, my block just calls the following method when I right click on it and prints the result to the console and in that case the result was always 0.

    public static int get(BlockPos pos) {
        return data.get(new ChunkPos(pos));
    }

 

As of why the map is static, its basically because that way I can move the map later into an API so other mods can easily change the polution data and can read the data to display it or do other stuff with it.

 

EDIT: Using the debug mode will also show me that the value data.get(key) within saveData is 0. And I think an easy way to solve the problem with mutliply dimensions having the same chunk coordinates would be to also store the dimension id within the map.

Edited June 9, 2017 by Bektor

[Bektor]

12 hours ago, diesieben07 said:

How is that even remotely close to a valid reason?

 

Well, that way the stuff is changes always, it doesn't matter where I change it and I don't have to create a new instance of the map etc. everytime the event is called.

Also all blocks don't need to create or access some instance of it to be able to get values.

 

12 hours ago, diesieben07 said:

Well, start using the debugger. See if you even put anything in the map at all.

Ok, I created now a new world to get rid of all old NBT data which might have been saved and the result with the debugger is that the first block placed in the chunk will always have the value 0, which should be quite easy to fix.

But it seems like the problem that it was always 0 came from a lot of testing with saving the stuff to NBT etc.

 

12 hours ago, diesieben07 said:

This is, once a gain, not a reason. You would need the mods to specify the world anyways.

 

What do you mean by it that mods need to specify the world anyways? 

Also in your last answer you suggested using world capability or WorldSavedData. So what's exactly the difference between these two and which one would be a better choice to give also other mods access to the polution data saved per chunk so that these mods may add their own blocks and items using the values from them?

I also guess that those world capability stuff is just this thing here:  AttachCapabilityEvent<World>

 

 

12 hours ago, diesieben07 said:

This is extremely ugly and will probably leak memory all over the place unless you are very careful.

Why would this leak memory? That way it would just be saved like it is currenlty, except for that it is not only chunkPos.toString() anymore but instead world.provider.getDimension() + chunkPos.toString()

[Choonster]

59 minutes ago, Bektor said:

Also in your last answer you suggested using world capability or WorldSavedData. So what's exactly the difference between these two and which one would be a better choice to give also other mods access to the polution data saved per chunk so that these mods may add their own blocks and items using the values from them?

I also guess that those world capability stuff is just this thing here:  AttachCapabilityEvent<World>

 

World Saved Data lets you store data per-dimension or per-map. World Capabilities are just a nicer wrapper around per-dimension WorldSavedData. For a public API, I recommend using Capabilities rather than World Saved Data.

 

I helped someone with a similar system and created my own implementation of it in this thread. You can see the API here and the implementation here.

[Bektor]

On 6/10/2017 at 3:42 PM, Choonster said:

 

World Saved Data lets you store data per-dimension or per-map. World Capabilities are just a nicer wrapper around per-dimension WorldSavedData. For a public API, I recommend using Capabilities rather than World Saved Data.

 

I helped someone with a similar system and created my own implementation of it in this thread. You can see the API here and the implementation here.

Ok, thx.

Just one question, why do you send a packet to the client instead of changing the value directly in your ChunkEnergy class? I mean, you basically build your system on top of the existing one from forge and the one from forge changes the energy direclty without sending an packet which does this.

[Choonster]

8 minutes ago, Bektor said:

Just one question, why do you send a packet to the client instead of changing the value directly in your ChunkEnergy class? I mean, you basically build your system on top of the existing one from forge and the one from forge changes the energy direclty without sending an packet which does this.

 

The server and client(s) each have their own IChunkEnergy/IChunkEnergyHolder instances. The server handles any modifications to the energy amount and syncs the new value to the relevant clients so they can render it on screen (with the Chunk Energy Display item held).

 

If there was no packet, the client-side GUI wouldn't be able to display the current energy amount.

 

Forge's energy capability doesn't have any kind of automatic client-server syncing built-in.

Edited June 15, 2017 by Choonster

[Bektor]

Just now, Choonster said:

 

The server and client(s) each have their own IChunkEnergy/IChunkEnergyHolder instances. The server handles any modifications to the energy amount and syncs the new value to the relevant clients so they can render it on screen (with the Chunk Energy Display item held).

 

If there was no packet, the client-side GUI wouldn't be able to display the current energy amount.

Oh, yeah. So basically you are directly sending the new data to the client while with the forge implementation each block/item has to ask for the current value when it wants to display it, correct?

[Choonster]

1 minute ago, Bektor said:

Oh, yeah. So basically you are directly sending the new data to the client while with the forge implementation each block/item has to ask for the current value when it wants to display it, correct?

 

Any mod that displays an energy value on the client needs to sync it somehow.

 

If it's an energy bar in a GUI, this is usually handled through the Container (which syncs the value when it changes). If it's rendered outside of a GUI, it could either be always synced when the value changes or synced on demand when the value needs to be rendered.