Code review comment for lp:~widelands-dev/widelands/bug1504366

Revision history for this message
GunChleoc (gunchleoc) wrote :

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.

review: Approve

« Back to merge proposal