Haggle1996 / RevolutionPack

Revolution Pack for Minecraft 1.7.10
16 stars 17 forks source link

Pregen world prevents sluice from working #462

Closed Nagapito closed 8 years ago

Nagapito commented 8 years ago

Pregenarating the world, at least with the PregenSpawn command does not generate the HardOre slice data for the chunk and the sluice will think there are no more ores available on the chunk and produce nothing.

My pregenerated world has the HardOreData generated for the chunks around spawn that were generated normally. The chunks that were pegenerated have no HardOreData, even with ores in it.

Time to start a new world...

Haggle1996 commented 8 years ago

@Draco18s, fyi. We tell players to use AdminCommandsToolbox if they want to pregenerate a world, though the pack doesn't require it.

Thanks, @Nagapito, for reporting this is important and may solve some of the sluice problems. Just to clarify, this world was generated using the latest version of REV|2?

Nagapito commented 8 years ago

it might actually not be related with the pregeneration.

So, started a new world, wondered around until I find a nice place to set home and decided to cheat in the items and test it before I waste more time. Glad I did it.

Sluice not working again. I googled a little and actually found your closed issue about the sluice and flowers not working. So, I tried the bone meal and no flowers. This, while seeing 4 iron ore blocks on a hole 7 blocks away from me. Counted the iron blocks with COFH, eighty something!

Seems the previous issue is still not totally fixed and sometimes the data is not generated... Could it be that when the world generation is stressed, it doesnt have time to generate the data? I was flying in creative just looking around for a nice place to start my game and generating chunks like a mad man...

if @Draco18s needs, I am available to test with a dev version. I am a dev to, logs dont scare me :)

Haggle1996 commented 8 years ago

Something just occurred to me, @Nagapito. Did you generate with Vanilla world type instead of RTG or BoP?

Draco18s commented 8 years ago

BTW, what version of my mod are we running here?

Haggle1996 commented 8 years ago

If he's on the latest version, it's 1.7.10-HarderOres-14.25.1e

Draco18s commented 8 years ago

That's the question: if he's not latest then the problem is that he needs to update. If he is...hoo boy.

Haggle1996 commented 8 years ago

You're not going to like this, but I tested it on 14.25.1e with the same seed, both pregenerated and not pregenerated. The good news is that this is very easy to replicate. :)

On a pregenerated world, all flower/ore data is missing. It appears as though it's actually deleted, as even the spawn chunks are stripped of their ore data.

On the same seed, but flying manually to the same several spots, the sluices produce ore and flowers spawn with bonemeal, even in the spawn chunks.

In the interim, I'm going to remove AdminCommandsToolbox and recommend players do not pregenerate.

Haggle1996 commented 8 years ago

Just some insight into how AdminCommandsToolbox works: it generates 1000 chunks before actually saving chunks to disk. I don't know if possibly there's some array or similar that's overflowing (or what I would even look for in the log).

Draco18s commented 8 years ago

That's interesting. I don't know what AdminCommandsToolbox does either (I've never used it) but it does sound like something it does skips over an event required by my mod to save the data. My guess is that all mods that have per-chunk data like this would be effected.

Thing is, my mod is supposed to detect the missing data and do a re-scan.

Toksyuryel commented 8 years ago

I believe Immersive Engineering adds per-chunk data for its excavator veins, so it would be worth checking to see if those happened to spawn in the pregenerated world.

Nagapito commented 8 years ago

Sorry for the delay, I went to bed.

Latest version of the pack with the latest version of the mod. I even checked Curse to see if there was a new version.

My first world with this issue was pre-generated and played on a client. This second world, is running on a server and chunks were generated by me flying crazy fast and waiting for them to show up, so I was making the server strugle with world generation.

While ACT does not save the chunks immidiatle, so doesnt Minecraft. It would take a couple minutes for MC to actually write stuff on disk. I noticed this when forcing worldgen and waiting for it appear on the files so I could check them with NBTExplorer. And, to be honest, its good MC does not write to the disk every single time you break a block or a cow moves... If it did it, I would need a new SSD every month!

I just searched for my chunk on the save files and HardOreData NBT only has one entry called 'version' with the value 2. Same for all the chunks around it. This are chunks that actually contain HardOres, since I can see them.

And this might also be affecting other mods, since WildlifeTreeData nbt data also only contains one entry with the name version and value 2! But this might also mean nothing since it also only has this entry in chunks that HardOreData actually has proper values.

If you want, make a a special version of the mod that logs everything about the missing data and rescan and see if we can figure out why rescan is not working.

Nagapito commented 8 years ago

My skills reading java code are not the best, I am a C# developer, so dont get too mad if I incorrectly assume something.

On your OreDataHooks, your ReadData function will only fail and force a recount of ores it doesnt find the NBT tag HardOreData. But the tag exists with the version=2 child... So, it assume the chunk has the information generated and tries to read the details, but since there are no details, it never adds the hash with the data to your graphs storage.

Later, the getOreData function, will try to access this hash in the graph, but since it does not exist, the function returns 0 instead of starting a recount when the hash is empty.

Now, the part that I am not sure. The chunkLoad and chunkSave are events. True nature of events mean they are async. If they are async, there is nothing that can prevent the chunkSave event to be fired right after the chunkLoad, before the chunkLoad event as time to finish. So, you would be saving an empty NBT data...Under heavy load and low memory available, it makes sense the engine tries to save and unload chunks the fastest possible, to release memory.

My suggestion: save function should not always create the NBT tag. It should only create the tag when it finds the first ore to save. Load function should check if there are actually slices inside the NBT tag. If not, recalc. getOreData should recalc if the map is null, since it means that the map was never loaded!

Or I could be totally wrong and out of mt league :)

Draco18s commented 8 years ago

We can certainly try doing that, see if it helps. The whole chunk save/load/unload process is woefully annoying. The worst part is that the Unload event (to remove data from the hashmap) and the Save event (to write the data) have virtually no relation to each other (e.g. chunks can get saved without being unloaded). There was a point during my original coding--before anything was released--where I'd get an event sequence like this (for a single chunk): Save Unload Save

So it would literally save good data, remove the data from the map, then be asked to save that data again and not have it.

Draco18s commented 8 years ago

Alright, give this a go. https://dl.dropboxusercontent.com/u/7950499/1.7.10-HarderOres-14.25.2b.jar

You may need the updated Lib, but I don't think so (some API alterations were made for Harder Wildlife last week, although that update isn't complete yet, there's still a persistent issue that needs to be tracked down). But here it is, if you do need it. https://dl.dropboxusercontent.com/u/7950499/1.7.10-HardLib-14.25.2b.jar

Nagapito commented 8 years ago

It actually makes sense.

Multiple saves can be called while its loaded and always call a last save after then unload to make sure that all changes between last save and the unload are saved and nothing is lost.

An easy solution for that is, on the unload, force a save. So, when it asks for the unload, you save the NBT with the latest valid data and unload the data from the map. The next save, will do nothing since there are is no data on the map and the chunk keeps the last saved data from the unload.

This is, assuming that if you dont save nothing to the NBT, what was previously there is not lost. If its lost, then, you need a second map, where you store the unloaded data and on save, if you dont find it in the normal map, you search of the unloadedMap. If its on the unloadedMap, save that and remove it from the UnloadedMap.

Ok, testing the new version. Will report back in a few minutes

Draco18s commented 8 years ago

Can't save data during an Unload event: no access to the chunk NBT. That's why it's a problem!

Nagapito commented 8 years ago

I dont bring good news... Since the majority of the chunks didnt had the data generated, at load time its trying to generate data for all of them and taking a long time. So long, that the client entered that weird state where the world is still loaded and running but you are back on the main menu screen.

Restarted client and tried it again, but for some reason, its crashing while trying to track some entity. Fresh backup, tried again and same issue... Crash log only from loading the world, http://paste.atlauncher.com/view/b6be0b15

This made me think of a couple of things, while I waited. First, how MC reacts if you are scanning a chunk while its getting unloaded.You know, loading world, loads some chunks and unloads them right immediately. Might cause a lot of issues reading/saving the NBT

Second and more importantly. You might not need any of this. if I understood you are reading/writing to the chunk information that you only use in two scenarios, bonemealing and sluicing. Why use this heavy and prone to bugs mechanic? Why not calculate the mapping of the chunk at runtime when one of this tasks require it?

Sluice starts, no mapping, scan the chunk. If there is a map, use the map. Chunk unloads, remove the mapping. Dont store it or rebuild it from stored data. Sluice scans a 3x3 area? big deal. Much better then a 8x8 or 10x10 for each player...And this will only happen once per loaded chunk on sluicing or bonemealing!

I think that removing the NBT dependency will speed up the loadings, remove the bugs and cause almost negligible load on clients or servers. Sometimes, is best to just recalc the data then store it for later use.

Unless I am missing something!

PS: Totally missed the detail of no NBT on unloading. That is... weird, specially if you can trigger a save after unload. This is the bad java conventions. Event should be called chunkUnloaded, so, no access to his data since its already unloaded. ChunkUnload to me its a ChunkUnloading, so you should still have access!

Nagapito commented 8 years ago

Also forgot to mention that, as you can notice in the logs, chunks are being scanned twice...

Nagapito commented 8 years ago

Also, reverted to the 'latest' oficial version, game loaded without entity tracking issues but only one rescan message on chunk 0,0

Draco18s commented 8 years ago

I can't calculate at use-time for two reasons: 1) Mining blocks shouldn't alter the data (the 'dust' had already leached info the soil, etc.) 1b) Silk Touch and placing ore shouldn't alter the data! 2) Using the sluice depletes the data (because it is "free" ore, you shouldn't be able to set it atop a couple ore blocks and sluice it forever)

Draco18s commented 8 years ago

Alright, try this instead https://dl.dropboxusercontent.com/u/7950499/1.7.10-HarderOres-14.25.2c.jar

It queues up rescan operations and does 1 per 5 ticks.

Nagapito commented 8 years ago

Rescan queue mechanic seems solid except for the part where it never fired :)

Nagapito commented 8 years ago

ohh, and I thought the sluice was depleting the ore blocks. At least that is what I read somewhere. So, after a long time, if I tried to manually mine, the ore blocks would be empty. That is why I thought there was no need to rescan, since the data would be equal to the ore blocks in the chunk.

Draco18s commented 8 years ago

FFS. Chews on the event

depleting the ore blocks

No no, it doesn't actually destroy any ore blocks anywhere.

Draco18s commented 8 years ago

Ok, the event works for me: [16:43:42] [Server thread/INFO] [STDOUT]: [com.draco18s.ores.OresEventHandler:worldTickEnd:341]: World Tick Event Called [16:43:42] [Server thread/INFO] [HarderOres]: Chunk [0](-26,0,-4) is missing ore data, it will be rescanned. Chunks way out on the edge of the world may not save and cause this message to repeat next launch; do not be alarmed. [16:43:42] [Server thread/INFO] [HarderOres]: 488 chunks remain.

Nagapito commented 8 years ago

I will give it another try and move around to load and unload chunks.

I only loaded the and stared at the console... Also, running it on SSP, server takes even more time to stop/start. Shouldnt affect it, right?

Nagapito commented 8 years ago

Nope, nothing on the console.

Moved around, generated new chunks... TPS at 20.

Unless doRescan is actually called but doing nothing. Failing on the if statement...

Draco18s commented 8 years ago

Are you seeing INFO level notifications?

Because this is the code involved:

http://pastebin.com/wU6kQ119

Nagapito commented 8 years ago

You can confirm it yourself, http://paste.atlauncher.com/view/0a4c8d7f And, yes, that is the code I am seeing when I decompile the jar that is being used in the client. Also checked in-game mod list to confirm I really have the correct version, 14.25.2c

Nagapito commented 8 years ago

My decompiled code has a slight difference, if ((event.phase == TickEvent.Phase.END) && (event.world.func_72820_D() % 5L == 0L)) {

But mod of 2 or 5 should still produce a 0...

Draco18s commented 8 years ago

The %2 was a change I made in dev to just get my machine to chew through entries faster (I was loading a flat world which has no ore).

Try generating a new world.

Nagapito commented 8 years ago

Generated a new world and it worked \o/

Stop the world, loaded it again, and it was still working.

Load the previous world that was not working and it start working!!! Why? No clue :P

Nagapito commented 8 years ago

I still see the rescan being called twice to same chunks And the stupid constant load/unload of chunks my MC is also screwing the generation. Was down at 5300 chunks from 5500, I am again at 5500 chunks

http://paste.atlauncher.com/view/e0200bb4

Seems like this number is going to take a lot of time until going down

Nagapito commented 8 years ago

And suddenly it stopped, claiming there were still 5709 items in the queue. Its just me or is that size desynced?

Reloading the world started it again and size jumped to 6800

ps: stopped generating again after a while. moved around, generated new chunks... nothing

Nagapito commented 8 years ago

What happens if you try to generate data on unloaded chunks? Like, it queues 3k chunks to generate data but while its doing it, I move away 500 blocks. Those chunks are not loaded anymore...

And every time I close and load a world, the queue size just gets bigger and bigger. Seems like the queue is not reseting when I reload a world.

Could it still have the queued chunks from previous reload and being unloaded causing the generator to stop?

At 10k now....

Draco18s commented 8 years ago

The code does not in any way attempt to figure out if the chunk is still in memory or not. It was just a "make a queue" it's not optimized yet.

The counter on how many chunks are left is simply a counter of how many items are left in the queue, but due to the asynchronous nature of load/rescan, it is possible to add new chunks to the queue while it's still scanning.

Don't know why it would stop scanning with remaining chunks left. It does check that the chunk that's about to be rescanned is part of the world being ticked, but that shouldn't prevent the rescan from completing unless the chunk next in the queue is part of a world that is fully unloaded (e.g. inactive Mystcraft dimensions).

Nagapito commented 8 years ago

So, if for some reason, a chunk is momentarely loaded on another dimension and unloaded again, since that dimension is not ticking anymore, the whole system stops.

That could explain why my generation stops, since this packs has like 50 dimensions and most of them are constantly loaded (hate that feature in galacticraft...)

I also uploaded the new version to the server and he never did try generating the information... probably a similar issue?

Draco18s commented 8 years ago

Could be. I'll pick this up tomorrow and do some more clever queue shuffling.

Nagapito commented 8 years ago

Just restarted the server and its now generating, even before I joined it.

The most amazing thing is it is generating for chunks everywhere!! Chunks like -50,-50 or 0,4 or, 20,55. Not all this chunks so far apart should be loaded.... Something is really messy on how MC loads chunks

Nagapito commented 8 years ago

Suggestion: Add a config option to blacklist dimensions for harderOres or at least for generating the mapping. No point generating info on dimensions that will never have harder ores ore sluices like nether.

I just notice after I left the nether for the 1st time, queued chunks skyrockets to 100k and sent my TPS down to 12...

Haggle1996 commented 8 years ago

@Draco18s any movement on this?

Draco18s commented 8 years ago

I did not get back to it "tomorrow." I try to devote my weekend time to fixing my mod bugs as my weekday hours are on my other project. "Weekend hours" basically didn't happen this last weekend.

I haven't forgotten though.

Haggle1996 commented 8 years ago

No worries -- I know you stay on top of stuff; I just wanted to make sure there wasn't another 'oh, forgot to publish that' moment. :D

Draco18s commented 8 years ago

Whew! Finally got around to making the queue system smarter. It now has a separate queue per dimension (which can be blacklisted, blacklisted dimensions skip the entire ore data system, later re-enabling it will cause every chunk to be put into the queue when the world loads). Dims -1 and 1 (Nether and End) are blacklisted by default. @Nagapito https://dl.dropboxusercontent.com/u/7950499/1.7.10-HarderOres-14.25.2d.jar

Haggle1996 commented 8 years ago

Oooh...new bits to test!

Haggle1996 commented 8 years ago

I can't get any flowers to spawn using bonemeal with 25.2d

EDIT: And of course, the sluices don't work for same reason. On the plus side, chunks generate faster in the non-overworld. I assume that's related to your blacklist. :D

Draco18s commented 8 years ago

I got flowers in dev just fine.

Nagapito commented 8 years ago

Confirm, nothing is happening...

Dont see any info on logs about generating the information, bonemeal does not work...

Could you make a version that logs everything, that just spams my console! Like, 'Entered TickEnd event for dimensionID: x', before the if, calling doRescan if it passes the if.... Just log every step before a condition and after it, so we can understand if its entering the condition and if not, why?

I ask this because, with all the extra mods and extra load on the client that I have, your code might behave a little different then only with a few mods.

Haggle1996 commented 8 years ago

Every time I load the same world, I get this exact message, but bonemeal and flowers do not work in the spawn zone, or out at a distance from spawn. I never get messages for any other chunks.

[14:16:04] [Server thread/INFO] [HarderOres/]: Chunk [1](0,0,0) is missing ore data, it will be rescanned.  Chunks way out on the edge of the world may not save and cause this message to repeat next launch; do not be alarmed.
[14:16:04] [Server thread/INFO] [HarderOres/]: 1 chunks remain.
Draco18s commented 8 years ago

That message is for dimension 1 not the overworld.