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

Timetable: auto-reset delay counter #2944

Closed
DorpsGek opened this issue Jun 1, 2009 · 6 comments
Closed

Timetable: auto-reset delay counter #2944

DorpsGek opened this issue Jun 1, 2009 · 6 comments
Labels
enhancement Issue would be a good enhancement; we accept Pull Requests! good first issue Good for newcomers

Comments

@DorpsGek
Copy link
Member

DorpsGek commented Jun 1, 2009

kvtb opened the ticket and wrote:

if time tables are enabled, and if there is a valid time table set for a certain orderlist (where for each order a duration is known), I would like to have to possibility to ensure that if there is a delay, the delay does not 'explode'.

let's assume the total time table (the cycle time) is 80 days
then the delay of a vehicle in this schedule, can never be larger than 80 days, because as soon as the delay is 80 days, it is back in its original time table cyle.

in fact, what I try to explain, is really easy using some pseudocode :-)

if delay >= timetable cycle time then
delay := 0 // reset delay counter

Reported version: trunk
Operating system: All


This issue was imported from FlySpray: https://bugs.openttd.org/task/2944
@DorpsGek
Copy link
Member Author

DorpsGek commented Jun 20, 2009

kvtb wrote:

I just checked the source to see if I could do this myself in 5 minutes

the total time table duration (in the first message referred to as timetable cycle time) is not stored in some structure, it is calculated when the timetable dialog is opened.
it is the total_time variable on line 220 in timetable_gui.cpp

a prerequesit for this is that total_time is moved to OrderList.
Once the vehicle's order list has the total timetable time field, it is possible to simple add a few lines to UpdateVehicleTimetable in timetable_cmd.cpp

basically, something like this:

 /* Reset vehicle delay if is becomes larger than the duration of the timetable,
  \* because we are back in the normal timetable loop.
  \*/
 if( v->lateness_counter >= v->orders.list->timetable_duration ) {
     v->lateness_counter = 0;
 }

This comment was imported from FlySpray: https://bugs.openttd.org/task/2944#comment6255

@DorpsGek
Copy link
Member Author

frosch wrote:

You should better use a modulo operator "%" to keep multiple vehicles in sync. But be careful with negative lateness.

Good luck implementing your patch though :)


This comment was imported from FlySpray: https://bugs.openttd.org/task/2944#comment6256

@DorpsGek
Copy link
Member Author

kvtb wrote:

okay, yesterday I did not look correctly, the contents of timetable_gui.cpp looked as if there was no total time table duration stored somewhere.
Fortunately, there was already a function GetTimetableTotalDuration somewhere, that I could use.

Using the module operator as suggested by frosch, the code to be added is simply this:

/* Reset vehicle delay if it becomes larger than the duration of the timetable,

  • because then we are back in the normal timetable loop. */
    v->lateness_counter %= v->orders.list->GetTimetableTotalDuration();

this code has no influence on negative delays

Please apply the attached patch to trunk, thank you in advance!

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/2944#comment6257

@DorpsGek
Copy link
Member Author

kvtb wrote:

if the timetable was not complete, the delay counter would always be reset.
I think that's not needed, so this updated patch uses GetTimetableDurationIncomplete instead of GetTimetableTotalDuration

Attachments


This comment was imported from FlySpray: https://bugs.openttd.org/task/2944#comment6258

@DorpsGek DorpsGek added Core flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) labels Apr 7, 2018
@TrueBrain TrueBrain added good first issue Good for newcomers patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay enhancement Issue would be a good enhancement; we accept Pull Requests! and removed enhancement from FlySpray labels Apr 12, 2018
@frosch123 frosch123 removed the Core label Apr 14, 2018
@andythenorth andythenorth added the stale Stale issues label Jan 5, 2019
@andythenorth
Copy link
Contributor

Thanks for this. There's been no activity on this for some time, and as it stands, it doesn't look likely that it will go any further. Since OpenTTD moved to GitHub, we use pull requests rather than patches, as they are a much more productive workflow.

I'm planning to close this soon (in 7 days), as we try to keep the issue count low for OpenTTD, it helps us focus on things that are important and fun.

If you would like to continue with this patch, the best way would be to move the patch to your own GitHub fork, update it for the current OpenTTD master, and then create a pull request. For more information, please see our CONTRIBUTING.md.

We are also happy to discuss directly on the issue, or in #openttd irc, including help to get this into a pull request. Thanks for your contribution!

@andythenorth
Copy link
Contributor

Closing, thanks.

@andythenorth andythenorth removed flyspray This issue is imported from FlySpray (https://bugs.openttd.org/) patch from FlySpray This issue is in fact a Patch, but imported from FlySrpay stale Stale issues labels Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue would be a good enhancement; we accept Pull Requests! good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants