From 3e9271bf9d89364c5dbe22ce353ad54d0b381632 Mon Sep 17 00:00:00 2001
From: andrew <andrew@arobeia.co.uk>
Date: Tue, 10 Nov 2026 19:57:10 +0000
Subject: [PATCH 1/2] gtk4: change the structure of the unit selection data

The unit selection dialog builds a data structure to populate the gui from, but the
structure does not match the structure of the dialog itself - it groups first by
activity, then by the tab of the notebook to include, rather than by tab first.
This reverses that structure to make changes to the population easier.

See RM #2475
---
 client/gui-gtk-4.0/unitselect.c | 44 ++++++++--------
 client/unitselect_common.c      | 92 ++++++++++++++++-----------------
 client/unitselect_common.h      | 10 ++--
 3 files changed, 74 insertions(+), 72 deletions(-)

diff --git a/client/gui-gtk-4.0/unitselect.c b/client/gui-gtk-4.0/unitselect.c
index c1ded34816..a88594c40e 100644
--- a/client/gui-gtk-4.0/unitselect.c
+++ b/client/gui-gtk-4.0/unitselect.c
@@ -329,7 +329,7 @@ static void usdlg_tile(struct unit_select_dialog *pdialog,
 *****************************************************************************/
 static void usdlg_refresh(struct unit_select_dialog *pdialog)
 {
-  struct usdata_hash *ushash = NULL;
+  struct usdlg_data *dlg_data = NULL;
   enum unit_select_location_mode loc;
 
   if (!pdialog) {
@@ -337,12 +337,12 @@ static void usdlg_refresh(struct unit_select_dialog *pdialog)
   }
 
   /* Sort units into the hash. */
-  ushash = usdlg_data_new(pdialog->ptile);
+  dlg_data = usdlg_data_new(pdialog->ptile);
   /* Update all tabs. */
   for (loc = unit_select_location_mode_begin();
        loc != unit_select_location_mode_end();
        loc = unit_select_location_mode_next(loc)) {
-    bool show = usdlg_tab_update(pdialog, ushash, loc);
+    bool show = usdlg_tab_update(pdialog, dlg_data->tabs[loc], loc);
 
     if (!show) {
       gtk_widget_set_visible(pdialog->tabs[loc].page, FALSE);
@@ -361,7 +361,7 @@ static void usdlg_refresh(struct unit_select_dialog *pdialog)
   }
 
   /* Destroy the hash. */
-  usdlg_data_destroy(ushash);
+  usdlg_data_destroy(dlg_data);
 }
 
 /*************************************************************************//**
@@ -598,11 +598,11 @@ static bool usdlg_tab_update(struct unit_select_dialog *pdialog,
       }
 
       activity_type_iterate(act) {
-        if (unit_list_size(data->units[loc][act]) == 0) {
+        if (unit_list_size(data->units[act]) == 0) {
           continue;
         }
 
-        unit_list_iterate(data->units[loc][act], punit) {
+        unit_list_iterate(data->units[act], punit) {
           GtkTreeIter it_unit;
 
           usdlg_tab_append_units(pdialog, loc, act, punit, FALSE,
@@ -630,7 +630,7 @@ static bool usdlg_tab_update(struct unit_select_dialog *pdialog,
       activity_type_iterate(act) {
         GtkTreeIter it_act;
 
-        if (unit_list_size(data->units[loc][act]) == 0) {
+        if (unit_list_size(data->units[act]) == 0) {
           continue;
         }
 
@@ -643,19 +643,19 @@ static bool usdlg_tab_update(struct unit_select_dialog *pdialog,
 
         /* Level 2: Display unit activities. */
         usdlg_tab_append_activity(GTK_TREE_STORE(store), loc, data->utype,
-                                  act, unit_list_size(data->units[loc][act]),
+                                  act, unit_list_size(data->units[act]),
                                   &it_act, &it_utype);
 
         /* Level 3: Display all units with this activity
          *          (and transported units in further level(s)). */
-        unit_list_iterate(data->units[loc][act], punit) {
+        unit_list_iterate(data->units[act], punit) {
           GtkTreeIter it_unit;
 
           usdlg_tab_append_units(pdialog, loc, act, punit, FALSE,
                                  &it_unit, &it_act);
         } unit_list_iterate_end;
 
-        count += unit_list_size(data->units[loc][act]);
+        count += unit_list_size(data->units[act]);
 
         /* Update sum of units with this type. */
         gtk_tree_store_set(GTK_TREE_STORE(store), &it_utype, 2, count, -1);
@@ -967,54 +967,54 @@ static void usdlg_cmd_exec(GObject *object, gpointer mode_data,
   case ROW_UNITTYPE:
     {
       gint loc, utid;
-      struct usdata_hash *ushash;
+      struct usdlg_data *dlg_data;
       struct usdata *data;
 
       gtk_tree_model_get(model, &it, USDLG_COL_LOCATION, &loc,
                          USDLG_COL_UTID, &utid, -1);
 
       /* We can't be sure that all units still exists - recalc the data. */
-      ushash = usdlg_data_new(pdialog->ptile);
+      dlg_data = usdlg_data_new(pdialog->ptile);
 
-      usdata_hash_lookup(ushash, utid, &data);
+      usdata_hash_lookup(dlg_data->tabs[loc], utid, &data);
       if (data != NULL) {
         activity_type_iterate(act) {
-          if (unit_list_size(data->units[loc][act]) == 0) {
+          if (unit_list_size(data->units[act]) == 0) {
             continue;
           }
 
-          unit_list_iterate(data->units[loc][act], punit) {
+          unit_list_iterate(data->units[act], punit) {
             usdlg_cmd_exec_unit(punit, cmd);
           } unit_list_iterate_end;
         } activity_type_iterate_end;
       }
 
       /* Destroy the hash. */
-      usdlg_data_destroy(ushash);
+      usdlg_data_destroy(dlg_data);
     }
     break;
   case ROW_ACTIVITY:
     {
       gint loc, act, utid;
-      struct usdata_hash *ushash;
+      struct usdlg_data *dlg_data;
       struct usdata *data;
 
       gtk_tree_model_get(model, &it, USDLG_COL_ACTIVITY, &act,
                          USDLG_COL_LOCATION, &loc, USDLG_COL_UTID, &utid, -1);
 
       /* We can't be sure that all units still exists - recalc the data. */
-      ushash = usdlg_data_new(pdialog->ptile);
+      dlg_data = usdlg_data_new(pdialog->ptile);
 
-      usdata_hash_lookup(ushash, utid, &data);
+      usdata_hash_lookup(dlg_data->tabs[loc], utid, &data);
       if (data != NULL
-          && unit_list_size(data->units[loc][act]) != 0) {
-        unit_list_iterate(data->units[loc][act], punit) {
+          && unit_list_size(data->units[act]) != 0) {
+        unit_list_iterate(data->units[act], punit) {
           usdlg_cmd_exec_unit(punit, cmd);
         } unit_list_iterate_end;
       }
 
       /* Destroy the hash. */
-      usdlg_data_destroy(ushash);
+      usdlg_data_destroy(dlg_data);
     }
     break;
   case ROW_UNIT:
diff --git a/client/unitselect_common.c b/client/unitselect_common.c
index b73b5d6381..6c9f0525bd 100644
--- a/client/unitselect_common.c
+++ b/client/unitselect_common.c
@@ -47,20 +47,15 @@ static void usdata_destroy(struct usdata *data);
 *****************************************************************************/
 static struct usdata *usdata_new(void)
 {
-  enum unit_select_location_mode loc;
   enum unit_activity act;
 
   struct usdata *data = fc_calloc(1, sizeof(*data));
 
   data->utype = nullptr;
 
-  for (loc = unit_select_location_mode_begin();
-       loc != unit_select_location_mode_end();
-       loc = unit_select_location_mode_next(loc)) {
-    for (act = 0; act < ACTIVITY_LAST; act++) {
-      /* Create a unit list for all activities - even invalid once. */
-      data->units[loc][act] = unit_list_new();
-    }
+  for (act = 0; act < ACTIVITY_LAST; act++) {
+    /* Create a unit list for all activities - even invalid once. */
+    data->units[act] = unit_list_new();
   }
 
   return data;
@@ -72,16 +67,11 @@ static struct usdata *usdata_new(void)
 static void usdata_destroy(struct usdata *data)
 {
   if (data != nullptr) {
-    enum unit_select_location_mode loc;
     enum unit_activity act;
 
-    for (loc = unit_select_location_mode_begin();
-         loc != unit_select_location_mode_end();
-         loc = unit_select_location_mode_next(loc)) {
-      for (act = 0; act < ACTIVITY_LAST; act++) {
-        if (data->units[loc][act] != nullptr) {
-          unit_list_destroy(data->units[loc][act]);
-        }
+    for (act = 0; act < ACTIVITY_LAST; act++) {
+      if (data->units[act] != nullptr) {
+        unit_list_destroy(data->units[act]);
       }
     }
     free(data);
@@ -91,23 +81,32 @@ static void usdata_destroy(struct usdata *data)
 /*************************************************************************//**
   Create a unit selection data set.
 *****************************************************************************/
-struct usdata_hash *usdlg_data_new(const struct tile *ptile)
+struct usdlg_data *usdlg_data_new(const struct tile *ptile)
 {
-  struct usdata_hash *ushash = usdata_hash_new();
+  struct usdlg_data* dlg_data = fc_calloc(1, sizeof(*dlg_data));
+
+  enum unit_select_location_mode loc;
+
+  for (loc = unit_select_location_mode_begin();
+       loc != unit_select_location_mode_end();
+       loc = unit_select_location_mode_next(loc)) {
+    dlg_data->tabs[loc] = usdata_hash_new();
+  }
 
-  /* Add units of the player(s). */
-  if (client_is_global_observer()) {
-    players_iterate(pplayer) {
+  players_iterate(pplayer) {
+    if (client_is_global_observer() || pplayer == client_player()) {
       unit_list_iterate(pplayer->units, punit) {
-        usdlg_data_add_unit(ushash, ptile, punit);
+        for (loc = unit_select_location_mode_begin();
+             loc != unit_select_location_mode_end();
+             loc = unit_select_location_mode_next(loc)) {
+          if (usdlg_check_unit_location(punit, ptile, loc)) {
+            usdlg_data_add_unit(dlg_data->tabs[loc], ptile, punit);
+          }
+        }
       } unit_list_iterate_end;
-    } players_iterate_end;
-  } else {
-    struct player *pplayer = client_player();
-    unit_list_iterate(pplayer->units, punit) {
-      usdlg_data_add_unit(ushash, ptile, punit);
-    } unit_list_iterate_end;
-  }
+    }
+  } players_iterate_end;
+  
 
   /* Add units on the tile (SELLOC_UNITS). */
   unit_list_iterate(ptile->units, punit) {
@@ -117,30 +116,37 @@ struct usdata_hash *usdlg_data_new(const struct tile *ptile)
       struct usdata *data;
       const struct unit_type *ptype = unit_type_get(punit);
 
-      usdata_hash_lookup(ushash, utype_index(ptype), &data);
+      usdata_hash_lookup(dlg_data->tabs[SELLOC_UNITS], utype_index(ptype), &data);
 
       if (!data) {
         data = usdata_new();
         data->utype = ptype;
-        usdata_hash_insert(ushash, utype_index(ptype), data);
+        usdata_hash_insert(dlg_data->tabs[SELLOC_UNITS], utype_index(ptype), data);
       }
 
-      unit_list_append(data->units[SELLOC_UNITS][ACTIVITY_IDLE], punit);
+      unit_list_append(data->units[ACTIVITY_IDLE], punit);
     }
   } unit_list_iterate_end;
 
-  return ushash;
+  return dlg_data;
 }
 
 /*************************************************************************//**
   Destroy a unit selection data set.
 *****************************************************************************/
-void usdlg_data_destroy(struct usdata_hash *ushash)
+void usdlg_data_destroy(struct usdlg_data *dlg_data)
 {
-  usdata_hash_data_iterate(ushash, data) {
-    usdata_destroy(data);
-  } usdata_hash_data_iterate_end;
-  usdata_hash_destroy(ushash);
+  enum unit_select_location_mode loc;
+
+  for (loc = unit_select_location_mode_begin();
+       loc != unit_select_location_mode_end();
+       loc = unit_select_location_mode_next(loc)) {
+    usdata_hash_data_iterate(dlg_data->tabs[loc], data) {
+      usdata_destroy(data);
+    } usdata_hash_data_iterate_end;
+    usdata_hash_destroy(dlg_data->tabs[loc]);
+  }
+  free(dlg_data);
 }
 
 /*************************************************************************//**
@@ -167,15 +173,7 @@ static void usdlg_data_add_unit(struct usdata_hash *ushash,
 
   for (act = 0; act < ACTIVITY_LAST; act++) {
     if (usdlg_check_unit_activity(punit, act)) {
-      enum unit_select_location_mode loc;
-
-      for (loc = unit_select_location_mode_begin();
-           loc != unit_select_location_mode_end();
-           loc = unit_select_location_mode_next(loc)) {
-        if (usdlg_check_unit_location(punit, ptile, loc)) {
-          unit_list_append(data->units[loc][act], punit);
-        }
-      }
+      unit_list_append(data->units[act], punit);
     }
   }
 }
diff --git a/client/unitselect_common.h b/client/unitselect_common.h
index 2c4375b6bf..b6d4a6282d 100644
--- a/client/unitselect_common.h
+++ b/client/unitselect_common.h
@@ -23,7 +23,7 @@ struct tile;
 
 struct usdata {
   const struct unit_type *utype;
-  struct unit_list *units[SELLOC_COUNT][ACTIVITY_LAST];
+  struct unit_list *units[ACTIVITY_LAST];
 };
 
 #define SPECHASH_TAG usdata
@@ -36,8 +36,12 @@ struct usdata {
 #define usdata_hash_data_iterate_end                                         \
   HASH_DATA_ITERATE_END
 
-struct usdata_hash *usdlg_data_new(const struct tile *ptile);
-void usdlg_data_destroy(struct usdata_hash *ushash);
+struct usdlg_data {
+  struct usdata_hash* tabs[SELLOC_COUNT];
+};
+  
+struct usdlg_data *usdlg_data_new(const struct tile *ptile);
+void usdlg_data_destroy(struct usdlg_data *ushash);
 
 #ifdef __cplusplus
 }
-- 
2.47.3

