I will take into account your comments and rework all the setting
configurations. As I see it, there should be the following functions (in
settings.c?)
bool settings_check(int cmd, union bisval, char *msgstr, int *msglen)
bool settings_set(int cmd, union bisval, char *msgstr, int *msglen)
bool settings_send(int cmd)
- int cmd:
number of the command in the settings struct
- union bisval:
bool/int/string value for the option
- char *msgstr:
return message by the function (error/success; only displayed if length > 0)
- int *msglen
length of msgstr
should there also be a function
struct settings *op = settings_get(int cmd)?
Some questions:
Am Thursday 18 June 2009 03:45:29 schrieb Madeline Book:
> - Sometimes you cmd_reply (= notify_conn) in the
> set_command_*() function, and sometimes you
> notify_conn in its caller; in other words, what
> is the point of passing 'pastr' if you just
> cmd_reply inside it anyway.
that is the difference between cmd_reply and notify_conn? From the comments to
cmd_reply I take that it does not duplicates output on the console? Should
this be used for calls from the command line?
Should this function(s) be called every time something should be printed or at
the end one time to print all text (especially for the rulesets)?
> - You replace the fast pointer checks with slow
> string comparisons (strcmp(op->name,...)) for
> no reason (yes we can debate which is better
> but the fact is that both depend on some hard-
> coded name, be it either the string name or
> the variable name, so you do not gain anything
> by changing it).
So the following line tests, if it points to the same place in memory? At
first I thought that it will be true, if the option value is equal to the
value of aifill (so if aifill=5 and mrg_distance = 5 it would be called two
times). If it is not the case, I will use this faster version!
if (op->int_value == &game.info.aifill) {
> My suggestion is to factor out into separate
> functions [by "factor out" I mean take the duplicated
> code and put it in one place so there is only one
> copy, not make 3 copies; cf. 2x + xy + 3xz =
> x(2 + y + 3z)]:
> - setting value checking with bool return indicating
> success and error message buffer for reason:
> bool setting_check(const struct setting *op,
> int ibval, const char *sval, char *err, int errlen)
first arg int cmd or struct setting *op?
> Of course if you start to use the above approach
> and it turns out to be worse than what you already
> have, feel free to ignore my suggestions. :)
After I read your comments I thing there is place for improvement ... ;-)
>
>
> Really I would have been happy to commit the last
> version, since if you dig much deeper into this
> code you will probably end up making a substantial
> rewrite. It's up to you if you want to continue
> with this less than exciting stuff or just leave
> it for later...
You can commit it but I will check (and perhaps rewrite) the settings system
and as this is a new feature, it can wait.
I will need some time for the next version of the patch ... first learning a
little bit git and there is also real life ...
--
Matthias Pfafferodt - http://www.mapfa.de
Matthias.Pfafferodt <at> mapfa.de
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
