MineMaarten / Signals

Minecraft mod that adds OpenTTD style signaling and more!
GNU General Public License v3.0
17 stars 14 forks source link

Don't generate chunks when exploring rail network #80

Closed AntiBlueQuirk closed 6 years ago

AntiBlueQuirk commented 6 years ago

In a related issue to #63, I've found that Signals still doesn't play well with worldgen rail networks. Specifically, when exploring the network, Signals will generate chunks to explore the network. In Lost Cities worlds, depending on configuration, the subway rail network can span the entire world. This means that the rail network exploration can theoretically never end.

As an example, I placed a signal next to a rail in my world, and realized my mistake quickly when the server ground to halt. Afterwards, I saw that it had explored a single rail line out to 16 kilometers away from spawn, generating chunks in a line along the way. (The farthest we have explored is out to 5 km.) At the end of this rail is a single missing track, part of random damage, suggesting that it would have kept exploring until the server crashed if able.

signals-railgen-bug

Because of this, I suggest checking to see if a chunk is generated before probing a block for rail network objects. This will prevent Signals from generating chunks while exploring networks. I really don't think there's any reason for Signals to generate chunks at any rate, since world gen rail networks are rarely appropriately signaled.

I'm not really familiar with the code base, so I'm not sure if this is the only change required. I also haven't set up a build environment for this mod, so this change is untested, but it's a very simple change, so I don't think it should be hard to check.

MineMaarten commented 6 years ago

Just this change won't do it I don't think. The thing I'm afraid of is when the chunks get eventually loaded, I don't think the new rails are included in the rail network without giving a neighbor block update adjacent to one of the new rails.

This is why this isn't included yet, I still needed to think about a performant way of marking the newly generated parts dirty when chunks get loaded. Could possibly done by marking the rails dirty next to the loaded chunk, but because there could be a lot of rails to filter (the whole rail network), it might require having some objects to partition the rails in chunks, so not all rails have to be queried for their xyz.

MineMaarten commented 6 years ago

Hmm now I think about it, this might do it, as Signals, when placed down (or generated) will try to build the network surrounding them, so even if a signal is worldgenned (I might decide to add worldgen later), the Signals themselves should make sure the network is properly built with the new rails in mind.

By using a 'is the chunk genned' instead of 'is the chunk loaded' it should work better (I previously had the latter in mind :P).

I will still so some testing to make sure though

AntiBlueQuirk commented 6 years ago

I guess I hadn't considered that Signals might add some worldgen itself. (It would be pretty cool.) I don't know if worldgenned blocks trigger block updates. If they do, then rails should just add themselves to the network after a chunk is generated, but I don't think that's the case.

It might be easier to just keep track of which rails are connected to ungenerated chunks, which should be a pretty small list. Then just respond to the chunk generation by checking this list and restarting network discovery for any rails next to the new chunk. If a signal gets worldgenned, it'll just start network discovery with what's available, and continue searching as chunks become available.

MineMaarten commented 6 years ago

I think you slightly misunderstood when I said: "Hmm now I think about it, this might do it" 'this' was referring to this PR :). I think, without any extra additions.

This is because when new Signals would be generated, the first thing they do is try to 'attach' themselves to a network. This would then automatically include the newly generated rails also. The only thing I need to do before merging is to test it, which I hope to find time for this weekend.

Your alternative suggestion does sound like a good approach though if the above does fail somehow :). Indeed, keeping track of the 'not generated chunk rails' should be a small list and therefore fast to query on every chunk load.

MineMaarten commented 6 years ago

Tested and it looks good. There is a situation where it will go wrong when Signals worldgens networks, but this is a TODO for later and will probably be resolved using your suggestion of keeping track of rails next to ungenerated chunks:

2018-08-25_11 33 50

If the red chunks get generated, the Signals on the red chunks will make sure that this part is included in the network. However, when the yellow parts get generated, the rails in that section won't be part of the rail network until a neighbor block update occurs next to the rails on the border of the two sections.