<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40480 >
Both the aifill and autotoggle setting were using the setting
value validation callbacks to perform side-effects, contrary
to the expected behaviour of these callbacks in the body
of the set_command function. That is to say, they would make
changes to the server state even if check==TRUE (i.e. when
the set command should only check if the arguments are valid
and not actually perform any actions).
This results in the incorrect behaviour as reported here:
http://forum.freeciv.org/viewtopic.php?p=21448#21448
(The patch in PR#40475 being used to allow voting of the
aifill command.)
Attached patch removes the validate callbacks and moves
their side-effects into the body of the set command where
the conditions for their execution are properly checked.
The drawback is that since a 'setting' has no unique
identifier (unlike commands, e.g. CMD_SET), then a rather
brittle check on the setting value address is made in
order to determine which setting side-effects should be
executed. For example, to know if we are to handle the
aifill side-effects, we check
op->int_value == &game.info.aifill
This has the disadvantage of being a hard-coded reference
to the pointer given in the setting definition, but other-
wise is as fast as a unique identifier check without
requiring a re-design of the setting data structures to
incorporate unique identifiers.
A FIXME comment is left in the relevant code section for
anyone who would care to think of a better work-around or
redesign the setting data structures to remove the need
for the pointer reference.
----------------------------------------------------------------------
思わずに瓶の中身をガブガブ飲んだ。
diff --git a/server/settings.c b/server/settings.c
index e1bb83b..3937593 100644
--- a/server/settings.c
+++ b/server/settings.c
@@ -46,27 +46,6 @@ const char *sset_level_names[] = {N_("None"),
N_("Changed")};
const int OLEVELS_NUM = ARRAY_SIZE(sset_level_names);
-
-/**************************************************************************
- A callback invoked when autotoggle is set.
-**************************************************************************/
-static bool autotoggle_callback(bool value, const char **reject_message)
-{
- reject_message = NULL;
- if (!value) {
- return TRUE;
- }
-
- players_iterate(pplayer) {
- if (!pplayer->ai.control && !pplayer->is_connected) {
- toggle_ai_player_direct(NULL, pplayer);
- send_player_info_c(pplayer, game.est_connections);
- }
- } players_iterate_end;
-
- return TRUE;
-}
-
/*************************************************************************
Verify that a given allowtake string is valid. See
game.allow_take.
@@ -186,16 +165,6 @@ static bool maxplayers_callback(int value, const char **error_string)
return TRUE;
}
-/*************************************************************************
- Create/remove players when aifill is set.
-*************************************************************************/
-static bool aifill_callback(int value, const char **error_string)
-{
- aifill(value);
- error_string = NULL;
- return TRUE;
-}
-
#define GEN_BOOL(name, value, sclass, scateg, slevel, to_client, \
short_help, extra_help, func, default) \
{name, sclass, to_client, short_help, extra_help, SSET_BOOL, \
@@ -435,7 +404,7 @@ struct settings_s settings[] = {
N_("Total number of players (including AI players)"),
N_("If set to a positive value, then AI players will be "
"automatically created or removed to keep the total "
- "number of players at this amount."), aifill_callback,
+ "number of players at this amount."), NULL,
GAME_MIN_AIFILL, GAME_MAX_AIFILL, GAME_DEFAULT_AIFILL)
/* Game initialization parameters (only affect the first start of the game,
@@ -870,7 +839,7 @@ struct settings_s settings[] = {
N_("Whether AI-status toggles with connection"),
N_("If this is set to 1, AI status is turned off when a player "
"connects, and on when a player disconnects."),
- autotoggle_callback, GAME_DEFAULT_AUTO_AI_TOGGLE)
+ NULL, GAME_DEFAULT_AUTO_AI_TOGGLE)
GEN_INT("endyear", game.info.end_year,
SSET_META, SSET_SOCIOLOGY, SSET_VITAL, SSET_TO_CLIENT,
diff --git a/server/stdinhand.c b/server/stdinhand.c
index f9096ff..9bab7b9 100644
--- a/server/stdinhand.c
+++ b/server/stdinhand.c
@@ -2960,6 +2960,23 @@ static bool set_command(struct connection *caller, char *str, bool check)
}
if (!check && do_update) {
+
+ /* Handle immediate side-effects of special setting changes. */
+ /* FIXME: Redesign setting data structures so that this can
+ * be done in a less brittle way. */
+ if (op->int_value == &game.info.aifill) {
+ aifill(*op->int_value);
+ } else if (op->bool_value == &game.info.auto_ai_toggle) {
+ if (*op->bool_value) {
+ players_iterate(pplayer) {
+ if (!pplayer->ai.control && !pplayer->is_connected) {
+ toggle_ai_player_direct(NULL, pplayer);
+ send_player_info_c(pplayer, game.est_connections);
+ }
+ } players_iterate_end;
+ }
+ }
+
send_server_setting(NULL, cmd);
/*
* send any modified game parameters to the clients -- if sent
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
