Review: Approve

The assert(nr_players <= 99) isn't a loop boundary, it is there to ensure that 
a condition is met. In debug mode, if the condition isn't met, Widelands will 
terminate with a message. The reason for assertions is to define certain wanted 
behaviour and thus help guard against regression bugs.

In this case, the assertion is there because there is some string assembly 
going on directly below it that relies on the number not being larger than 2 
digits. Your code change is not wrong at the moment, because MAX_PLAYERS is 
smaller than 99, but the 99 is semantically more clear. E.g. imagine we would 
allow more than 99 players some day, then the string assembly code would need 
changing - this is is signalled by the assertion.

Good catch about the button, I have fixed it.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug1504366/+merge/273995
Your team Widelands Developers is subscribed to branch 
lp:~widelands-dev/widelands/bug1504366.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to