PhilSophus wrote:

The attached hg export (relative to 4df9e8656293 / SVN 14215) of two patches of my realistic timetables series (http://www.tt-forums.net/viewtopic.php?t=39276) implements such a feature. It modifies behavior of autofill as follows:

1. Autofill is started with the first order as before, but does not end when it encounters a filled order, but always does the full round (or until it is manually disabled of course).
2. Clicking "Autofill" clears the the timetable and except for 1) behaves as before.
3. -clicking "Autofill" does not clear the timetable when starting autofill. Travel times are always replaced by the newly determined value, waiting times only if the new value is larger than the timetabled value.

Attachments

• non-destructive-tt-autofill.hg.patch (5.34 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4669

Rubidium wrote:

This doesn't reduce waiting times at stations (for loading/unloading) if that is possible, something the destructive version does reduce.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4953

PhilSophus wrote:

That was intended, as I was assuming that the waiting times are manually tuned and don't reduce drastically when buying new (read faster) vehicles.

However, I have another small patch that allows to set a minimum waiting time for auto-fill (by selecting an order with a waiting time set before starting auto-fill). That could make part of the above argument invalid.

So, if you prefer that, I could make the waiting times behave as the travel times in non-destructive-tt-autofill and leave the rest for a second patch (see the attached patch for this variant).

Attachments

• non-destructive-tt-autofill-wait-time-he17a18bb827f.patch (5.42 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4955

Rubidium wrote:

Ah, didn't think of that use of timetables ;) On the other hand it happens that newer trains have considerably shorter (un)loading times, so it might be useful to make a setting to tell whether to reset the waiting times too or not and make the default for that setting to reset waiting times too.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4959

PhilSophus wrote:

So, a combination of both variants.

Hmm, that should be a setting each player should be able to set differently. So, a client-only setting.

On the other hand, the core must know, what it has to do, so yet another bit ;-) to be passed to the auto-fill command handler in p2 (fortunately, we have enough of them). Then it needs to be stored somewhere during auto-fill, so maybe another vehicle flag (a second flag for auto-fill seems wasteful, but on the other hand, still two bits left after that).

Any thoughts on that?

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4960

Rubidium wrote:

I see no problems in adding another vehicle flag, and yes a _settings_client.gui.something seems to be the best choice.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4962

PhilSophus wrote:

And here it is, non-destructive autofill with client-side setting as ordered, Sir ;-)

The setting is in the vehicle tab (for the next one we need to resize it). Changing it only effects autofills started after the change (I see that as a feature rather than a bug, as it allows you to have different autofills running at the same time).

I didn't bump savegame format as technically it isn't changed and the additional vehicle flag bit should just be ignored on versions before this patch.

Base revision is he336f1784ba4 = SVN r14541

Attachments

• non-destructive-tt-autofill-he336f1784ba4.patch (9.99 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4964

Rubidium wrote:

Sorry to keep bothering you... but... is the destructive autofill still necessary with this patch? A non-destructive autofill with resetting of waiting times is basically the same as a destructive autofill without the zero-ing of all entries.
However... I'm not sure whether other people are actually using the destructive one for something useful, and not having a patch setting to influence what gets destructed and not should give less confused users I think.

Too bad that one only realises these things when one actually tests the patch :(.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4968

PhilSophus wrote:

However... I'm not sure whether other people are actually using the destructive one for something useful, and not having a patch setting to influence what gets destructed and not should give less confused users I think.

Sorry, you lost me in this sentence. Do you mean that, we should drop the patch setting, drop destructive autofill and instead use to select between "overwrite waiting times" and "take current waiting times as minimum"?

Off-head I could imagine the following application of destructive mode (not a very important one, though): You can easily see, which entries are already (re-)filled and which not. This is nice to have when you have forgotten which vehicle of the route you started the autofill on and want to adjust some times manually or want to know if the autofill has finished.

Marking vehicles with autofill enabled in the shared vehicle list could help, though, (I already got that request for my ITiM headway window), but that would definitely be another patch.

It would be even better if one could see which orders are already filled. However, it would be easier to implement such stuff as properties of the order list rather than of vehicles (actually the same is valid for autofill in general). Actually, I do have a patch which refactors the order list code, so that there is a common instance of an OrderList class which maintains the linked order list and linked list of shared vehicles. Actually, thanks to a code review by Alberth and thanks to being that a part of ITiM for some time without problems, I consider it having trunk quality. I'm just not sure that you or another dev want to review a 50k patch (actually it is quite readable and split into 10 sub patches of tiny to moderate size). Shall I open a task for this as it leads us quite away from this issue here?

BTW: Since non-destructive autofill is part of ITiM since end of August, it could be useful to ask there for opinions on whether destructive autofill can be dropped: http://www.tt-forums.net/viewtopic.php?t=39276

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4970

PhilSophus wrote:

Just for reference: I put the order list refactoring in #2389

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4971

PhilSophus wrote:

Sorry for the delay (some RL business).

The attached patch removed the advanced setting again and behaves as follows:
- The timetable is never cleared when starting autofill (i.e. autofill is always non-destructive)
- -click on autofill causes the waiting time to be preserved unless the real waiting time is larger

BTW, what is the normal proceeding for other languages if a string (like the autofill tooltip here) changes semantically? Removing it from all other language files until it is retranslated?

Mercurial patch against h44d201d223f9 (SVN 14565)

Attachments

• non-destructive-tt-autofill-h44d201d223f9.patch (6.36 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment4996

Rubidium wrote:

We normally remove the changed strings in a separate commit.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124#comment5037

Rubidium closed the ticket.

Reason for closing: Implemented

In r14592.

This comment was imported from FlySpray: https://bugs.openttd.org/task/1124