Re: [Freeciv-Dev] (PR#40171) bug in gtk client - select CMA none

[Marko Lindqvist]

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

On 28/03/2008, [EMAIL PROTECTED]  wrote:
>  This is because cmafec_preset_get_parameter gets CMA_NONE as an argument.
>  It can be fixed by testing idx != CMA_NONE before calling
>  cmafec_preset_get_parameter. It is correct because if the city is
>  CMA-controlled (controlled !=0) and we are selecting non-controlled cities
>  (idx == CMA_NONE), then we shouldn't select the city under consideration
>  (select = FALSE).

 Thanks for reporting.

 I ended up with a bit different patch. I consider this design just a
bit more robust against future changes in other parts of the codebase
(special cases are weeded out earlier, and not one by one when ever
they are considered illegal)


 - ML


diff -Nurd -X.diff_ignore freeciv/client/gui-gtk-2.0/cityrep.c freeciv/client/gui-gtk-2.0/cityrep.c
--- freeciv/client/gui-gtk-2.0/cityrep.c	2008-03-08 16:12:56.000000000 +0200
+++ freeciv/client/gui-gtk-2.0/cityrep.c	2008-03-30 12:39:01.000000000 +0300
@@ -499,15 +499,20 @@
       controlled = cma_is_city_under_agent(pcity, &parameter);
       select = FALSE;
 
-      if (idx == CMA_CUSTOM && controlled
-          && cmafec_preset_get_index_of_parameter(&parameter) == -1) {
-        select = TRUE;
-      } else if (idx == CMA_NONE && !controlled) {
-        select = TRUE;
-      } else if (idx >= 0 && controlled &&
-        	 cm_are_parameter_equal(&parameter,
-        				cmafec_preset_get_parameter(idx))) {
-        select = TRUE;
+      if (idx == CMA_NONE) {
+        /* CMA_NONE selects not-controlled, all others require controlled */
+        if (!controlled) {
+          select = TRUE;
+        }
+      } else if (controlled) {
+        if (idx == CMA_CUSTOM) {
+          if (cmafec_preset_get_index_of_parameter(&parameter) == -1) {
+            select = TRUE;
+          }
+        } else if (cm_are_parameter_equal(&parameter,
+                                          cmafec_preset_get_parameter(idx))) {
+          select = TRUE;
+        }
       }
 
       if (select) {
@@ -524,8 +529,8 @@
 
 /****************************************************************
  Create the cma entries in the change menu and the select menu. The
- indices CMA_NONE (aka -1) and CMA_CUSTOM (aka -2) are
- special. CMA_NONE signifies a preset of "none" and CMA_CUSTOM a
+ indices CMA_NONE and CMA_CUSTOM are special.
+ CMA_NONE signifies a preset of "none" and CMA_CUSTOM a
  "custom" preset.
 *****************************************************************/
 static void append_cma_to_menu_item(GtkMenuItem *parent_item, bool change_cma)

_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev