Follow-up Comment #1, bug #13621 (project freeciv):
Some basic things I noticed looking at the
patch:
You can just use the simpler
for (op = settings; op->name; op++)
instead of 'do {} while' to iterate over
settings. (Maybe there should be a settings
_iterate macro.)
You probably do not need to send info to
clients (if that is what you mean by "update"
in load_ruleset_game_set()) since there will
be a send_game_info() some time after. But
check this please.
You do not need the 'bool check' argument
to ruleset_set_cmd().
You do not need the '{} /* no error */' stuff.
Instead of looking up the value twice in
order to check that it exists, just pass in
the op's current value the first time. So:
val_bool = secfile_lookup_bool_default(pfile, *op->bool_value, "game.%s",
op->name);
if (val_bool == *op->bool_value) {
...
and similarly for the other setting types
(of course strcmp for strings).
Do not declare and initialize variables over
case labels in a switch statement. Declare
them all in the enclosing scope of the switch
statement, or right after the starting brace of
the switch (before cases), or make a new scope
in each case where you need new variables.
Do not add header includes to other headers
unless there really is no way to avoid it.
In this case please just include fc_types.h in
the c file, not in ruleset.h.
----------------------------------------------------------------
厳しいだが、不公平ではない。
_______________________________________________________
Reply to this item at:
<http://gna.org/bugs/?13621>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
