Follow-up Comment #7, patch #6104 (project freeciv):
> Someone must do a careful review.
Here is what I was able to find.
> + bool ok;
Readability: Could you come up with a more descriptive variable name? (I know
it is hard to do when the brain is in programming mode) Suggestion: escaped
> + if (fc_rand(2) == 1
> + && vunit->moves_left > pkiller->moves_left) {
Nitpick: if you check for moves left before rolling the dice you can save a
dice roll.
> + if (!is_native_tile(unit_type(vunit), ptile)) {
You test for native tile in stead of livable tile (can_exist_at_tile). Could a
Trireme end up escaping to unsafe terrain?
> + if (move_cost + pkiller->moves_left > vunit->moves_left) {
Readability: Why add the victim's move cost to the killer's moves in stead of
subtracting them from the victim's moves?
There is also white space at the end of some lines, including blank lines.
Perhaps you could change your editor settings so white space at the end of a
line you modify is removed or high lighted?
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?6104>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
[email protected]
https://mail.gna.org/listinfo/freeciv-dev
