boekabart wrote:

Coding style: all operators (/ * : ?) should have spaces around them.

I see a lot of code duplication (DAY_TICKS/_patches.daylength_multiplier) (DAY_TICKS*_patches.daylength_multiplier) and (_patches.daylength_correction?_patches.daylength_multiplier:1), I'd suggest making these into (inline) functions. (see diff)

Also, but that can be me, I don't really understand the 'meaning' of DAY_TICKS/multiplier .. what is the 'meaning' of this corrected day_ticks number? The * one is obvious: this is the Ticks-Per-Day() number. But the other one...? Some comment on this, somewhere, would be in place (Or we should give the inline function a good name that explains it).

Attachments

• daypatch_v1_bb_r10263.diff (21.70 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment1411

Chris wrote:

Fixes/Changes:
- AI now correctly starts a few months after the player as set in difficulty
- AI Player # 2 now doesn't start only a few days after the first but as intended
- AI idle time and build time is now properly calculated again
- Removed unneccesary code in industry_gui.cpp
- Removed daylength_multiplier division (see note below)
- Made inline function EconomyMultiplier() ///< Daylength Correction
- Made inline function DaylengthMultiplier() ///< DAY_TICKS * daylength_multiplier
- PERIODIC_RESORT_DAYS will now be correctly multiplied with DAY_TICKS so with longer days
it happens more often in days since they are slower

Note:
I had divided DAY_TICKS by the daylength_multiplier in the network code and AI code because of wrong thinking.
The set value of DAY_TICKS (which is 74) is not directly changed by the patch and therefore checking every 74 Ticks
if a client has lag is totally fine. If we divide DAY_TICKS by daylength_multiplier we check more frequently
for lag which is not necessary obviously and might even cause problems with very slow/bad connections.

EconomyMultiplier():
This inline function (set through the bool switch Daylength Correction) makes sure that the cost / yr is displayed
correctly on all vehicle types in the gui and vehicle info. If you use a daylength multiplier of 1 you can leave it off
since the default cost / yr value stored in GRFs is the one used for a multiplier of 1.

DaylengthMultiplier():
This inline function (can be set in economy settings from 1 to 30) slows down the game by the factor set in the setting.
1 is equal to the defaul of 2,22 seconds per day, whereas 2 equals 4,44 seconds, etc.

Be aware of the following effects:
Industry production is displayed as "per month". Therefore an increased daylength thus increases industry production.
This is not a bug but just reflects the higher production because of a "longer" month.
You will be able to build very long tracks on large maps with a higher daylength setting without having trains travelling
for more than a year simply due to the fact that the year takes long but the train doesn't travel slower. This can "fix"
the minor income issue in the performance measurement when you build very very long tracks.

Attachments

• daypatch_v11_r10267.diff (15.19 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment1416

Chris wrote:

Added the latest version for recent trunk with minimal changes and a fix for high daylength values :) Still opting this for trunk ;) hehe.

Attachments

• r10499-daypatch.diff (15.09 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment1589

TrueBrain wrote:

Is it possible to remove DAY_TICKS totally? As otherwise other patch creates might still use DAY_TICKS as a counter, and therefor many bad things happen :)

Also, DaylengthMultiplier itself isn't really a multiplier. It might be nice to correct the function name to reflect what it is. Some comments around that would be nice too.

You might want to think about the english.txt text. The 2,22 sec part isn't really that nice. Maybe you can find a way to correct that?

Please review those requests, then we will review your patch again :) Nice work anyway!

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment1676

Chris wrote:

Thanks for the comments, I will try to update the code asap. Also the current .diff is quite old already.

The reason I didn't remove DAY_TICKS totally is that some things (like auto resort lists) do not really depend on the length of the days but should rather be executed every xx seconds/minutes. So maybe a solution would be renaming DAY_TICKS to some SCHEDULED_TICKS which is then set to a value close to 74 x 2,22 sec and the DaylengthMultiplier can work on its own. I'll work on this for the next build :)

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment1690

Chris wrote:

Ok, here's the latest result of the daypatch.

The inline functions were renamed:
Daylength() is now simply the length of a day as you might guess ;)
and
VehRunCostFactor() is the factor used to multiply vehicle running costs when daylength affects vehicle running costs is enabled
Also the names of the patch in the economy patch screen were changed slightly.

I was thinking about removing DAY_TICKS totally and tried several possibilities but with bad results. What I did now is renaming DAY_TICKS to DAY_BASE because the 74, which it is defined as, is used in many places as some kind of base value (for example resort timers, sync checks in network games) which only happen to occur on a daily basis because in the original game the length of a day is equal to DAY_TICKS.
Therefor we have DAY_BASE now which is the original base value used in various places where it doesn't make sense to do something daily (e.g. sync checks every minute on long daylenghts would be bad ;-)) and Daylength() which is an actual game day. So whenever another patch wants to do something on a daily basis it can use Daylength() and if it just needs some base value for executing (like resort timers) it can use DAY_BASE.

You'll see the new version is not really a rewrite but mostly renaming stuff and sorting things to make the code more clean and easier to understand. I don't know where else to improve so I am open to any suggestions if the patch still needs improvement :)

Attachments

• r10817-daypatch.diff (24.63 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment1825

Chris wrote:

Ok here's another attempt of a trunk ready version of daypatch :) I really feel good with the current version of the patch. The whole idea of DAY_BASE from the previous version was withdrawn.

The current version adds a function DayLength() to date.h and VehRunCostFactor() to vehicle.h and makes intelligent use of the recently added functions GetRunningCost() and GetDisplayRunningCost() highly reducing the amount of code for the "daylengths affects vehicle running costs" part of the patch. Also the patch code itself has been cleaned and tightened making it change as few code as necessary.
Two impacts this patch has on current game mechanics is a change in players.cpp so competitors start at the proper time set in the difficulty settings and it changes timetables so timetabling in days can only be enabled with daylength set to 1. This avoids the rounding part of the timetables for very long daylengths which would result in a very innacurate timetable.

Thread in OpenTTD forum for further discussion of the patch:
http://www.tt-forums.net/viewtopic.php?f=33&t=31657

Attachments

• r11039-daypatch.diff (12.25 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/908#comment2045

Rubidium closed the ticket.

Reason for closing: Duplicate

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