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
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
- 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:


  Message sent via/by Gna!

Freeciv-dev mailing list

Greetings from Imhotep

Freeciv-dev mailing list

Reply via email to