[Freeciv-Dev] (PR#10913) Re: (PR#11889) notradesize in "more options" in connection dialog

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=10913 >

Paul Zastoupil wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=11889 >
> 
> When you try to set "notradesize" in the "More options" above the
> default of 0 it always fails.  
> 
> This is because it sends the command to the server before the
> "fulltradesize" option which has to be greater.

Same as PR#10913.

Problem this isn't fixable without making these two operations atomic. 
We could switch them but then we'd have the opposite problem (when 
trying to lower fulltradesize and notradesize).  Or we could make them 
atomic so there would be a single command (/set notradesize 5 
fulltradesize 8) that would be sent for dialogs (incidentally this would 
make voting much better).  Or we could drop the restriction that 
notradesize < fulltradesize and do this check at runtime (via MAX() and 
MIN()).

This patch does the latter.  It's for 2.0 (slight changes will be needed 
for the dev branch I think, and probably updated docs as well).

-jason

? q
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.249.2.6
diff -u -r1.249.2.6 city.c
--- common/city.c       24 Dec 2004 16:59:57 -0000      1.249.2.6
+++ common/city.c       10 Jan 2005 23:59:49 -0000
@@ -2185,15 +2185,16 @@
   int dist;
   unsigned int val;
   int trade_penalty;
+  int notradesize = MIN(game.notradesize, game.fulltradesize);
+  int fulltradesize = MAX(game.notradesize, game.fulltradesize);
 
-  assert(game.notradesize < game.fulltradesize);
-  if (pcity->size <= game.notradesize) {
+  if (pcity->size <= notradesize) {
     trade_penalty = trade;
-  } else if (pcity->size >= game.fulltradesize) {
+  } else if (pcity->size >= fulltradesize) {
     trade_penalty = 0;
   } else {
-    trade_penalty = trade * (game.fulltradesize - pcity->size) /
-       (game.fulltradesize - game.notradesize);
+    trade_penalty = trade * (fulltradesize - pcity->size) /
+      (fulltradesize - notradesize);
   }
 
   if (g->corruption_level == 0) {
Index: server/settings.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settings.c,v
retrieving revision 1.5.2.6
diff -u -r1.5.2.6 settings.c
--- server/settings.c   10 Dec 2004 22:12:00 -0000      1.5.2.6
+++ server/settings.c   10 Jan 2005 23:59:49 -0000
@@ -44,34 +44,6 @@
 
 
 /**************************************************************************
-  Verify that notradesize is always smaller than fulltradesize
-**************************************************************************/
-static bool notradesize_callback(int value, const char **error_message)
-{
-  if (value < game.fulltradesize) {
-    return TRUE;
-  }
-
-  *error_message = _("notradesize must be always smaller than "
-                    "fulltradesize; keeping old value.");
-  return FALSE;
-}
-
-/**************************************************************************
-  Verify that fulltradesize is always bigger than notradesize
-**************************************************************************/
-static bool fulltradesize_callback(int value, const char **error_message)
-{
-  if (value > game.notradesize) {
-    return TRUE;
-  }
-
-  *error_message = _("fulltradesize must be always bigger than "
-                    "notradesize; keeping old value.");
-  return FALSE;
-}
-
-/**************************************************************************
   A callback invoked when autotoggle is set.
 **************************************************************************/
 static bool autotoggle_callback(bool value, const char **reject_message)
@@ -541,6 +513,12 @@
          GAME_MIN_AQUEDUCTLOSS, GAME_MAX_AQUEDUCTLOSS, 
          GAME_DEFAULT_AQUEDUCTLOSS)
 
+  /* Notradesize and fulltradesize used to have callbacks to prevent them
+   * from being set illegally (notradesize > fulltradesize).  However this
+   * provided a problem when setting them both through the client's settings
+   * dialog, since they cannot both be set atomically.  So the callbacks were
+   * removed and instead the game now knows how to deal with invalid
+   * settings. */
   GEN_INT("fulltradesize", game.fulltradesize,
          SSET_RULES, SSET_ECONOMICS, SSET_RARE, SSET_TO_CLIENT,
          N_("Minimum city size to get full trade"),
@@ -548,7 +526,7 @@
             "The penalty is 100% (no trade at all) for sizes up to "
             "notradesize, and decreases gradually to 0% (no penalty "
             "except the normal corruption) for size=fulltradesize. "
-            "See also notradesize."), fulltradesize_callback, 
+            "See also notradesize."), NULL, 
          GAME_MIN_FULLTRADESIZE, GAME_MAX_FULLTRADESIZE, 
          GAME_DEFAULT_FULLTRADESIZE)
 
@@ -558,8 +536,7 @@
          N_("Cities do not produce any trade at all unless their size "
             "is larger than this amount. The produced trade increases "
             "gradually for cities larger than notradesize and smaller "
-            "than fulltradesize. See also fulltradesize."),
-         notradesize_callback,
+            "than fulltradesize. See also fulltradesize."), NULL,
          GAME_MIN_NOTRADESIZE, GAME_MAX_NOTRADESIZE,
          GAME_DEFAULT_NOTRADESIZE)