NoxyNixie / Noxys_Multidirectional_Trains

The Unlicense
3 stars 2 forks source link

Interesting interaction with Logistics Train Networks #2

Closed kirazy closed 5 years ago

kirazy commented 6 years ago

Symptom: LTN dispatcher generates duplicate requests to the max number of trains for a given station (i.e. if a request for 75k crude is needed, and the station can handle up to 4 trains, it will generate 4 trains for the same request).

My hypothesis: When the rear train is rotated, Factorio is "remaking" the train, so it's not technically the same train, and so LTN regenerates the request as it doesn't think any train is assigned.

Given that this is an issue with mods interacting strangely, whether you care about this or not is up to you. Just wanted to mention it, given the prevalence of LTN (and the awesomeness that is this mod fixing the one downside to dual-headed trains).

Latest version of Factorio, LTN, and this mod (0.16.30, 1.7.5, 0.0.1 respectively.)

NoxyNixie commented 6 years ago

This is something LTN needs to handle on their end.

Yes the trains are created because I'm having to resort to uncoupling and recoupling the trains for the rotation to work.

LTN can handle this by properly handling train ID changes in the event on_train_created.

This event comes with the variables event.old_train_id_1 and event.old_train_id_2. All LTN needs to do is copy its train specific data to the new id.

Not unsurprisingly I already have something like that in my handler. All LTN needs to do is properly copy its states and there should not be any issue.

Unfortunately I can't fix this specific case on my end unless factorio allows rotating of connected rolling stock (which might not happen ever).

I may look into doing a pull request to LTN to fix their end but that may take a bit since I'd need to test with it first and I've never used that mod so a test scenario is in order plus I'd need to understand their code. Currently there is not much I can do.

Thanks for the report though its always appreciated.

0ptera commented 6 years ago

LTN invalidates active deliveries whenever train.id changes. This prevents deliveries being active when the corresponding train has been destroyed or removed by script. I see no reason to change this.

NoxyNixie commented 6 years ago

In my opinion this is wrong since the train is not always destroyed or removed by script when on_train_created is called.

More accurately the train is merely uncoupled and coupled real quick with my mod and should not be seen as "destroyed" by LTN.

If you @Yousei9 do not want to work on this I may do so (in the form of a PR) when I have some more time.

0ptera commented 6 years ago

I tried to wrap my head around the scenarios and came to the same conclusion as before. I simply can't properly migrate constantly train ids without a major rewrite of LTN.

Scenario 1: It fails to couple all carriages together Suddenly there are two, or more, trains running the same delivery. In LTN train id is the primary key for deliveries. There must not be more than one id per delivery.

Scenario 2: It couples 2 trains to one There's no way to discern which schedule that train will get. Probably the one with the lower id. I'd have to take a look at the schedule 1 tick after it coupled and decide by that which delivery this new train is trying to do. The overhead alone makes me not want to implement this.

Even outside these edge cases the way Factorio keeps throwing the event on_train_created in rapid succession makes me cringe.... it will take so much logic just to decide how to merge the ids. While the possibility for an api extension that can rotate without decoupling exists I won't pursue this further.

NoxyNixie commented 5 years ago

Release 0.0.8 should fix this.