Applied Energistics 2

Applied Energistics 2

137M Downloads

minMeteoriteDistance functionality doesn't entirely make sense

unexceptable opened this issue ยท 10 comments

commented

Describe the bug
minMeteoriteDistance doesn't seem to really work all that consistently. If set to smaller numbers it does appear to work, but it seems that the generation code doesn't actually take into account distant ones, and will spawn them all too close.

I'm trying to make them rare, but no matter what value I set, this doesn't really produce the intended result. There are simply too many around.

To Reproduce
set minMeteoriteDistance to 999999. This outright breaks it, and causes them to spawn all the time.

set minMeteoriteDistance to 3500, look around for spawns, measure distance between them. Doesn't seem to actually honour the 3500 setting, and I've seen them spawn as close as 400 to each other.

set minMeteoriteDistance to 100 appears to work consistently enough, so clearly the config isn't outright broken.

Expected behavior
I expect the value to be honoured when set, so I can control how rare the meteorite spawns are, or some other value to control rarity if this value cannot be honoured over larger distances.

Environment
This is in the FTB Modpack Revelation version 3.2.0

  • Minecraft Version: 1.12.2
  • AE2 Version: appliedenergistics2-rv6-stable-7.jar
  • Forge Version: forge-14.23.5.2846
commented

@yueh not the first report in 5 years ...
https://github.com/AppliedEnergistics/Applied-Energistics-2/issues?q=is%3Aissue+meteorite+distance
this was reported several times already and never fixed, just closed
e.g. #3857
often it was just overlooked, that the intended behaviour seems to be broken. (or the config value is broken, I have experience the issue myself and only found the closed threads, that don't really address the issue, but since the are all closed I did not care to create a new issue)

commented

The distance calculation is still based on ints internally using a basic pythagorean theorem. Therefore at some point it will start to overflow. Currently that is around 32768. Feel free to calculate the intervall in which it goes negative and positive ;).

Should probably be documented somewhere, but at the rate of 1 report in 5 years or so, it is probably not that urgent. Also a distance of 32k blocks would mean about 1 and a half hour of sprinting between each meteor. Not taking the terrain, day/night cycle or similar into account. I'd say that should even be sufficient for grind oriented packs.

3500 should be work, but keep in mind there is also a chance to spawn cluster meteorites. These will cause some meteors to be much closer. I currently cannot remember, if their distance is related to the minMeteoriteDistance, if it could explain the closer one.
Also the generator is clearly not thread safe, so in case a pregenerator is used and it tries to parallelize it, it will most certainly run into issues.

Could you please share a bit more details about how you generated it?

Just in case, if it is based on some pregenerator, I probably will not do anything about it for 1.12 anymore. The current approach is designed with "manual" exploration in mind. I have a few ideas how to change it, but that will most certainly break any existing world. So certainly something for a potential port to another minecraft version.

commented

Because they reported clusters, which are a feature. Reporting them as bugs is like reporting AE adds blocks to minecraft as bug.

This is the first one reporting a bug outside of the cluster range. Or not understanding that the minimum distance between meteors does not mean minimum distance from spawn.

commented

So I've got meteoriteClusterChance set to 0.0, so I know I'm not seeing clusters. It's possible that the modpack in question has some means of parallelized generation, although I'm not sure. All I did was alter config, and recreate the world multiple times, and just explore in creative mode flying around.

I'm guessing that the logic for getting nearby meteorites is entirely in memory, so any previously created ones it won't pick up, or maybe it clears them from memory after a while. I tried reading through the code, but my java is rusty. :(

Essentially it seems that only occasionally does the value seem to be taken into account, and only up to a certain threshold, so likely the code or memory is dropping references to previously created meteorites. Does it take into account previously created ones? Or is it purely based on what is in memory right at the moment?

I have a feeling this isn't easily fixable, and I remember reading in a different issue that you were reworking the meteorite spawning for newer versions. I'd suggest doing something purely probabilistic, as sure they may be closer occasionally, but it should be more consistent. I'm likely to be looking at building a 1.14 or 1.15 server sooner rather than later, so I'd be more interested in you getting this right in a newer version than the effort it may take to fix now.

commented

We have a custom format to persist the meteor details, so it will be saved.

The basic implementation is like:

  1. Queue spawning a meteor in every new chunk
  2. Check against registry if there is any meteor within the min distance
    a. Yes: Skip chunk
    b. No: Spawn meteor, register location
  3. Mark chunk as done

As long as it is run sequentially, it should work. Maybe the usual rounding errors.
If it somehow runs in parallel, it could create a meteor before it was able to register another nearby one.

I was thinking about using something like a hex grid as approximation for the min distance. But that is basically just a voronoi diagram of a very evenly distributed delaunay triangulation.
Something deterministic would be nice, but I haven't found a good way to work in an infinite space, which is only partly generated and also not continuous.

Purely random can easily run into some worst case situation, were a player won't ever find some because they just stay inside a small area, where no meteors spawn and just walking 1 or 2 chunks into a lateral direction would spawn them in. So it is not really a solution.

commented

Interestingly, if I make a minecraft instance with just forge, AE2, and Journey map, the config seems to act as expected.

With minMeteoriteDistance set to 1000, and after a short amount of testing I only ever saw meteorite spawns as close as 1100ish.

I can't see any mod in the Revelations modpack that looks like it would add parallel chunk creation (can that even be done with just mods?), so I'm not sure what makes it happen more often than I'm setting it. Is there maybe another mod that is interfering or conflicting weirdly?

And yes, I agree that purely random isn't ideal either. :(

commented

Certainly yes and might not even be on purpose.

FTBUtils for example did parallel chunkloading in I think 1.7. Not in terms of performance, but they did not check in single player, if the server or client thread was accessing a chunk. So it could cause the chunk to be loaded from the client thread. Something which should never happen and could run into some race condtions like a chunk being loaded twice or even something like loaded from the client thread while the server was just unloading it.
But certainly not blaming this mod here. Could be really anything.

commented

Keep in mind that is pure speculation. Could also be some other mod replacing the normal meteor generation or whatever else.

But locking is probably a pretty bad idea. Iirc chunkloading is now multithreaded at least for distant chunks. And iirc also there was the idea to multithread rendering. No idea, if that was in 1.12 or later. Or at all. But worst case is that we lock up a rendering/client thread for a couple minutes until the server thread is done generating the world.

And I am not sure, if it is even possible with the worldgen in 1.13 and later to spawn something with a min distance and being randomly generated. Vanilla is pretty much using pregenerated structures, which are copypasted around and are always identical.

commented

Ouch. So is locking meteorite creation if another thread is doing it possible? Like, don't lock/wait. Lock at the moment of entering the 'place' logic. If another thread hits that point it doesn't wait, it just exits as if the minDistance wasn't met.

Ideally what we'd want is a magical function that takes:
world_seed, min_distance, x, y
and spits out a deterministic yay or nay, but that would either be potentially crazy math, or very hacky. :(

commented

I was more thinking, don't lock, but rather using a locking like mechanism to trigger a no spawn if another thread is spawning one already. So a semaphore like construct around the placer logic, which if tripped just tells it not to place so the thread which hit the lock moves on, while the one holding the lock completes the place and moves on. But even that isn't nice, and can potentially cause some odd cases.