So I did some cleanup but have some questions (see comments below)
Marko Lindqvist wrote:
Update of bug #20577 (project freeciv):
Category: None => general
_______________________________________________________
Follow-up Comment #1:
While it still needs some work, it looks promising first version.
- In the future, please submit new features as "patch" and not "bug"
don't know how this happened, I did want to open a patch ... perhaps my
instinct suspected it would be more of a bug ;-)
- Debug stuff currently just commented out needs to be removed
done, but might have missed something
- Most log_normal() calls should be made log_debug() if not removed
completely
done, with one exception, changed to log_verbose instead of log_debug
- I don't point out all the individual style issues from this version, please
check it against doc/CodingStyle and once you submit updated version I can
point out the remaining ones
I have tried, and done some, but there sure are issues left.
I do indentation automatically via emacs, c-style "gnu". I changed this
to "k&r".
To check, I ran it on a copy of aiferry.c and compared with the original
version.
No success.
I ran indent -kr -i2 -l77 on aiferry.c and compared with the original
version ... it looked completely different.
CodingStyle says "Declaring variables in the middle of the function body
is forbidden"
which is fairly reasonable because you don't want to look for variable
declaration in every odd places.
But I'm not sure if it is meant to hold also for variables that are used
only inside of blocks.
For example, in plrhand.c:kill_player I have
if (prebelcapital) {
/* try to give the new player a GameLoss unit */
struct unit_type *putype = NULL;
int sucount = strlen(game.server.start_units);
int i;
log_debug("try to give the new player a GameLoss unit");
for (i = 0;i < sucount; i++) {
putype = crole_to_unit_type(game.server.start_units[i], NULL);
log_debug("unit has char %c", game.server.start_units[i]);
if (utype_has_flag(putype, UTYF_GAMELOSS)) {
create_unit_full(prebelplayer, city_tile(prebelcapital),
putype, 0, 0, putype->move_rate,
putype->hp, NULL);
}
}
}
If this rule really is meant to hold literally for such cases, too, then
I would prefer to put these blocks into separate functions.
Another style question:
I have put some snippets inside of braces, so it would be easier to move
them if necessary.
I didn't find anything in the CodingStyle to ban this, but then again,
some things might be so horrible
that most sane people wouldn't even be able to imagine.
- You should define default value of its own to gameloss_style, not to use
RS_DEFAULT_BASE_POLLUTION
done
- gameloss_style should be documented in ruleset comments
In every game.ruleset or is there some global document I am not aware of?
_______________________________________________________
Reply to this item at:
<http://gna.org/bugs/?20577>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Greetings from Imhotep
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
