Daylength Patch [12/09/2008] Compatibility: r14293

[chrissicom]

I also like the second option better with just using the DAY_BASE for the timetables. I'll see how it can be done nicely without confusion of the player

[KrevRenko]

IMO, a simple unit rename would be enough, if it is not 'days', it has no relation to the game days. Just can't think of a replacement name.

Looking forward to what you make of it I have a long game planned for when this is sorted out

[Aydan]

you could calculate it down to hours if days less than 2 and internally use ticks as a base

[MagicBuzz]

I assume I'll have to survey this thread, as my separation patch is impacted with timetables

[chrissicom]

I assume I'll have to survey this thread, as my separation patch is impacted with timetables

Well daypatch doesn't really change the timetables except that it replaces the original days with the daylength days. Does your patch autofill with ticks or with days or both?

[MagicBuzz]

my patch does not autofill anything

it just uses what is stored in the timetable.

internally, timetable uses ticks, but rounded to the greater day ticks (when autofill).
ie. 140 ticks ~ 2 days = 148 ticks, and 149 ticks ~ 3 days = 222 ticks

so if you change the way these values are stored and used by the timetable, it may impact my patch (it shouldn't till you don't rewrite the way timetable works)

[PhilSophus]

I have been thinking about introducing a concept of virtual time into OpenTTD as I think timetables should be in hours/minutes not in days or even in an internal measure like ticks. Such a virtual time would essentially just represent a second timescale of the game that can be shown in the status bar and would be used for the timetables.

I thought without modified day length 24 hours would match 1 tropical year (365.2425 days) of the current OpenTTD calendar, so that midnight is around new year and noon around beginning of July (boggles around a bit due to switch years). So 4 minutes virtual time would equal one calendar day, which equal 2.2 seconds real time.

As there is no need to keep this tight relationship between virtual time and OpenTTD calendar, with day length patch one could just define that always 4 virtual minutes are 2.2 seconds real time. So virtual time always runs in the same speed independent of the day length setting.

Let me give an example: You set day length to 4. The year now takes four times longer, but 24 hours virtual time are no longer a year but only 1/4 year, 16 minutes now make a day, midnight is four times a year. So day length would have no influence on timetables.

What do the others think?

[chrissicom]

The idea is good of course and should work on new computers, but the problem is that you can't do things in realtime in OpenTTD because that implies that your computer can handle OpenTTD at full speed. Let's say a game with 1500 trains runs fine on my PC because I have a C2D and a second is a second but then let's open that game on an old Athlon XP where that second on my PC will surely be more time. Of course the game internally doesn't have problems with that, it just doesn't work as fast as it could but it makes it useless for the users to use seconds as a measure of time since he can't possibly know how long that "second" really takes if he has a slow machine. Therefore using a virtual unit like ticks is the right way already I think. Well, maybe it should be made more obvious to users at some point as to what one tick is.

[MagicBuzz]

Well, timetables are already based on ticks.

So DayLength patch virtually doesn't change anything the way the timetable works.

But for eye candy, the author of timetable patch maked the timetable values to be multiples of days. This to allow display and input using "day" unit. If you check the option "display timetable in ticks", you'll just see ticks, nothing else. And you can type anything, this value will be used "like this".

I mean when you set a trip of 500 ticks, then the timetable system will use 500 ticks, not (int(500 / 74) + 1) * 74 = 518.
But when you active "autofill", when the trip takes 500 ticks, it fills the table with 518 to fit an exacte day.

IMHO, we just have to disable this in order to store the exact ticks count in the timetable.

[MagicBuzz]

The idea is good of course and should work on new computers, but the problem is that you can't do things in realtime in OpenTTD because that implies that your computer can handle OpenTTD at full speed. Let's say a game with 1500 trains runs fine on my PC because I have a C2D and a second is a second but then let's open that game on an old Athlon XP where that second on my PC will surely be more time. Of course the game internally doesn't have problems with that, it just doesn't work as fast as it could but it makes it useless for the users to use seconds as a measure of time since he can't possibly know how long that "second" really takes if he has a slow machine. Therefore using a virtual unit like ticks is the right way already I think. Well, maybe it should be made more obvious to users at some point as to what one tick is.

Well, if you use a core 2 duo, then OTTD actually just uses 50% of you CPU, not 100%, as it's not multi-threaded

[chrissicom]

Well it uses ~25% with 1500 trains thus the 74 ticks really are 2,22 seconds, but on a slow computer the 74 ticks are much more time... I try different approaches tho and post an update to the patch as soon as I am happy with the solution.

[PhilSophus]

I think you misunderstood me. I didn't mean to couple the virtual time to real time but just to the ticks (but not to days), so that the virtual time runs independent of the day length setting. The given 2.2 seconds were just the duration of the tick when the computer is fast enough. Of course, if the computer is too slow, the virtual time will be slower too (as the whole game).

[KrevRenko]

Let's just not overcomplicate this.

[MagicBuzz]

Let's just not overcomplicate this.

The "virtual time" already exists, and it's the ticks.

A "normal" day is 74 ticks, while a day with day_length patch set to 10 is 740 ticks.

Because of this, we don't need a new unit, as this one remains inchanged when the day_length setting change.

Luckily, the timetable already uses the ticks to store internal times.

So, as I said, the only problem concern only one line in the whole OTTD code : when a autofilled time computed in ticks is changed to fix a day multiple. It must remains in ticks, and the GUI should be using tick unit only when day_length setting is > 1.

That's a very minor change, and it should just fix the current problem.

[MagicBuzz]

According the r11015 :

timetable_cmd.cpp lines 160 to 164 :

Code: Select all

/* Round the time taken up to the nearest day, as this will avoid
 * confusion for people who are timetabling in days, and can be
 * adjusted later by people who aren't. */
time_taken = (((time_taken - 1) / DAY_TICKS) + 1) * DAY_TICKS;

Must be changed to :

Code: Select all

/* Round the time taken up to the nearest day, as this will avoid
 * confusion for people who are timetabling in days, and can be
 * adjusted later by people who aren't. 
 * Don't round if day_length patch is active */
if (_patches.day_length == 1) {
    time_taken = (((time_taken - 1) / DAY_TICKS) + 1) * DAY_TICKS;
}

And timetable_gui.cpp lines 37 to 46 :

Code: Select all

static inline void SetTimetableParams(int param1, int param2, uint32 time)
{
	if (_patches.timetable_in_ticks) {
		SetDParam(param1, STR_TIMETABLE_TICKS);
		SetDParam(param2, time);
	} else {
		SetDParam(param1, STR_TIMETABLE_DAYS);
		SetDParam(param2, time / DAY_TICKS);
	}
}

Must be changed to :

Code: Select all

static inline void SetTimetableParams(int param1, int param2, uint32 time)
{
	if (_patches.timetable_in_ticks || _patches.day_length > 1) {
		SetDParam(param1, STR_TIMETABLE_TICKS);
		SetDParam(param2, time);
	} else {
		SetDParam(param1, STR_TIMETABLE_DAYS);
		SetDParam(param2, time / DAY_TICKS);
	}
}

PS : remplace the "_patches.day_length" variable name by the correct value.

[chrissicom]

I really like the way you solved that issue we discussed and I have just finished a new version of daypatch which highly reduces the amount of code changed compared to the previous version and I will also submit the update to Flyspray as trunk candidate and hopefully get it included this time as I feel the patch to be really clean and not intrusive in its current state

[PhilSophus]

hopefully get it included this time as I feel the patch to be really clean and not intrusive in its current state

I just looked through the patch and, without having seen the code before, find it quite understandable.

One small issue though:

Code: Select all

--- src/timetable_gui.cpp	(revision 11039)
+++ src/timetable_gui.cpp	(working copy)
@@ -36,7 +36,7 @@
 
 static inline void SetTimetableParams(int param1, int param2, uint32 time)
 {
-	if (_patches.timetable_in_ticks) {
+	if (_patches.timetable_in_ticks || _patches.daylength > 1) {
 		SetDParam(param1, STR_TIMETABLE_TICKS);
 		SetDParam(param2, time);
 	} else {

If I understand this correctly you implicitly switch the display of timetables to ticks. Although I find it okay, that timetables are shown in ticks with the patch, I think this should be done explicitly by enforcing that the timetable_in_ticks setting is enabled. To my knowledge this can be done by providing a callback for setting the option. This callback could set _patches.timetable_in_ticks if needed and show message box in this case, so that the user knows what is going on.

You may have a look at the last column of _patch_settings in settings.cpp for examples how the callbacks are used.

Nevertheless, you did a great work!

[chrissicom]

Hmm might be a good idea for a more advanced version. So when changing daylength from 1 to something else it changes timetables to ticks if necessary and tells the user and after switching back to 1 it changes the timetable back to days if it was days before. This would imply tho that the daypatch remembers the setting of timetables from before daypatch was changed away from 1 so it might be a little too much off code overload. Maybe just showing a user warning that daylength > 1 and timetable in days can't be used together when loading a game is sufficient if at all required.

[PhilSophus]

So when changing daylength from 1 to something else it changes timetables to ticks if necessary and tells the user and after switching back to 1 it changes the timetable back to days if it was days before.

I think, this is overkill. Just change timetable_in_ticks to 1 if it isn't and notify the user in this case. And probably invalidate the settings widget of timetable_in_ticks so that the GUI also represents this change. Or just refuse to enable daylength patch if timetable_in_ticks is not enabled.

IMHO, in any case the timetable_in_ticks setting and behavior should be consistent. Showing timetables in ticks although the setting is "show in days" (even if it's documented, but who reads the manual after all ) just calls for unnecessary bug reports.

I could imagine that subtly changing behavior of other patch settings (as it does now) could be an issue to block the patch from trunk inclusion.

[KrevRenko]

Well that's what I meant when I said that having a day-like base unit that would be equal to a day would not break anything. It's just that the 74 ticks in a timetable will not be called days, but for example base-days (or bds)that way you would not have to adjust other patches and play according to their settings. I think the optimal solution is leaving the rounding to 74 ticks is OK as it is a simple human-perceptible time quantum to show a difference between two trains moving but not too long.