Train lookups about factorio-stdlib HOT 5 CLOSED

LuaTrain is not actually an LuaEntity, it's a meta-object which contains a collection of entities (locomotives + wagons). That is why the Entity class methods don't work.

I definitely agree with @Choumiko that train library stuff is useful and I would absolutely assist / accept pull requests. My one suggestion to your example is to your naming/organization scheme. If you want to search for an object on a surface, then the search probably belongs in the Surface class. So instead of Trains.get_all_trains_surface, it would be more clear to have Surface.get_trains(...). Alternatively, if you want a more generic train-searching function, you could name it Trains.find_filtered({surface = ..., force = ..., area = ...}) which match how the existing factorio find_filtered methods work. Basically, try to match the existing factorio naming scheme, it makes it a lot easier to figure out where to look for stuff.

All said, Train API sounds super-useful!

from factorio-stdlib.

Hi Afforess,

I'm glad to hear you're open to it. I've put together a rough, proposed API structure and usage examples below. If you can cast your eye over and give me any feedback you might have, that'd be appreciated.

I've added some train events to this class because they're very train specific, but I can see an argument for adding them to the event system.

stdlib/trains/trains

--- Enables or disables the library's train tracking features. Defaults to disabled.
-- <p>If train tracking is disabled, you may get references to trains that no longer exist (eg because the carriages were uncoupled).
-- If you need to use this librarie's train features, you should enable train tracking</p>
-- <p>Train tracking requires binding to different Factorio events, using this libraries event module.
-- If you need to use train tracking, you should also use the event module. Otherwise, leave this disabled.</p>
-- @usage
----Trains.enable_train_tracking(true)
-- @param bool enabled
-- @return void
Trains.enable_train_tracking(enabled)

--- Given search criteria, a table that contains at least a surface_name,
-- searches the given surface for trains that match the criteria
-- @usage
----Trains.find_filtered({ surface_name = "nauvis", state = defines.train_state.wait_station })
-- @param table criteria A table with at least the surface_name key
-- @return A list of TrainDetails, if any are found matching the criteria. Otherwise the empty list.
Trains.find_filtered(criteria)

-- example criteria structure
criteria = {
  surface_name : string,
  state : int, -- uses defines.train_state
  num_carriages : int -- this sort of thing isn't that important, but could be helpful
}

--- Given the train name, finds the corresponding TrainDetails
-- @usage
----Trains.find_train('super_train') -- names are given by TrainDetails
-- @param string name The name of the train
-- @return TrainDetails or nil if none found
Trains.find_train(name)

--- Registers an event handler to be called when the given train changes state
-- For more, see http://lua-api.factorio.com/latest/events.html#on_train_changed_state
-- <p>This library will handle cleaning up event handlers when a train no longer exists.</p>
-- @param string name The name of the train to register the callback for
-- @param function callback A function accepting a single parameter
-- @return void
Trains.on_train_changed_state(name, callback)

--- Registers an event handler to be called when a train has been removed.
-- <p>This is a custom event fired by this library</p>
-- @usage
----Trains.on_train_removed(function(name)
----  logger.Log("Train was removed: " .. name)
----end)
-- @param function A function accepting the name of the train as its only parameter
-- @return void
Trains.on_train_removed(callback)

I'm sure there are other events that would be helpful, but I think you get the picture of what their signatures would look like.

stdlib/trains/TrainDetails
In order to make trains work well with the existing entity system, we need to provide a duck type for them. This is it.

TrainDetails = {
  name : string, -- derived from the locomotives in the train
  train: LuaTrain -- reference to the underlying Factorio train instance
  -- we could potentially copy some of the details
  -- like number of carriages, current state
  -- up to this level but I don't see the point
}

from factorio-stdlib.

The overall scope looks good @AndyBursh, I'm not too worried about the library becoming esoteric. Trains are a core part of Factorio, so it certainly would follow that they need specific library functions.

I do have a couple suggestions based on your outline. First, I don't think you need an explicit Trains.enable_train_tracking(enabled) method. Stdlib classes are only loaded as requested by modders, so using whether or not the train class was loaded is enough information to tell that they want train information. Anyone who wants the Train api will have a require stdlib/trains/trains in their control.lua scope and that is enough to tell us that they want the train API!

Second, instead of the callback methods, I'd prefer you created fields that were event_ids, and let modders use the Event system for callbacks. It's more consistent and means less repeated code. See the stdlib/event/time for an example of new events.

Other than that, everything looks good. Looking forward to seeing the actual implementation.

from factorio-stdlib.

That all sounds reasonable to me. I'll submit a pull request in the coming days.

from factorio-stdlib.

Closing this since it's been merged now.

from factorio-stdlib.