Alberth wrote:

Just a few code style comments and other things I noticed, I have no idea about how good or bad this patch is in other aspects.
I just pick random code fragments I encounter, most comments apply at many places in the code.

+ if( _settings_game.economy.day_length_balance_type == DBT_ALL_COSTS )
+ res.AffectCost( _settings_game.economy.day_length_balance_factor );
Coding style errors.
no space after if
an additional space inside the outer parentheses
statement after the if is not allowed in this way. either at the same line, or use curly brackets

+ if( (((_date_fract % DAY_TICKS) == 0) && ( !(_settings_game.economy.slow_down_industry_changes) )) ||
+ ( _date_fract == 0 ) )
+ {
+ IndustryDailyLoop();
+ }
Coding style errors.
no space after if
an additional space inside the outer parentheses
unneeded parentheses all over the place
open curly bracket at same line as the if, not below it

+* Returns a number of days in current month.
+*/
+static Day DaysInCurrentMonth()
Also document the returned value with @return

+* Return a day and tick, where new month on day length 1 would start.
+*/
+static void NewMonthAt( Day cur_day, DayTick *dt )
"return" ?
formal parameters must also be documented with @param

+# define DAYS_IN_YEAR 365 ///< days per year
+# define DAYS_IN_LEAP_YEAR 366 ///< sometimes, you need one day more...
don't change enums back to # define

+STR_CONFIG_PATCHES_DAY_LENGTH_FACTOR :{LTBLUE}Day length factor: {ORANGE}{STRING}
+STR_CONFIG_PATCHES_DAY_LENGTH_BALANCE :{LTBLUE}Balancing of day length: {ORANGE}{STRING}
nearby are CONFIG_SETTINGS which seems a better name than CONFIG_PATCHES to me

+ UpdateGraphPerformance();
in a list if InitializeXXXX calls. looks fishy. perhaps rename it?

+static bool UpdateAllCosts(int32 p1)
static functions must also be documented

This comment was imported from FlySpray: https://bugs.openttd.org/task/3157#comment6557

pavel1269 wrote:

Updated coding style.

Attachments

• day_length_r17304.patch (41.21 KiB)

This comment was imported from FlySpray: https://bugs.openttd.org/task/3157#comment6559

andythenorth closed the ticket.

Reason for closing: Won't implement

Mass closure of patch tickets with no commentary for >5 years. Goal is to reduce patch queue as an experiment to see if it aids faster reviewing and rejection/acceptance (it may not). If this offends you and the patch is maintained and compiles with current trunk, discuss with andythenorth in irc. (andythenorth has no ability to review patches but can re-open tickets).

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