Train schedules are not copied from original blueprints

[elswindle]

I'm pretty sure this is known and you haven't implemented this yet, but I just noticed train schedules aren't being copied from blueprints. Trains that are added to new blueprints won't have schedules. Is this something you plan on implementing?

[redruin1]

Yes, it is. It's been hampered since there's been a lot of changes behind the scenes, compounded by the fact that I'm not sure what the best API for interacting with schedules would be, which is why setting custom schedules has to be done manually. If you have any ideas for what a good API for train schedules would be, let me know.

Copying train schedules into Draftsman should still be preserved when exporting that same blueprint from Draftsman, however; that can be fixed regardless of any API decisions. Lemme see if I can come up with a fix.

[elswindle]

I should clarify this a bit. I am importing a blueprint and assigning its entities to a Group and then adding the group to another blueprint. Maybe the transition from blueprint to group or adding a group to a blueprint is where the train schedule is getting lost.

Looked it over and it is definitely the transition from blueprint to group. There is no structure to store schedules within the group. It would also need to update the Association to the respective locomotives, so it's not a simple copy and paste. How this will work is up to you. We could add a schedule parameter to the constructor of the Group object or have group accept blueprints and copy over anything that might be useful.

[redruin1]

That makes much more sense. After a little bit of tweaking to the deepcopy methods (and the addition of a schedules attribute to Group, which I think is warranted), I managed to get this syntax working:

blueprint_string = "..."

rail_group = Group(string=blueprint_string)
print(rail_group.schedules) # [{"locomotives": ..., "schedule": [...]}]
blueprint = Blueprint()
blueprint.entities.append(rail_group)

print(blueprint.to_string())

This syntax works if and only if both the entities and schedules are copied at the same time during the same deepcopy cycle. If you don't do this, it makes determining which locomotives belong to which copy basically impossible, but the current API makes no effort to indicate this to the user. For example, an average user might reasonably expect the following snippet to have the same functionality to the above, but it instead generates an error:

test_string = "..."

rail_group = Group(string=test_string)
blueprint = Blueprint()
blueprint.entities = rail_group.entities
blueprint.schedules = rail_group.schedules

print(blueprint.to_string()) # Error: Locomotive not in list

Once the line blueprint.entities = rail_group.entities has finished, it "forgets" that old locomotive 0xABC was copied into new locomotive 0xDEF, so when we attempt to copy a schedule that has locomotive 0xABC we have no idea which locomotive (if any) the copied schedule should point to; so it just retains it's old address 0xABC. Then when we try to create the blueprint string, it tries to find an entity that is not contained within blueprint, which results in the error we see. Ideally, this "old-to-new" mapping should be preserved for future operations, but there's no guarantee that we actually need this information for future operations; and where would it be stored? What would it's lifespan be? And what if we tried to do something like this?

blueprint.entities = rail_group.entities
blueprint.schedules = some_other_group.schedules

A user might very well want to copy the train routes from a different blueprint (assuming the stop names and conditions are the same) but obviously the train Associations themselves would be completely nonsensical. One could get around it by writing something like blueprint.schedules["locomotives"] = [Association(blueprint.entities["some locomotive"])] (manually fixing the Associations yourself), but there's no indication that this must be done and the error that is generated if you don't is cryptic in regards to the actual problem at hand. In addition, the "correct" way of doing it is almost identical to the incorrect way of doing it, which sucks from an user-experience standpoint.

The more I look at it, it seems to me that this is another example of the API being "too flexible". These kinds of operations necessarily have a lot of memory management and bookkeeping that goes on behind the scenes which is easy to mess up if you dont know exactly what's happening, which seems excessive to have to make the end user understand. I'm tempted to come up with a wrapper function that performs all these operations for you in the correct way: something like blueprint.copy_data_from(group) which would take care of all this extra junk, of course at the cost of user flexibility. What do you think?

Perhaps we could have something like:

blueprint.copy_data_from(blueprintable_or_group) # copy all data, entities, schedules, labels, descriptions, etc.
blueprint.copy_entities_from(blueprintable_or_group) # only copy entities, preserving their associations
blueprint.copy_tiles_from(blueprintable) # only copy tiles
blueprint.copy_schedules_from(blueprintable_or_group) # only copy the schedules, and wipe the locomotives clean...
blueprint.schedules[0].add_locomotive(blueprint.entities["some train"]) # so the user can specify them correctly

That, alongside making blueprint.entities and the like read-only to prevent manual assignment of the lists themselves might make for a better (and clearer!) interface.

[elswindle]

My opinion would be to only copy schedules from a blueprint to a group when passing the string as an argument. If a user is only giving entities, there must be a reason. That being said, the "default" way of importing a blueprint into a group should be clearly stated to be with a string. That way new users will be less likely to run into the issue of schedules not being carried over to the group. An alternative would be to allow schedules to be passed alongside entities when creating a new group and so the copy function can be performed together.

For the transition from group into a blueprint, I can't imagine why any one would manually set a blueprint's entities and schedules from the same group, like you showed in the example above. If you are adding a group into a blueprint, appending the entire group to the blueprint's entities just makes so much more sense. If any new schedule is added from any source, the user must be the one to assign the new locomotives to it. It really isn't difficult at all to do so. Took me 30 minutes or so to create a workaround for my project by extracting schedules and assigning any locomotives inside a group added to a blueprint to the right one.

So I don't think those wrapper functions are really necessary except as maybe an optional method. The capabilities and limitations regarding schedules just needs to be clearly stated within the documentation. If one wishes to keep schedules attached to a set of entities, any operation to copy or add them must be performed together. Anything else will be on the user to do. There really just isn't any one good way of handling this, there are too many possibilities.

So regarding the API, simple functions that would enable the user to perform these kind of tasks would be most beneficial.

• add_locomotive(schedule, locomotive entity)
• create_schedule(**something)
• find_trains_filtered(train_length, num_locomotives, num_wagons, wagon_type, orientation, etc)
  • Helping the user find configurations of trains (2-4, 1-1, etc) would be helpful
• create_train(length, num_locomotives, location, station, schedule)
  • This would've been nice for my case especially if you could place it such that it is behind a specified station. Either it be an entity or by station name, iterating on stations of the same name until there is space to place it. It's hard to know where to place a locomotive and a wagon such that they will be attached to each other.

This is bleeding into a bit different of a topic, but I think that you get the idea. Hopefully my opinion regarding schedules is clear enough, I revised my opinion a couple times while writing this so it may be a bit disjointed.

On a similar note, while creating my workaround, I noticed that the blueprint to_string function clobbers the locomotive associations within a schedule and overwrites them with the entity number of the locomotive. This is easily reproducible, just try to create a string from a blueprint twice in a row. You'll get an error the second time.