add LuaTrain to delivery events

[loneguardian]

I would like to propose adding the LuaTrain reference to the payload of on_delivery_pickup_complete and on_delivery_completed events. This will facilitate other mods which uses this event to alter the train - currently with just a train ID other mods have to do a train scan or in my case, maintain a lookup table.

[0ptera]

I think I recall additional overhead passing game objects in events since those objects are actually C++. But even without that a local lookup table will always be faster.

[loneguardian]

I agree that lookup table is faster, but from my point of view, maintaining the lookup table is the actual burden here. With my limited understanding of how Factorio mods work, API calls are the expensive part of a mod, and I try to eliminate that.

I am aware that the on_train_created event is good enough for a train lookup table, and the amount of train IDs shouldn't cause problems in a normal situation. However, the advent of Space Elevator in the Space Exploration mod changes that landscape. The way Space Elevator works as of now causes a lot of TrainID and LuaTrain events.

A forum post of someone who is concerned can be seen here: https://forums.factorio.com/viewtopic.php?f=18&t=102762

That implies every train lookup table (at least for those that deals with an absurb amount of trainIDs) now ought to have some kind of cleanup mechanism - either by (i) listening to train created events and ALL rolling-stock mined/destroyed events, or (ii) some kind of periodic scanning of the lookup table to clear invalid trains.

I will leave it to you to weigh the implications and I respect your decision since I am not the expert here.

[0ptera]

I stress tested handling with a mod raising train-id way faster than Space Exploration. That mod flipped all reverse locomotives whenever a train changed direction. To flip a carriage it needs to uncouple from both sides, rotate, coupe on both sides resulting in +4 on train id if it was coupled on both sides While annoying to handle, the performance impact wasn't really noticeable.

There is only 1 event for train creation and 4 for removal:

• on_train_created handles all train creation and id changes
• on_pre_player_mined_item, on_robot_pre_mined, on_entity_died and script_raised_destroy handle removal of entities. The first 3 can be filtered with filter="rolling-stock" for better performance.

Periodic cleanups are not required, the table should be kept up to date with the events mentioned above. Though also check for train.valid on first time a lookup is done on a tick. For example deleting a chunk or surface will remove all entities on it without raising any entity specific events. In other mods I entirely rely on that method to find no longer existing entities and don't even subscribe to removal events. That's up to your preference.

[loneguardian]

I tried comparing 50 data points of execution times for two implementations, from just before on_delivery_pickup_complete event is raised, to allocating the train object to a local var in my mod:

Scenario 1 - Lookup:

1) LTN raises event without LuaTrain object 2) My mod uses TrainID given by the event for lookup 3) Lookup table returns LuaTrain object 4) My mod set a local var ref to that LuaTrain object

Scenario 2 - LuaTrain:

1) LTN raises event with LuaTrain object 2) My mod set a local var ref to that LuaTrain object

My impression is that Lookup isn't significantly faster, but LuaTrain does have more variation in terms of execution time.

TLDR: Both approaches do not seem to differ significantly.

Limitation of this comparison: Both scenarios were timed using a profiler that was controlled through remote calls (I couldn't find a way to use the same LuaProfiler object across mods by reference)

Implication: If the handling of rolling-stock events is indeed insignificant according to your stress test, I'll go with the Lookup approach and move on for now. Bear in mind that other mods that depend on these events are doing the same handling on their own.

[0ptera]

Added in next version.