[1.13.2] Modifying vanilla ore generation 1.13+?

[treebranch]

I'm trying to handle when a vanilla ore is generated so I can cancel it under certain conditions.

So far I've tried looking into OreGenEvent.GenerateMinable and MinecraftForge.ORE_GEN_BUS, but neither of those appear to exist in 1.13.

I tried searching these forums, but nobody seems to have asked about this yet. There isn't any up-to-date online documentation either.

I tried skimming through events manually and guessing by their names, but I couldn't find anything.

Does anybody know the new way to handle when an ore block is generated?

 

[treebranch]

I came across this thread:

 

Here's the code I ended up coming up with as a test, replacing all coal ore with diamond ore every time a chunk loads:

 

@SubscribeEvent
public void oreGenMinable(ChunkEvent.Load event) {
	IChunk chunk = event.getChunk();
	LOGGER.info("SCANNING CHUNK at " + chunk.getPos().toString());
	IBlockState coaldefault = Blocks.COAL_ORE.getDefaultState();
	IBlockState diamonddefault = Blocks.DIAMOND_ORE.getDefaultState();
	for (int x = 0; x < 16; x++) {
		for (int z = 0; z < 16; z++) {
			for (int y = 2; y <= 256; y++) {
				BlockPos pos = new BlockPos(x, y, z);
				if (chunk.getBlockState(pos) == coaldefault) {
					chunk.setBlockState(pos, diamonddefault, false);
				}
			}
		}
	}
}

 

I noticed the chunk class no longer has .markDirty(), I'm wondering if this is still necessary. I also still need a way to avoid overwriting the same chunk twice.

Edited April 17, 2019 by treebranch

[Cadiboo]

11 minutes ago, treebranch said:

LOGGER.info(

Use debug or nothing pls (log spam & logging stuff slows down loading times)

 

11 minutes ago, treebranch said:

BlockPos pos = new BlockPos(x, y, z);

Please use a MutableBlockPos or a PooledMutableBlockPos.

protip: PooledMutableBlockPos implements AutoClosable so it can be used in try-with-resources (AKA automatic resource management) blocks

Example:

try (PooledMutableBlockPos pos = PooledMutableBlockPos.retain()) {

//loop

pos.setPos(x, y, z);

//do stuff with pos

}

Edited April 16, 2019 by Cadiboo
Slam -> spam

[treebranch]

3 minutes ago, Cadiboo said:

snip

Thanks for the tips. Could you please further explain what PooledMutableBlockPos is? What does .retain() do?

Edited April 16, 2019 by treebranch

[V0idWa1k3r]

This code isn't doing what you want it to do and is horribly inefficient. First of all Chunkevent.Load fires every time the chunk is loaded, not just the first time it generates which will run this snippet of pretty expensive code every time ANY chunk is loaded. Not to say it will also replace the coal ore(in this case) regardless of how it ended up in the chunk - whether it was generated or placed by the player your code cares not.

 

Look at how vanilla's features now work, I believe the generators are registered per biome to a list and you should be able to just search that list on startup and remove the generators you don't want present.

[treebranch]

7 minutes ago, V0idWa1k3r said:

This code isn't doing what you want it to do and is horribly inefficient. First of all Chunkevent.Load fires every time the chunk is loaded, not just the first time it generates which will run this snippet of pretty expensive code every time ANY chunk is loaded. Not to say it will also replace the coal ore(in this case) regardless of how it ended up in the chunk - whether it was generated or placed by the player your code cares not.

 

Look at how vanilla's features now work, I believe the generators are registered per biome to a list and you should be able to just search that list on startup and remove the generators you don't want present.

 

It may be inefficient but I'm blindly trusting what @Draco18s said on the other thread:

 

On 12/15/2017 at 10:00 AM, Draco18s said:

For reference, you can scan an entire chunk and replace blocks in about 400,000 nanos (0.4 ms). You don't really need to worry about time slicing. 

 

Removing generators won't do me well because I'm going to need certain blocks to generate differently depending on how far away from world center the chunk is.

And don't worry, I know it's fired everytime a chunk loads. I'm still looking for a good way to keep track of what chunks have been modified and which ones haven't.

 

However I appreciate your help, and please continue suggesting things if you know alternative ways to do what I want, because there isn't much info out there.

Edited April 16, 2019 by treebranch

[Cadiboo]

A PooledMutableBlockPos is a BlockPos that is Mutable (can be changed/moved) and Pooled. Retain gets a PooledMutableBlockPos from the pool for you and close (called automatically by the try-with-resources block in the same way as in a try-finally block) releases the PooledMutableBlockPos you got from retain back into the pool.

 

So instead of creating 16x16x255 (65,280) new BlockPos objects you only create maximum 1 new object (the PooledMutableBlockPos is likely to have already been created and added to the pool).

Doing this avoids the cost of initialising all those objects (this cost is pretty small, but still deserves mentioning), avoids using up 786,432 bytes of ram (each block pos contains 3 integers and each integer is 4 bytes of ram) for each chunk and it avoids the cost of destroying (garbage collecting) all those objects.

 

This may not seem large, but remember that all this is being run for each chunk, and that any reduction in load time counts in big modpacks. 

Edited April 16, 2019 by Cadiboo

[treebranch]

Just now, Cadiboo said:

snip

Thank you very much for the clear explanation.

[treebranch]

I think IChunk is immutable, and that's why .markDirty() doesn't exist. Older versions of Forge had this method simply return a mutable Chunk. I can't figure out how to get my hands on a Chunk instead of an IChunk, and searching for IChunk on this forum brings this thread up as the only result. Does anybody here have more info?

 

Edit: So far I've had no visible problems casting my IChunk to a Chunk and calling the method anyway. Not sure if this is a bad idea or not.

Edited April 16, 2019 by treebranch

[treebranch]

Okay so I ended up creating a table called IgnoreChunks to keep track of what chunks have been modified and therefore should be ignored. The chunk's position is added to IgnoreChunks after it's been modified, and the position is unloaded when the chunk is unloaded. The chunk's ignore status is saved via NBT on OnChunkData.Save, and that NBT data is checked on OnChunkData.Load to populate the IgnoreChunks table again.

 

It works as expected, and there are no memory leaks, but there's a problem. The ignore state is lost when the game is entirely restarted (closed then re-opened) which causes chunks to get overwritten again. The cause seems to be that ChunkDataEvent.Load is never called (!!!) and I can't tell whether this is a Forge bug or my own mistake.

 

Here's the code:

 

@SubscribeEvent
public void onChunkLoadData(ChunkDataEvent.Load event) {
	NBTTagCompound chunkdata = event.getData();
	
	// If this chunk is marked as ignored in NBT, load its position to the ignore list so that onChunkLoad knows not to modify it
	if (chunkdata.getBoolean(ModInfo.MODID + "_ignore")) {
		IgnoreChunks.put(event.getChunk().getPos().asLong(), true);
	}
}

@SubscribeEvent
public void onChunkSaveData(ChunkDataEvent.Save event) {
	NBTTagCompound chunkdata = event.getData();
	
	// If this chunk's position is loaded in the ignore list, mark it as ignored in NBT before it's saved
	if (IgnoreChunks.containsKey(event.getChunk().getPos().asLong())) {
		chunkdata.setBoolean(ModInfo.MODID + "_ignore", true);
	}
}

@SubscribeEvent
public void onChunkLoad(ChunkEvent.Load event) {
	Chunk chunk = (Chunk) event.getChunk();
	
	// If this chunk's position is loaded in the ignore list, don't modify it (return immediately)
	if (IgnoreChunks.containsKey(chunk.getPos().asLong())) {
		return;
	}
		
	IBlockState coaldefault = Blocks.COAL_ORE.getDefaultState();
	IBlockState diamonddefault = Blocks.DIAMOND_ORE.getDefaultState();
	try (PooledMutableBlockPos pos = PooledMutableBlockPos.retain()) {
		for (int x = 0; x < 16; x++) {
			for (int z = 0; z < 16; z++) {
				for (int y = 2; y <= 256; y++) {
					pos.setPos(x, y, z);
					if (chunk.getBlockState(pos) == coaldefault) {
						chunk.setBlockState(pos, diamonddefault, false);
					}
				}
			}
		}
	}
	
	// Load the chunk's position to the ignore list
	IgnoreChunks.put(chunk.getPos().asLong(), true);
  
	chunk.markDirty();
}

@SubscribeEvent
public void onChunkUnload(ChunkEvent.Unload event) {
	// Unload this chunk's position from the ignore list if it's loaded
	Long pos = event.getChunk().getPos().asLong();
	if (IgnoreChunks.containsKey(pos)) {
		IgnoreChunks.remove(pos);
	}
}

 

Edited April 16, 2019 by treebranch

[treebranch]

So after a good night's sleep I've rethought my approach and come up with a solution that doesn't involve any data saving. It simply marks chunks as new on ChunkGeneratorEvent.ReplaceBiomeBlocks—this event could be anything, I'm just using it because it's called during the generation of new chunks—then the chunks are replaced later in on ChunkEvent.Load.

 

It works, almost too well. However it's causing a memory leak. Even though I remove new chunks from my NewChunks list after they've been replaced, it seems that ReplaceBiomeBlocks is being called more often than ChunkEvent.Load is, and the list steadily grows faster than it shrinks.

 

Here's the code:

 

@SubscribeEvent
public void onReplaceBiomeBlocks(ChunkGeneratorEvent.ReplaceBiomeBlocks event) {
	// This event is called whenever a new chunk is being generated, so we can prepare the chunk for replacement
	NewChunks.put(event.getChunk().getPos().asLong(), true);
}

@SubscribeEvent
public void onChunkLoad(ChunkEvent.Load event) {
	Chunk chunk = (Chunk) event.getChunk();
	
	// If this chunk isn't marked as a new chunk, don't modify it (return immediately)
	if (!NewChunks.containsKey(chunk.getPos().asLong())) {
		return;
	}
		
	IBlockState coaldefault = Blocks.COAL_ORE.getDefaultState();
	IBlockState diamonddefault = Blocks.DIAMOND_ORE.getDefaultState();
	try (PooledMutableBlockPos pos = PooledMutableBlockPos.retain()) {
		for (int x = 0; x < 16; x++) {
			for (int z = 0; z < 16; z++) {
				for (int y = 2; y <= 256; y++) {
					pos.setPos(x, y, z);
					if (chunk.getBlockState(pos) == coaldefault) {
						chunk.setBlockState(pos, diamonddefault, false);
					}
				}
			}
		}
	}
	
	// Remove this chunk from the list of new chunks to free up memory and prevent it from being replaced again 
	NewChunks.remove(chunk.getPos().asLong());
	
	chunk.markDirty();
}

 

Edited April 17, 2019 by treebranch

[Cadiboo]

Why not just use a HashSet? Also there’s probably a better event for what you want (like the chunk decorate event)

[treebranch]

8 minutes ago, Cadiboo said:

Why not just use a HashSet? Also there’s probably a better event for what you want (like the chunk decorate event)

Wasn't aware of HashSet, it looks like it just uses a HashTable internally so I'll switch anyway since it makes more sense for my code.

 

Unfortunately there's no chunk decorate event in 1.13. I've looked at all the past code examples I could, and it seems like almost every event used for purposes like this has been removed. ChunkGeneratorEvent.ReplaceBiomeBlocks is the closest I could find.

Edited April 17, 2019 by treebranch

[treebranch]

Well, I'm just about at the end of my wits trying to figure this out. I went as far as to turn the HashSet into a ConcurrentHashMap and iterate through it every ServerTickEvent to deal with new chunks. Theoretically this should always modify chunks the tick after they're created. But ChunkGeneratorEvent.ReplaceBiomeBlocks is called on so many not-loaded chunks that this method slowed the game down to a crawl. Distributing it over several ticks is too slow and introduces the problem of people closing the game before every chunk has been replaced. Does anybody here know of a simpler way for me to just edit a chunk after its been created?

[treebranch]

Okay, I need to move on with my mod. I've went back to the original method of indiscriminately overwriting chunks every time they're loaded. Mostly this will involve replacing rare ores with stone unless the chunk is a certain distance away from spawn, or unless the block's Y level is deep enough. My solution to the problem of player-placed ores getting overwritten looks something like this:

 

@SubscribeEvent
public void onBlockHarvestDrops(BlockEvent.HarvestDropsEvent event) {
	if (event.isSilkTouching() && event.getState() == Blocks.COAL_ORE.getDefaultState()) {
		event.getDrops().clear();
		event.getDrops().add(new ItemStack(Items.COAL));
	}
}

 

If anybody has a problem with this, please let me know the way you'd do it instead, and I'll happily switch. Until then...I guess I'll just warn people on the mod page that they shouldn't use this mod with creative mode.