[r8797] initialization of convoi_t::recalc_speed_limit

[THLeaderH]

I noticed that convoi_t::recalc_speed_limit is not initialized when a convoi_t object is created. In general, not initializing variables can cause unexpected behaviors and network game de-sync.

The solution is inserting the following code into L166 and L2002 of simconvoi.cc.

Code Select Expand


recalc_speed_limit = true;


Thank you for your attention.

[DrSuperGood]

I noticed that convoi_t::recalc_speed_limit is not initialized when a convoi_t object is created. In general, not initializing variables can cause unexpected behaviors and network game de-sync.

I am a strong supporter of state initialization. However in theory it only is an issue if the variable is used before being set. If it never is used before being set then in theory there is no need to set it during object creation.

However one might as well set it for better code quality.

[Ters]

This isn't technically about uninitialized variables. Member variables in C++ are always default initialized if no explicit initialization takes place in the constructors initializer list. This is about a member variable that isn't (re)set along with a bunch of others in certain conditions.

Looking through the code, recalc_speed_limit looks like a way to signal that part of the state is stale, rather than proper state itself. It is not saved or sent over the network in any way. Setting recalc_data to true signals exactly the same state recalculation, so setting both to true is superfluous. As far as I can see, setting either one to true has exactly the same effect. One might just as well merge them.

[prissi]

recalc_data also calculates the weight, which iterates over all ware_t it has loaded. So there is a reason to have both. But initialising cannot hurt.

[Ters]

The searches I did didn't reveal any if (recalc_data). All I found was if (recalc_data || recalc_speed_limit).

But initialising cannot hurt.

It was always initialized.

[THLeaderH]

I posted this because Undefined Behavior Sanitizer (UBSan) reported me that recalc_speed_limit was read and it had illegal value as a bool variable. Anyway, thank you for incorporating.

[prissi]

The recalculation of the weight is only done, when recalc_data is set. Two line below the above mentioned if

[Ters]

Just for the record: I got my languages messed up apparently. C++ does not appear to initialize primitive member variables, just whatever the other kind is. Java initializes both primitives and its other kind.