Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reverse at signal timeouts occur unexpectedly quickly, affects title game #7159

Closed
JGRennison opened this issue Feb 1, 2019 · 6 comments
Closed
Labels
Milestone

Comments

@JGRennison
Copy link
Contributor

@JGRennison JGRennison commented Feb 1, 2019

Version of OpenTTD

master: from commit e934f09 onwards

Expected result

When _settings_game.pf.reverse_at_signals is true, reversing of trains at a red signal occurs after the interval given by _settings_game.pf.wait_oneway_signal or _settings_game.pf.wait_twoway_signal.

Actual result

When _settings_game.pf.reverse_at_signals is true, reversing of trains at a red signal occurs at a shorter interval than expected.

Steps to reproduce

Watch the title game for a few minutes, diesel trains going over the bridge at the top occasionally reverse on the bridge or in the the signal block prior to the bridge, due to moderate delays which are less than that given by _settings_game.pf.wait_oneway_signal. This did not occur prior to commit e934f09 and is probably undesirable in the title game.

@PeterN PeterN added the regression label Feb 1, 2019
@PeterN

This comment has been minimized.

Copy link
Member

@PeterN PeterN commented Feb 1, 2019

Seems to ultimately be caused by separate things using the same counter?

@J0anJosep

This comment has been minimized.

Copy link
Contributor

@J0anJosep J0anJosep commented Feb 1, 2019

Is it caused by #7114?

@JGRennison

This comment has been minimized.

Copy link
Contributor Author

@JGRennison JGRennison commented Feb 1, 2019

Seems to ultimately be caused by separate things using the same counter?

Train::wait_counter is only used for one thing in this case.
It looks to me like the change in e934f09 would cause Train::wait_counter to be incremented every tick as the signal is now checked every tick, whereas previously some progress would have to be made between checks. Adjusting the scaling constants in the tests with _settings_game.pf.wait_oneway_signal or _settings_game.pf.wait_twoway_signal ought to be sufficient.

Is it caused by #7114?

No, it is not a pathfinder issue. Reverting e934f09 is sufficient to resolve the issue.

@PeterN

This comment has been minimized.

Copy link
Member

@PeterN PeterN commented Feb 1, 2019

I was referring to the use of progress, not wait_counter.

@LordAro LordAro added this to the 1.9.0 milestone Feb 3, 2019
@michicc

This comment has been minimized.

Copy link
Member

@michicc michicc commented Feb 19, 2019

With the current code, a train waiting at a one-way signal will increment wait_counter twice each tick, i.e. on both TrainLocoHandler calls.

Previously, the update rate of wait_counter depended on train speed and acceleration. For example train 2 from the title game (Turner Turbo) incremented wait_counter every ten ticks, while train 12 (Lev4) incremented every five ticks.

The easiest change would be to the factor in the ++v->wait_counter < _settings_game.pf.wait_oneway_signal * 20 line, to get an approximate and speed independent wait time. Factor 200 would be right for the Lev4, 400 for the Turner, but to avoid integer overflow 255 is the max factor anyway. So maybe just stick with it.

@PeterN

This comment has been minimized.

Copy link
Member

@PeterN PeterN commented Feb 19, 2019

Apart from being twice as fast, this new behaviour also seems to match the setting description better. However I guess the comment was wrong before. Should be possible to at least not increment it on both TrainLocoHandler calls.

michicc added a commit to michicc/OpenTTD that referenced this issue Feb 19, 2019
…o short.

This is not an exact fix as previously, the wait time was speed/acceleration dependant. This simple fix ignores that and just makes it somewhat right.
michicc added a commit to michicc/OpenTTD that referenced this issue Feb 20, 2019
…o short.

This is not an exact fix as previously, the wait time was speed/acceleration dependant. This simple fix ignores that and just makes the 'days' from the settings comment to be actually days.
@LordAro LordAro closed this in 690d1dd Feb 21, 2019
nielsmh added a commit to nielsmh/OpenTTD that referenced this issue Mar 11, 2019
…o short.

This is not an exact fix as previously, the wait time was speed/acceleration dependant. This simple fix ignores that and just makes the 'days' from the settings comment to be actually days.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.