Feature: Custom Ore Gen for 1.11 working on my fork

[ShadowBoxx-LLC]

Hello ladies and gents,

As the title suggests, I have a working version of Custom Ore Gen for 1.11 on my fork I created a short while ago. I placed it in it's own branch, as that appears to be the convention you are following as of 1.7. If you would like to create a 1.11 branch on your side, I would gladly make a pull request.

There wasn't terribly too much too getting it to work, and from my testing in game, it appears things are working correctly. Although my testing hasn't been extremely thorough.

The only part that worries me is src/main/java/CustomOreGen/Util/BiomeDescriptor.java. Since BiomeDictionary.Type is no longer cast as an enum we can no longer use valueOf() in order to pass the Type back. So I came up with some clever hackery to work around it. As I stated, it all works, but I'm certain this solution is not perfect, and I almost think instead of testing for BiomeDictionary.hasType(biome, type) further down after the loop, if perhaps it would be better to simply place the totalWeight += desc.weight; statement directly in the loop, since the loop is effectively achieving the same goal as BiomeDictionary.hasType(biome, type) at this point.

Anyway, you can see the full extent of the changes here: master...Palejack:1.11

I will check back for a response tomorrow!

[lawremi]

Thanks that will be a useful reference for when we make the move. Feel free to make a PR against trunk. I'll branch off 1.10 before merging, although the patch is not exactly clean.

[ShadowBoxx-LLC]

Hey Lawremi!

That was quick!

Yes, I apologize for that. While I'm actually pretty good with Java (I worked for Research In Motion for several years on their Java based app servers for the Blackberry) I find myself fumbling with gradle quite a lot...

I actually wanted to write back and say I have managed to crash the game with this port to 1.11 by trying to re-create a world. I clicked the re-create button and then when I opted to change the custom ore gen settings I got a crash with a java.lang.NullPointerException: Updating screen events error.

The reason I decided to edit the world in the first place was because I seem to have managed to build a world in which there was either no ore on the surface, or biomes weren't getting selected. So it looks like I might have doofed something up after my initial tests, in my effort to tidy up the actual code.

I'll try to debug and figure out what is going on with both of those cases. Do you have any handy advice for cleaning things up before I send the PR? I would like to avoid causing you as much work as possible, and I hate the idea of other people having to tidy up my messes.

Oh, one more thing, and I am not sure if you were aware of it, but in src/main/resources/config/modules/Vanilla.xml, OptionChoice vanillaOreGen was being defined again, which was causing the mod to throw errors and the config menu to render nothing. I was able to reproduce that error in both 1.10.2 as well as 1.11 though. Not that issues like that aren't to be expected when you are pulling directly from source control and building the mod, just thought you should know. It's currently commented out in my fork, as I wasn't really sure what the best way to handle it was.

[ShadowBoxx-LLC]

Happy to report that I have the biome selection weight issue taken care of and I think I have the crash resolved when attempting to re-create a world as well in 1.11.

I took it to the extreme and cranked the frequency of everything up to 5. It was pretty interesting seeing mountains essentially made out of clay, and coal riddled stone. Haha!

I'm going to try to tidy things up a bit on my end and then I'll send the PR to trunk. In the meantime, if you have any tips on making it cleaner, please don't hesitate to let me know.

Thanks!

[lawremi]

Made some comments on your commits.

[ShadowBoxx-LLC]

Perhaps I am being daft or looking in the wrong spot, but I do not see your comments on either of my commits I made? :)

ShadowBoxx-LLC@f138f6e
ShadowBoxx-LLC@564805e

[lawremi]

They should show up if you click on the commits through this comparison: master...Palejack:1.11.

[SirChamomile]

The recreate world bug + crash exists unaddressed I'm the 1.10.2 version as well.

[lawremi]

Please file a separate issue.

[SirChamomile]

I will do that. I just wanted to mention in case it was relevant to fixing his bug, didn't see the follow up.

[ShadowBoxx-LLC]

@lawremi Thanks for your feedback. You pointed out a few changes that I made, which were supposed to be backed out again, but ended up making it in anyway. Odds are it happened during a merge between two branches on my local machine. I'll get those fixed.

I responded asking for additional feedback on the other comments you made, if you would be so kind. :)

[leagris]

@PaleJack I cloned your fork, but the build.gradle is about forge 1.10.2
Is there a place I can download your source for 1.11.2 or a pre-build binary of COG for 1.11.2?

Nevermind, I found out :)
git clone -b 1.11 https://github.com/Palejack/CustomOreGen.git

[Draco18s]

Updating from 1.11 to 1.12 was very easy. A few interfaces moved around (the IChunkGenerator hierarchy, just remove the imports and reimport them), the ChunkProviderXxx became ChunkGeneratorXxxx, .hasSky() became something else (I used .isSkyColored() which is only false for the End, not the Nether, but you can't ever see the sky in the nether anyway, soooo...close enough?), and the VertexBuffer became BufferBuilder. Eclipse was able to auto-correct most other changes (xPosition -> x, etc).

https://github.com/Draco18s/CustomOreGen/tree/1.12

[lawremi]

Thanks. A pull request would be appreciated. I think hasSky() and isSkyColored() are pretty different. I think the intent of hasSky() was to alter generation in underground/cave-like worlds.

[Draco18s]

I agree, but I can't find a method that does what that method did. There's hasSkyLight but that appears to be set to true for the three vanilla dimensions.

[Draco18s]

Turns out isSkyColored() (and biome.getBiomeName()) are client side only. So that won't work. The only other option is hasSkyLight(), nothing else that even comes close to the "same."

Biome.getBiomeName() is a bit harder. I move to using getRegistryName() but that would require updating all of the config files (there are differences between the name and the registry name, e.g. "beach" vs. "beaches"). We could drop the domain part entirely (that's easy, just append .getResourcePath() to it)...or we use Reflection.

[lawremi]

I think we're just going to have to drop support for hasSky(). There is still isSurfaceWorld() which is maybe more appropriate anyway for ore generation.

Is it the case that everyone refers to biomes by their registry name now, e.g., in debug mode, minimaps, etc? If so, we should probably make the switch, despite the pain of updating the configs.

[Draco18s]

Minimaps can make use of client-side-only information. Serverside, yeah, almost everything is handled via registry names now. I'm even removing vanilla recipes by registry name (because json support) instead of by output item.

[lawremi]

Got it, didn't realize the biome name was restricted to the client. Guess we will have to move to the registry name.

[lawremi]

Is the plan to merge this via PR?

[Draco18s]

@lawremi Thanks for the poke.
I wanted to go through the config xml files and update the biome names to use registry names first. Its on my list of things to do today.