Change: [YAPF] Penalty for crossing a reserved segment is now constant

[mrmbernardi]

Motivation / Problem

This issue is related to PR #10804 and issues #10467, #10840, and #9609.

Currently the cost associated with driving over reserved rails is applied once per tile. For typical train lengths this doesn't make much sense as you can have large differences in costs between a 1 tile train and a 5 tile train even though these trains may take very similar amounts of time to clear the way. It results in the weird behavior with short trains depicted in the issues mentioned above.

Description

I have changed the reservation cost to now only apply once per reserved segment. Consequently, I've also multiplied the related default cost settings by 5 to replicate the behaviour with a typical 5 tile train.

Limitations

In theory, perhaps it does make sense for paths over super long reserved sections to have higher costs because it could take longer for the reserving trains to clear the track (depending on the speed of the trains in question). If it's desired, it wouldn't be too hard to actually count the length of the current reserved segment and have a fixed cost for lengths of say 10 blocks and then have it increasing again after that. But maybe that's overkill.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

• The bug fix is important enough to be backported? (label: 'backport requested')
• This PR touches english.txt or translations? Check the guidelines
• This PR affects the save game format? (label 'savegame upgrade')
• This PR affects the GS/AI API? (label 'needs review: Script API')
  • ai_changelog.hpp, gs_changelog.hpp need updating.
  • The compatibility wrappers (compat_*.nut) need updating.
• This PR affects the NewGRF API? (label 'needs review: NewGRF')
  • newgrf_debug_data.h may need updating.
  • PR must be added to API tracker

[PeterN]

Are you talking about the length of the train doing the pathfinding, or the length of other trains on track?

Isn't it the length of the reservation that is counted, not the length of any trains? There isn't necessarily any correlation between them.

Also changing pathfinder settings like this is basically pointless, anyone who already run OpenTTD before will not get the new values.

[mrmbernardi]

Are you talking about the length of the train doing the pathfinding, or the length of other trains on track?

Isn't it the length of the reservation that is counted, not the length of any trains? There isn't necessarily any correlation between them.

It's the length of other trains in the path. You're right that only the reservations are costed, but each train always reserves the tiles that it's currently occupying so the correlation is that train length sets the minimum reserved segment size. This is most relevant when trains are stopped as then the reserved segment length is exactly the length of the train. For short trains stopped at a signal, the cost is too small as the linked issues demonstrate.

The other argument against this is that it preferences shorter blocks as the reservations will be shorter. Every two tiles reserved is equivalent to 90 degree bend in the path, so the penalty for each reserved tile is quite high. The length of the reservation could be a proxy for how long the train will be in the block, but to do this properly you would need to take into account the speed of the reserving train.

The ideal situation for handling stopped trains would be to cost them proportional to the amount of time that they've already been waiting under the assumption that each waiting train is on average halfway through its wait, so the estimated time it will continue to be stopped is proportional to the time it has already waited. This is possible, but it could cost too much to do vehicle lookups in the pathfinder.

A couple other ways of tackling the linked issues would be:

• Apply a fixed penalty once for the first x reserved tiles, and then a small penalty for each tile after that.
• Apply a fixed penalty once for reserved tiles that the train is currently on and then a different small penalty for each tile it's yet to drive through.

Also changing pathfinder settings like this is basically pointless, anyone who already run OpenTTD before will not get the new values.

Agreed. How is this usually done?

[TrueBrain]

Agreed. How is this usually done?

We have some code for that these days; basically, our configurations are versioned, similar to savegames. Look in the code for IFV_LINKGRAPH_SECONDS to get an idea how to fix this for existing configuration!

[2TallTyler]

Based on our conversation about this today, I am definitely in favor of trying it -- but enough real player feedback to do a proper stress-test may be hard to do without merging and releasing it.

So, could we gather this data without a release? Or do we release and let players opt out of the change? What's the best way to do that? Is it time for YAPF2? Time to explore @TrueBrain's idea to add WASM, so users can write/choose/configure their own custom pathfinder?

Now you see why this hasn't been merged yet. 😛

[mrmbernardi]

We have some code for that these days; basically, our configurations are versioned, similar to savegames. Look in the code for IFV_LINKGRAPH_SECONDS to get an idea how to fix this for existing configuration!

I've added code to work with this, which means this PR could be merged and would handle the different ini file situation. As for how exactly to test the change, I'm not particularly sure. I think making a different pathfinder would be cleanest, but this seems like overkill for a 20 line change to the original YAPF.

[2TallTyler]

I guess you can always add a pathfinder setting (with the new behavior as default) so people can opt-out of the change.

I'd appreciate some other thoughts on how to move forward.