Follow-up Comment #12, patch #4830 (project freeciv):
On the global variables: I agree they are needed for cleanup purposes, but I
still think they should be scoped to multipliers.c. Consider adding helper
functions to game_ruleset_init() and game_ruleset_free() that operate on
static variables in multipliers.c: functions in multipliers.c will be able to
see these values, but they will not be directly accessible from other code.
For example, the achievements code stores a static array locally scoped to
achievements.c, which is initialised and cleared with functions called from
game.c. Doing it this way means they are automatically processed however the
ruleset is loaded, so you don't need to maintain separate initialisations in
client_main and srv_main (and are no longer missing one in ruledit, civmanual,
or any other ruleset-consuming tools that get added in the future).
r25288 bumped the packet ID count, so your packets need renumbering, but
that's very minor (and can be fixed on commit, rather than being an endless
chase). Similarly, the network capstring in fc_version will need to be
updated with the application of this patch (this, again, can be added on
commit).
I still worry about the non-static stuff in GTK, but don't know enough to be
certain.
In helpdlg_g.h, you have "Policiess", when I think you want "Policies".
In handle_ruleset_multiplier(), you have "Too many multipliers sended by
server", when I think you want "Too many multipliers sent by the server".
In experimental/multipliers.ruleset, you have "Does nothink", when I think you
want "Does nothing".
In helpdata.txt, you have "...which defined your civilization rights", when I
think you want "...which define your civilization's rights.".
Also in helpdata.txt, consider using a _() macro so that the "text" string is
translated.
As may be indicated by the increasingly minor and detailed nature of my
feedback, I think this is almost ready for inclusion.
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?4830>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
