DimensionalDevelopment / JustEnoughIDs

Use the 1.13 chunk format in 1.12 to remove the block, item, and biome ID limits
MIT License
32 stars 33 forks source link

Weird incompatibility with world generation #42

Open xAlicatt opened 6 years ago

xAlicatt commented 6 years ago

I have tested this with minimal mods and it's for sure an incompatibility with JEIDs and Chunk Pregenerator mod pregeneration. This is using the preview button method of pregeneration (on the world creation screen). I have not tested with the in-game command pregen. (edited to reflect updated understanding of the issue)

Basically the biomes are generating weirdly. The biome features all exist as normal, but the actual biome is almost all plains. There is a random chunk here and there that will have the proper biome, but over 90% of the map will generate with the biome ID being Plains. I've included a picture to better explain what i'm talking about, and I've seen you ask for logs, so those are here

Imgur

ZombieHDGaming commented 6 years ago

There's not really much I can do since ChunkPregenerator is closed source (I don't see what the point is of it being closed source). If you bring it to Speiger's attention, I'd be willing to work with him to make it compatible, but like I said, there's not much I can go off of based on a mod that I would have to decompile, which would be worst case scenario. Bring it to his attention and we'll go from there.

SalmonSays commented 6 years ago

I've noticed this same issue and didn't realize it was tied to JEIDs- my test environments have all included a beta terrain-height/noise modifier so I wrongly assumed the blame was there.

The same bug happens with Forge's built in pregeneration ("/forge generate") so I'm hopeful you won't need to see Chunk Pregenerator's source to fix this- the bug is identical between the two, so fixing it for Forge will probably fix it for ChunkPregenerator.

Quick Edit: To build off of xAlicatt, I have tested using the in-game commands of Forge and ChunkPregenerator to the same effect. The key thing about this issue is that it only happens when pre-generating; Exploring the terrain naturally on a client or server/client results in terrain that behaves exactly as it's supposed to.

xAlicatt commented 6 years ago

I'm wondering if it's the default world generator that causes it? A fellow modpack maker has tested pregen with BOP's generator and says this does not happen.

EDIT:: The friend was using NEIDs, not JEIDs

SalmonSays commented 6 years ago

It's not exclusively default world generation, in that a private build of RTG behaves the same way. I'm interested to see that you got unusual chunk borders in your sample photos in addition to the plains biome, because I ran into those too:

g3rurgw

It means not only are biome distributions being impacted, but chunks are being built wrong in seemingly random places in ways that don't match a corresponding naturally generated world-seed.

Speiger commented 6 years ago

Well ChunkPregen calls basically the same methodes as minecraft does, just with its own wrappers. But the functions are basically 99% identical. I basically do the same thing as "ChunkProviderServer.provideChunk" does @xAlicatt @ZombieHDGaming And thats my only connection to worldgen, everything else is basically done by minecraft itself.

I can provide more detail info on this if you have more questions.

On a Educated guess: Does a mod in this pack change the rule how chunks have to be populated/generated? Because this looks like something RTG did, that mod is basically incompatible to any Pregenerator, because it changes the population from 2x2 to 3x3 which makes me wonder if its even compatible to mc.

xAlicatt commented 6 years ago

@Speiger I have tested this with VERY minimal mods. Just Enough IDs, Chunk Pregenerator, Thaumcraft (to check for modded biome support), Hwyla, and JEI. I'm using vanilla generation. With JEIDs, I get this weird biome ID situation using pregen. Without JEIDs, it works as expected. With JEIDs, loading chunks as normal (without pregen) works as expected.

It's a conflict between pregeneration and JEIDs.

Edit: Basically, i don't think it has anything to do with your mod specifically, but sharing your code with @ZombieHDGaming may help him fix it.

Edit Again: So I just tested with BOP and it's still doing this. It's something to do with pregeneration versus normal generation.

Andyghandi commented 6 years ago

Can't believe i found the root for this problem. It drove me crazy! Hoping for a solution soon

Andyghandi commented 6 years ago

I can confirm that the issue remains even with quark's realistic gen

Speiger commented 6 years ago

@Andyghandi what was it? @xAlicatt i already did tell what code i use for it, basically the same as forges just with a customized wrapper that allows me to control crashes better so that the tool can restart quicker.

Andyghandi commented 6 years ago

@Speiger Sorry, I didn't mean like I found the bug, more like I found the issue for my world generating in all plains. In Journeymap, I can see the biomes name, even though the chunks are discovered, it's like, an 8 chunk radius out from the player makes a plains biome, and outside this 8 chunk radius, It displays the proper biome names on journeymap's full-screen map

Speiger commented 6 years ago

@ZombieHDGaming @xAlicatt the issue is in this class: https://github.com/DimensionalDevelopment/JustEnoughIDs/blob/master/src/main/java/org/dimdev/jeid/mixin/core/MixinChunk.java

You change the function getBiome to only have integers, using your intArray, thats fine (ignoring all the incompat you get by everything that uses the getBiomeArray byteversion) but your "getBiome" function does not account for if a biome is "uninitialized" which minecraft handles and also tries to regenerate that area in the biome generator if needed. I think thats the bug where the biomes are not generated properly and that could be an issue.

I bet that pregenerated worlds are not "PlainsOnly" but only the client side of things receives that kind of data.

Its a wonder that my tool didnt crash ^^"

Speiger commented 6 years ago

anything happend or no time or ignoring it until it disapears?

SalmonSays commented 6 years ago

@ZombieHDGaming This should probably be switched from the "incompatibility" label to the "bug" one since it happens with the built-in Forge chunk pregen system

Nightreaver commented 6 years ago

Hello

I've been hitting the same issue recently and I can confirm that it seems to be connected with both mods. In the last few days i've been regenerating my world plenty time... sometimes it took over 10h for the whole world... just to figure out that its still 90% plains and missing many biomes.

My first guess was, that is actually just a "preview" issue, so the biomes are actually there just the "tooltip" is reading the wrong biome, but as it has "broken chunks" the seem to be weird.. I wasnt sure.

I cannot config that weird "chuck border" issue mentioned here as well, as this only seem to happen when you generate, change generator settings+seed and continue to generate futher. Because for me everything seems fine and I can get the same "layout" over and over again as long as I keep the biome generator settings.

Please fix this soon!

Nightreaver commented 6 years ago

Hello there... me again o/

So I wasnt sure if its really "just" a display issue or a general "generation" issue. And for me it looks like there is something else happening as well.

I've been pregenerating a world 8000x8000 blocks and I can tell that it just looks wrong.

map1

Beside that fact that around half the screen is water, the landmass looks like its 80% Plains, I mean... you can barely see any trees/forests it looks like landmark features are missing as well.

I also went one step further and checked a smaller area in more detail.

Taking this part of my world from pregen pregen

It reflects almost everything what's wrong with landmass, way less trees/features than expected. I deleted the region files and just went there exploring "manually"... and look whats happening...

nopregen

It looks sooo much different... I cant believe its just an issue from JEID ? @Speiger

Between these to pictures I didnt change anything regarding mods/settings/seed etc. Just pregen world, visit an area, take a look... delete region files and go there again.

I dont think its hard to replicate.

PS: Another diagonal "flythrough"

image

Speiger commented 6 years ago

@Nightreaver you made sure you are not "Only Generating Terrain" which is the default option the seed previewer? Because seed previeing on a biome based system is a lot faster if you only have to do the terrain. If you click on the terrain button it switches to terrain & post then it willl generate everything.

Nightreaver commented 6 years ago

This is not seed preview, it is already fully pregenerated terrain and it took around 10h for the whole map. So if I run /pregen gen startWorldBorder I assue it will pregen everything?

Anyways, if I go to these places ingame, nothing is changing. If pregen would only generate terrain, then - I assume - once I go there all features will be generated afterwards? Because that's not the case either.

First 3 images are "dynmap" rendering after generation was completed. 4th is Ingame from Journeymap, the big "lifeless" areas are from pregen, then there is diagonal (bottom left -> top right) path I took after deleting the World, which is then properly populated.

Sidenote: Biome description/Name is correct after that (regular minecraft generation), no more "Plains" everywhere but proper biome names.

Speiger commented 6 years ago

well the start worldborder should generate everything unless you tell it not to do it so. On the other hand have you tried the forge version of it too. To ensure that this is only a chunkpregen issue.

Throwing that out of the window. Is the same issue happening without JEID. Because if thats the case then i am happy to investigate it further since its a issue where i dont handle things properly in vanilla. (But over 4-8 iteration of how to do worldgen properly each time learning something new so i doubt this is another case of that). If that is not happening in vanilla (i mean without JEID) then this is basically JEID not being compatible with vanilla completely and you found basically an edgecase for them. Assuming the vanilla test runs perfectly fine with Dynmap & JourneyMap.

Thats the requirements i set for accepting it as my bug @Nightreaver . Test it with forge, and test it without JEID if the same issues show up then i accept that chunkpregen is the cause. If both show its not only happening with chunkpregen but also other pregenerators that shows me that JEID is still needing more improvements

xAlicatt commented 6 years ago

@Speiger if you read the other posts in this thread, you'll see that we have already tested the pregen WITHOUT JEID and are not running into this biome issue. In my tests, pregen works as expected without JEID installed. But with it installed, I'm getting almost all chunks being marked as Plains, with the exception of a few (seemingly) random ones here and there with their appropriate biome marker.

They all seem to generate with the appropriate biome features (in my tests anyway) they are just marked as the wrong biome. Now I didn't personally do any in depth testing to check if they are receiving the Plains biome weather/climate/etc or if they were getting all of their trees and whatnot as stated in @Nightreaver 's testing, but my guess is that they will act like biomes that were generated normally and then had their biome changed via worldedit or some other means to Plains as far as their climate/weather goes.

EDIT:: I did want to add that I didn't do a great deal of testing myself. Just some preliminary testing to figure out what was causing the issue. I knew I wanted to pregenerate my map and that anything conflicting with that would have to go...so out JEIDs went. It should be noted that I did not experience this issue with NEIDs, but I ended up not needing an ID extender in my pack, so there was no further reason for me to test after I took JEIDs out.

EDIT AGAIN:: I do not think the problem is with pregen itself or with the Chunk Pregen mod. I think the issue is JEIDs and some incompatibility with pregeneration.

ZombieHDGaming commented 6 years ago

I'm locking the conversation for now until I have time to fix this. I know there is an issue and I plan to investigate, but with the end of my college semester coming up, I won't have much time to look into the issue until my finals and work are done in a few weeks.

BugmanBugman commented 5 years ago

Any follow up on this?

Speiger commented 5 years ago

I doubt they gonna fix it in 1.12. Because 1.14 is out and the porting will be starting as soon forge fixes 1 event that is missing.

sam-kirby commented 5 years ago

@Speiger This appears to be an issue exclusively with chunk-pregenerator when used in combination with JEID. I have stated this previously in my notes on JEIDsI's curseforge page, which you ignored for some reason.

I don't know what testing @SalmonSays did, but the results stated are not supported by the evidence I have gathered. Forge could have changed how the generate command behaves, though given that ChunkGenWorker has not changed functionally since April 2018 that seems unlikely.

Method

All worlds below use the seed -107391545297039861. Mapping performed by JourneyMap.

40000 chunks generated using either 100 radius (square) or Forge 40000 chunk spiral.

Where chunk generator was used, terrain and post were generated at the same time. I did test separating them, and it made no difference.

Versions

No other mods present

Mod Version
Forge 14.23.5.2838
JEID 1.0.2-26
Chunk Pregenerator 2.1
Biomes O' Plenty 7.0.1
JourneyMap 5.5.4
JEI 4.15.0.268

Default world

chunk-pregenerator invoked by command

0-cpgencom-default-40000

The area generated by minecraft at world creation around spawn is correct. An area extending a short way beyond this also seems to have generated correctly. The rest of the map does not generate correctly, biomes are all plains, decoration is minimal. Height map seems accurate. No issues at border (not shown). Correct block palette in most places.

chunk-pregenerator invoked using preview GUI

0-cpgen-default-40000

As above, but without spawn area.

Forge /forge generate command

0-fgen-default-40000-compressed

Biomes are correctly assigned, chunks are decorated. No obvious issues

Flying + Forge

Comparison image: File too large 0-flygen-default-40000-compressed More than 60% of the area was generated by flying randomly over it. Forge generate command was used to fill in the gaps. There is no distinction between terrain generated by either method. The same is obviously not true of chunk pregenerator, as can be seen in the invoked by command image.

BOP World

Forge generate

1-fgen-bop-40000

Chunk Pregenerator

1-cpgen-bop-40000

Again terrain blocks are placed in the correct places, but biomes are not assigned. Only one chunk had a biome other than plains - right at the spawn was a taiga chunk (would be expected).

sam-kirby commented 5 years ago

https://github.com/DimensionalDevelopment/JustEnoughIDs/issues/42#issuecomment-430989366

This comment correctly identifies part of the issue. Adding the line below to MixinChunk#getBiome results in Chunk Pregenerator performing as intended.

        if (biomeID == 0xFFFFFFFF) {
            biomeID = Biome.getIdForBiome(provider.getBiome(pos, Biomes.PLAINS));
            intBiomeArray[index] = biomeID;
        }

This restores functionality present in vanilla; specifically if getBiome is called before the biome array is initialised the provider is queried to find the expected biome.

However, this issue is not present in vanilla generation, or in Forge's generator because this situation never arises. Both use IChunkGenerator#generateChunk, the implementation of which for the overworld sets the biome array immediately after instantiating the chunk. As a result, it seems this condition is never reached in vanilla.

BOP world with chunk pregen and modified JEID

1-cpgen-bop-40000-chachu2

sam-kirby commented 5 years ago

The above fix produces the correct map in the preview GUI - hovering over any point at random produced the correct biome assignment.

However, upon spawning into the world, the biome is not what is expected, in the BOP world I expect to spawn in Taiga, but instead find myself in coniferous forest. This biome is decorated as Taiga, with small spruce trees and vanilla dirt instead of BOP's loamy dirt and fir trees as would be expected in coniferous forests.

Moving in this world causes a crash. crash-2019-08-12_12.13.03-client.txt

2019-08-12_12 17 59_cpgen-bop-40000-oldupdatedjeid_overworld_day

In the image above, you can see the block colours changing in areas that should be one continuous biome. This world has less than 255 biomes, so this is not a bitmasking or truncation issue - even if it were, you'd expect the biomes to be wrong consistently.

Results when using forge generate are unchanged, and there is no crash. I have validated the world generated by forge against a world generated by chunk-pregenerator without JEID present.

0-cpgen-default-40000-chachu-compressed Results are similar using vanilla biomes. Particularly obvious in the jungle, which has generated with patchy mesa biome overlaying. Again, Forge does not show any sign of issues.

What does chunk pregenerator do differently to forge?

Speiger commented 5 years ago

The only Major difference is that chunkPregen never calls onChunkLoad because it only adds entities/tileEntities to the world. Outside of that there is no difference. The preview actually starts only a custom Integrated server instance that allows itself to run without the player outside of that the preview gui does nothing different. But that is a giant hack on itself because mc isn't designed to be really run like that. And there is no way to fix that issue in that way so preview may be not a good option in some ways. But the corruption is weird to me too.

Speiger commented 5 years ago

@sk2048

sam-kirby commented 5 years ago

Looking back at what I did last night I must have been very tired - I used the provider to return the biome if biomeId at that position was uninitialised. However, I was not saving that result to the biome array.

This resolves the issues mentioned above and I have updated comments accordingly. So in conclusion a relatively small change to JEID should be made such that it better imitates vanilla behaviour. You may consider it worth your time Speiger to investigate why there is a deviation in your chunk generator vs vanilla/forge generators, but then again it works as it is - and very well I must say - so why fix something that somebody else broke? :)

Speiger commented 5 years ago

Well I already invested quite a lot of time into this exact issue @sk2048 But assuming since I have a copy of Vanillas MC save handler (which could be the cause of the issue) that could be an issue with the mod. The only downside is i actually can not access that function.

Outside of that it could be that the chunkloadevent might fix the issue with JEID but the issue is you lose a good chunk of Generation speed if you call a event that is may or may not used. (Yes even in a +forge+pregen that had a concidered performance increase)

Other then that its just managment of chunkloading, (aka not using vanillas ChunkProviderServer but using direct methodes) ensuring that forceloaded chunks stay loaded, memory tracking so that you do not run out of ram so quickly and cause worldcorrution, and a couple other things.

And the question here is because you were saying it:

small change to JEID should be made such that it better imitates vanilla behaviour

That could mean that JEID is not compatible to vanilla itself. If you change the rules of vanilla you have to expect issues coming up. Where ChunkPregen falls into that case is with the preview where crashes can happen because i am breaking the rules of minecraft myself there: There is either a dedicated server or a Integrated server with a owner. I create a integrated server without a owner, nobody expects so its a giant hack.

So the line falls back to the source:

so why fix something that somebody else broke? :)

Nightreaver commented 5 years ago

Well... the purpose of pregen is to generate terrain so you dont have to worry about it when you actually play. If it has a performance impact or not doesn't matter... thats the purpose of using PRE-gen. So I can understand forge did some consideration... but for pregen, just make it work well so we can enjoy playing afterwards.

Speiger commented 5 years ago

Yeah but the issue is if you can generate the 10000 chunks 10 minutes faster because of 1 tiny change or save hours on large scale servers that generate millions of chunks then people will take the fastest&relyable version that they can find.

The idea of ChunkPregen is that it is the most optimized version + the most features. It makes sure even if a crash happens you it won't have to restart from the beginning and actually can continue where you left of.

Forge is the most primitve and basic implementation you can find and without my chunkpregen version it would still take 6GB of ram just to pregenerate 40k Chunks while chunkpregen stays at sub 1.5GBs in worst cases for the same amount of work.

@Nightreaver ChunkPregen is the only "pregenerator" that has now 2 million downloads that exist in mc. Before that the best one was at around 100k and existed longer. People prefer mine because it has a lot of control for the user, tries to make sure you don't run out of ram and keep best performance and even if you run out it has backup plans that ensure that you never actually do. (Killing the game and picking where it left of when the server restarts) And even if it crashes you are at worst 1024 chunks behind which is less then a minute of work.

Nightreaver commented 5 years ago

I understand, though I think people use yours because its the only known one. I've been looking for a pregen for many times in the past years and there was simply no solution before that. IMHO most people just wanna pregen to have a smooth game, no matter how long it takes.

Speiger commented 5 years ago

Forge, Nuclear, AdminToolBox, ChunkGen The first three are well known and were praised for what they did. And I had to do a lot to compete with these because software like this builds on quality/efficiency.

And I only got so much usage after I had the preview feature. Before that every other pregenerator had more usage then mine. (And I did a hell of a lot advertising because else you do not get big in forge modding)

Nightreaver commented 5 years ago

Hey, no one is complaining about the effort you put here, so don't get me wrong. I'm just saying that people probably don't care the time loss.

Anyways..

Even though its more effort, how about maintaining an JEID version, and a vanilla one? Or change the algo if JEID is present? Or change the algo based on config settings?

Speiger commented 5 years ago

Hey, no one is complaining about the effort you put here, so don't get me wrong. I'm just saying that people probably don't care the time loss.

^^

Even though its more effort, how about maintaining an JEID version, and a vanilla one? Or change the algo if JEID is present? Or change the algo based on config settings?

Simple answer. No. I have a lot of other work to do right now and I don't have enough time.

SSyl commented 3 years ago

Seeing as this hasn't been touched in almost 2 years now, is it safe to assume I shouldn't use JEID with Cunk Pregenerator?

I'd rather have the performance boost from Chunk Pregenerator, so I'll be dropping JEID for NEID in my modpack.

Bexin3 commented 2 years ago

Any news on possible fix, or anyone has file with that needed change? NEID is not enough in my mudpack and causes issues with some mods so have to use JEID, however as of now apart from structures I get empty worlds

x0rp01s0n commented 9 months ago

any updates?

BinBashBanana commented 8 months ago

Fixed by https://github.com/TerraFirmaCraft-The-Final-Frontier/RoughlyEnoughIDs/pull/44

danielbro9 commented 4 months ago

Use roughlyenoughids it will fix it

Bexin3 commented 4 months ago

Use roughlyenoughids it will fix it

That's not a fix but a workaround, which doesn't work for everyone as they're mods that need you to use JEID

ACGaming commented 4 months ago

RoughlyEnoughIDs uses the same mod ID, so it shouldn't have dependency issues.

danielbro9 commented 4 months ago

Use roughlyenoughids it will fix it

That's not a fix but a workaround, which doesn't work for everyone as they're mods that need you to use JEID

I have yet to find a single mod that requires JEID not working with REID instead. REID is the official continuation of JEID and is almost the exact same with added compatibility features like auto converting NEID worlds to the JEID 1.13 stye. Trust me any mod you use that requires JEID will also work with REID.