micdoodle8 / Galacticraft

An advanced Space Dimension Mod for Minecraft
Other
617 stars 334 forks source link

Add config option for other mod world gen #827

Closed ChatFawkes closed 10 years ago

ChatFawkes commented 10 years ago

I recently updated to the latest build (154) and noticed that I can't use CoFHCore to generate ores in the moon dimension. I looked through the commits and saw: 715c760e8123ce4af641192c5a4324fed729cf9e

It would be nice if you could add in this config option within the next couple of commits because I had custom ore generation in my Galacticraft dimensions and without a config option I can't do that anymore.

radfast commented 10 years ago

ok but unfortunately it would be an "all or nothing" situation - if other mod oregen is enabled then you will also get (for example) Tinker's Construct Slime Islands, IC2 rubber trees, flowers and clouds from Natura mod, and I think fruiting trees from Pam's HarvestCraft.

This is because all these other mods make their ores, islands, flowers, trees and clouds in the same way - and Galacticraft has no way to know whether it is ores (wanted) or islands (not wanted).

ChatFawkes commented 10 years ago

Thanks for adding this. Also if someone was having an issue with the TiCon slime islands you can let them know that the config for TiCon has options to disable the slime island generation in certain worlds.

winsock commented 10 years ago

@radfast I can see a way to help that situation. You can use a security manager and block all classes except for WorldGenMinable(located in the package net.minecraft.world.gen.feature) and then exempt that class from the class check. It would sorta be like the FMLSecurityManager that is located in the package cpw.mods.fml.relauncher. If you are intrested I can make a pull request with an example that would only allow external mod ore-gen that excludes everything else.

radfast commented 10 years ago

Hey @winsock - yes please, if you have a way to do this, a pull request with an example would be much appreciated.

Right now we have made a bytecode hack to hook in to the call to GameRegistry.generateWorld(chunkX, chunkZ, world, chunkGenerator, chunkProvider) (which is vanilla/Forge code) and simply suppress that call in Galacticraft dimensions.

As you're probably aware, GameRegistry.generateWorld iterates through a list of all registered IWorldGenerators and calls each of them. Other mods typically register an IWorldGenerator in the init phase - it can be either ores or decoration. As I see it, the problems are (a) we have no way to tell which is which, and which is wanted and which is not wanted; (b) at present Galacticraft is not actually accessing the list of registered IWorldGenerators, it is just establishing a gateway which controls whether that gets called or not. With a bit of work we could solve (b) by copying and re-writing the code from GameRegistry.generateWorld() into Galacticraft with some access transforms on the fields it needs.

winsock commented 10 years ago

@radfast Hmm, I have another idea that may be simpler or may not be depending how my assumption holds up for ores. I feel like we may be going at this the wrong way. Every generator has to call World.setBlock(...) Here is WorldGenMineable for example(Line 79):

p_76484_1_.setBlock(k2, l2, i3, this.field_150519_a, mineableBlockMeta, 2);

Why not inject something like WorldUtil.isOre(Block block)? It would take one argument (the block generated) and return a boolean to return early. I'm not quite familiar with bytecode transformers and the like but basically this would be injected into World.setBlock(...) right after line number 507 (Right after the chunk variable is created so we can check if it is a new chunk)

if (!WorldUtil.isOre(p_147465_4_) && [insert some check for galactic craft world] && !chunk.isTerrainPopulated)
    return false;

Then we need to make sure that we fix the premature setting of the Chunk.isTerrainPopulated property. Remove this boolean assignment via bytecode. In Chunk.java Line 1516

this.isTerrainPopulated = true;

Insert this into ChunkServerProvider.java after line 316(Right after the call to the GameRegistry)

chunk.isTerrainPopulated = true;

Then in the WorldUtil.isOre(Block block) function add the check of OreDict and required tool. That is if I'm correct in assuming that checking if the block is registered in the ore dictionary and if it requires a pickaxe to mine then it must be an ore.

public static boolean isOre(Block block) {
    return OreDictionary.getOreIDs(new ItemStack(block).length <= 0;
}
radfast commented 10 years ago

Hey @winsock - I'm really grateful that you're thinking about how we can do that. But making bytecode changes to the vanilla code in World.setBlock() or anything in Chunk is difficult (it would need a lot of development time to do it correctly) and I am concerned it could affect the performance of the game - the setBlock method is called all the time in Minecraft and its mods, every time any block anywhere changes, and not only during worldgen / decoration, and the call to OreDictionary.getOreIDs is likely to be slow.

winsock commented 10 years ago

@radfast I feel like the performance hit will be non existent if we reorder my proposed check to put the ore check last. Then it would have no effect on already populated chunks and no effect on non galactic craft chunks. When I get home from work I'll try to create and profile the effects of such addition. Otherwise we would have to edit the bytecode of FML's security manager because for some reason FML doesn't want to allow other security managers. Technically we could get around that with threads but that starts to get messy with Minecraft not being very thread safe. But I do have an idea that could work with threading that I'll implement and compare with the setBlock edit.

radfast commented 10 years ago

@winsock, we achieve bytecode editing through MicdoodleCore's transformer, which changes the bytecode after FML has loaded the classes (so we don't have to worry about security manager etc). However as you can see if you look at that code, it requires a fair amount of careful coding for each transformation. Each transformation also increases the risk of incompatibility with other mods (especially coremods), Cauldron, or Nallar's TickThreading. Therefore a transformation is not something we do lightly.

I would advise against anything multithreaded in Minecraft, it has caused more problems than it solves every time we have tried it. Of course, that may be just our multithreading skillz ... :)

I think the direction I was headed with this before your comments was to re-write GameRegistry.generateWorld but with an analysis of which IWorldGenerator is which (using reflection to identify them by name) and have either a blacklist or a whitelist, which would have to be based on the mods we know about obviously.

Both approaches are a huge amount of work for what I would call a fairly niche + luxury issue - that is the desire in some modpacks to have other mods generate their ores on the Moon (and Mars etc). It isn't gamebreaking not to have this as players can always amass ores on the Overworld, and we intend that some ores (anything organic, like coal or crude oil) can't be found on other planets. For rare modpacks where it could be gamebreaking - e.g. some where players start on the Moon with no access to the Overworld - then there's this config option to enable worldgen, and as long as the modpack doesn't include Natura or some of the other nature/plant type mods, things should mostly be OK.

A third approach which might be way simpler would be an option to add other mods' ores to Galacticraft's own worldgen, this could probably fairly easily be done by players through the config using just OreDictionary names. I imagine in reality there's only a small handful of ores which need it (maybe Lead Ore, Ferrous Ore from Thermal Expansion, Silver Ore from IC2, Osmium Ore from Mekanism?) That doesn't seem too hard to implement.

So I don't want to discourage you from anything you were planning as it sounds like you really know your stuff with coding - but if you're feeling the itch to help out in Galacticraft there's also masses of other issues which could use some attention :) Maybe contact me or micdoodle8 to chat about this some time? (You can reach me on Skype at radfast2.)

radfast commented 10 years ago

Here's a good reason why we have to go for the "third approach" - some of the other mods oregens will only work while they can replace plain stone with their ores, they won't know that they can replace Moon rock or Mars stone with their ores. http://forum.micdoodle8.com/index.php?threads/1-6-4-metallurgy-oregen-in-gc-dimensions.4459/