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

Am Monday 13 April 2009 03:38:44 schrieb Madeline Book:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=40759 >
>
> > [guest - Sun Apr 12 18:29:01 2009]:
> > > > - Perhaps the values of the unit upkeep field should be
> > > > sent in unit info packets so that they do not have to
> > > > be recomputed at the client side.
> > >
> > > I will look into this.
> >
> > Should this be something like in the attached file
> > (unit_upkeep_packages.patch.diff)? Or are additional changes needed?
>
> The field 'upkeep' being an array (i.e. pointer) you need to
> copy element by element (e.g. with a loop, memcpy, etc.).
> Your way would only work if the memory for the unit struct
> was not overwritten before the packet is serialized (likely
> true, but not a good assumption to make).
>
> Also you would need to "unpack" the value again at the client
> side (from packet to unit struct). See unit info handling
> functions in client/packhand.c.
>
> I was also wondering whether there are any places in the
> client that could make use of this extra information. This
> would involve checking where the upkeep calculation functions
> are called on the client side and simplifying the calling
> code to instead use the punit->upkeep field. Better yet
> would be to make an accessor unit_get_upkeep() (or with a
> similar name), that would wrap read access to the field.
> As a first step it could just use the old upkeep calculation
> functions (i.e. so that it ends up executing the old code
> path) so that you can test to make sure everything is
> alright, then if everything is ok you can make it use
> the punit->upkeep field by only having to change that one
> function.
>
> Hopefully the client uses the upkeep values in a simple
> enough way that this would be possible, that is it avoids
> any kind of tricky setting of the values or calculating
> "virtual" values (i.e. upkeep for a unit that does not
> actually exist in the game).
>
>
> -----------------------------------------------------------------------
> 彼は陰険な奴である印象を受けた。

I updated the patch and hope, that I understood all the code ...

The upkeep information is now send to the client and the values are updated 
each turn and if

* a unit is disbanded
* a unit is bribed by a spy
* a unit is disbanded and
* a unit is updated

are this to much information? i.e should the client now this about the own 
(OK) and all visible units (?)

The update code only needs to be called, if there are changes to the city and 
not each turn. Does this values need to be saved in the savegames? They can 
easily be recomputed.

The changes to the clients are in PR#40763.

All patches of this stack together do compile. I did not have the time to test 
it on a game ...



One related question:

The end of the output array is marked by O_LAST. But there are O_COUNT, O_MAX 
and the variable 'num_output_types' all with the same value. Should/can this 
be unified to O_LAST?

diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//client/packhand.c freeciv-2.1.99svn15584.gold1//client/packhand.c
--- freeciv-2.1.99svn15584.gold//client/packhand.c	2009-04-13 20:57:13.610498460 +0200
+++ freeciv-2.1.99svn15584.gold1//client/packhand.c	2009-04-13 21:56:00.166499236 +0200
@@ -1195,7 +1195,7 @@
   bool check_focus = FALSE;     /* conservative focus change */
   bool moved = FALSE;
   bool ret = FALSE;
-  
+
   punit = player_find_unit_by_id(unit_owner(packet_unit), packet_unit->id);
   if (!punit && game_find_unit_by_number(packet_unit->id)) {
     /* This means unit has changed owner. We deal with this here
@@ -1406,6 +1406,10 @@
       }
     }
 
+    /* update unit upkeep */
+    output_type_iterate(o) {
+      punit->upkeep[o] = packet_unit->upkeep[o];
+    } output_type_iterate_end;
     punit->veteran = packet_unit->veteran;
     punit->moves_left = packet_unit->moves_left;
     punit->fuel = packet_unit->fuel;
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//common/fc_types.h freeciv-2.1.99svn15584.gold1//common/fc_types.h
--- freeciv-2.1.99svn15584.gold//common/fc_types.h	2009-04-13 20:57:03.206498100 +0200
+++ freeciv-2.1.99svn15584.gold1//common/fc_types.h	2009-04-13 21:04:48.062498776 +0200
@@ -75,6 +75,7 @@
 enum output_type_id {
   O_FOOD, O_SHIELD, O_TRADE, O_GOLD, O_LUXURY, O_SCIENCE, O_LAST
 };
+/* num_output_types = O_LAST; see common/city.c */
 #define O_COUNT num_output_types
 #define O_MAX O_LAST /* Changing this breaks network compatibility. */
 
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//common/packets.def freeciv-2.1.99svn15584.gold1//common/packets.def
--- freeciv-2.1.99svn15584.gold//common/packets.def	2009-04-13 20:57:29.806502033 +0200
+++ freeciv-2.1.99svn15584.gold1//common/packets.def	2009-04-13 21:04:48.062498776 +0200
@@ -747,6 +747,7 @@
   PLAYER owner;
   COORD x,y;
   CITY homecity;
+  UINT16 upkeep[O_MAX];
 
   UINT8 veteran;
   BOOL ai, paradropped;
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//common/unit.c freeciv-2.1.99svn15584.gold1//common/unit.c
--- freeciv-2.1.99svn15584.gold//common/unit.c	2009-04-13 20:57:03.158498606 +0200
+++ freeciv-2.1.99svn15584.gold1//common/unit.c	2009-04-13 21:04:48.066498664 +0200
@@ -34,7 +34,6 @@
 #include "unit.h"
 #include "unitlist.h"
 
-
 /**************************************************************************
 bribe unit
 investigate
@@ -1373,6 +1372,8 @@
     punit->tile = NULL;
     punit->homecity = IDENTITY_NUMBER_ZERO;
   }
+  /* upkeep will be set each turn within the cityturn loop */
+  memset(punit->upkeep, 0, O_LAST * sizeof(*punit->upkeep));
   punit->goto_tile = NULL;
   punit->veteran = veteran_level;
   /* A unit new and fresh ... */
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//common/unit.h freeciv-2.1.99svn15584.gold1//common/unit.h
--- freeciv-2.1.99svn15584.gold//common/unit.h	2009-04-13 20:57:03.166497823 +0200
+++ freeciv-2.1.99svn15584.gold1//common/unit.h	2009-04-11 12:39:05.000000000 +0200
@@ -142,6 +142,7 @@
   struct player *owner; /* Cannot be NULL. */
   int id;
   int homecity;
+  int upkeep[O_LAST]; /* unit upkeep with regards to the homecity */
 
   int moves_left;
   int hp;
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//server/cityturn.c freeciv-2.1.99svn15584.gold1//server/cityturn.c
--- freeciv-2.1.99svn15584.gold//server/cityturn.c	2009-04-13 20:57:29.810498568 +0200
+++ freeciv-2.1.99svn15584.gold1//server/cityturn.c	2009-04-13 21:25:45.474498694 +0200
@@ -93,10 +93,10 @@
 #define SPECVEC_TYPE struct cityimpr
 #include "specvec.h"
 
-/* Helper struct for storing a unit with its gold upkeep. */
+/* Helper struct for storing all units with gold upkeep.
+ * Replace this by unit_list? - MaPfa */
 struct unitgold {
   struct unit *punit;
-  int gold_upkeep;
 };
 
 #define SPECVEC_TAG unitgold
@@ -501,6 +501,33 @@
 }
 
 /**************************************************************************
+  Update upkeep needed for all units supported by the city
+**************************************************************************/
+void calc_city_units_upkeep(const struct city *pcity)
+{
+  int free[O_LAST], upkeep[O_LAST];
+
+  if (!pcity || !pcity->units_supported
+      || unit_list_size(pcity->units_supported) < 1) {
+    return;
+  }
+
+  memset(free, 0, O_COUNT * sizeof(*free));
+  output_type_iterate(o) {
+    free[o] = get_city_output_bonus(pcity, get_output_type(o),
+                                    EFT_UNIT_UPKEEP_FREE_PER_CITY);
+  } output_type_iterate_end;
+
+  /* save the upkeep for all units in the corresponding punit struct */
+  unit_list_iterate(pcity->units_supported, punit) {
+    city_unit_upkeep(punit, upkeep, free);
+    output_type_iterate(o) {
+      punit->upkeep[o] = upkeep[o];
+    } output_type_iterate_end;
+  } unit_list_iterate_end;
+}
+
+/**************************************************************************
   Reduce the city specialists by some (positive) value.
   Return the amount of reduction.
 **************************************************************************/
@@ -1649,20 +1676,14 @@
 static int city_total_unit_gold_upkeep(const struct city *pcity)
 {
   int gold_needed = 0;
-  int free[O_COUNT], upkeep[O_COUNT];
 
   if (!pcity || !pcity->units_supported
       || unit_list_size(pcity->units_supported) < 1) {
     return 0;
   }
 
-  memset(free, 0, O_COUNT * sizeof(*free));
-  free[O_GOLD] = get_city_output_bonus(pcity, get_output_type(O_GOLD),
-                                       EFT_UNIT_UPKEEP_FREE_PER_CITY);
-
   unit_list_iterate(pcity->units_supported, punit) {
-    city_unit_upkeep(punit, upkeep, free);
-    gold_needed += upkeep[O_GOLD];
+    gold_needed += punit->upkeep[O_GOLD];
   } unit_list_iterate_end;
 
   return gold_needed;
@@ -1734,7 +1755,7 @@
   while (pplayer->economic.gold < 0 && n > 0) {
     r = myrand(n);
     punit = units->p[r].punit;
-    gold_upkeep = units->p[r].gold_upkeep;
+    gold_upkeep = punit->upkeep[O_GOLD];
 
     notify_player(pplayer, unit_tile(punit), E_UNIT_LOST_MISC,
                   _("Not enough gold. %s disbanded"),
@@ -1760,7 +1781,6 @@
   struct cityimpr ci;
   struct unitgold_vector units;
   struct unitgold ug;
-  int free[O_COUNT], upkeep[O_COUNT];
 
   if (!pplayer) {
     return;
@@ -1783,16 +1803,10 @@
     goto CLEANUP;
   }
 
-  memset(free, 0, O_COUNT * sizeof(*free));
-
   city_list_iterate(pplayer->cities, pcity) {
-    free[O_GOLD] = get_city_output_bonus(pcity, get_output_type(O_GOLD),
-                                         EFT_UNIT_UPKEEP_FREE_PER_CITY);
     unit_list_iterate(pcity->units_supported, punit) {
-      city_unit_upkeep(punit, upkeep, free);
-      if (upkeep[O_GOLD] > 0) {
+      if (punit->upkeep[O_GOLD] > 0) {
         ug.punit = punit;
-        ug.gold_upkeep = upkeep[O_GOLD];
         unitgold_vector_append(&units, &ug);
       }
     } unit_list_iterate_end;
@@ -1826,7 +1840,6 @@
   struct cityimpr ci;
   struct unitgold_vector units;
   struct unitgold ug;
-  int free[O_COUNT], upkeep[O_COUNT];
 
   if (!pcity) {
     return;
@@ -1850,16 +1863,10 @@
     goto CLEANUP;
   }
 
-  memset(free, 0, O_COUNT * sizeof(*free));
-  free[O_GOLD] = get_city_output_bonus(pcity, get_output_type(O_GOLD),
-                                       EFT_UNIT_UPKEEP_FREE_PER_CITY);
-
   /* Create a vector of all supported units with gold upkeep. */
   unit_list_iterate(pcity->units_supported, punit) {
-    city_unit_upkeep(punit, upkeep, free);
-    if (upkeep[O_GOLD] > 0) {
+    if (punit->upkeep[O_GOLD] > 0) {
       ug.punit = punit;
-      ug.gold_upkeep = upkeep[O_GOLD];
       unitgold_vector_append(&units, &ug);
     }
   } unit_list_iterate_end;
@@ -2043,6 +2050,9 @@
   pplayer = city_owner(pcity);
   gov = government_of_city(pcity);
 
+  /* update upkeep of suported units */
+  calc_city_units_upkeep(pcity);
+
   city_refresh(pcity);
 
   /* Reporting of celebrations rewritten, copying the treatment of disorder below,
Only in freeciv-2.1.99svn15584.gold1//server: cityturn.c.orig
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//server/cityturn.h freeciv-2.1.99svn15584.gold1//server/cityturn.h
--- freeciv-2.1.99svn15584.gold//server/cityturn.h	2009-04-13 20:57:06.042496918 +0200
+++ freeciv-2.1.99svn15584.gold1//server/cityturn.h	2009-04-13 21:18:54.270500838 +0200
@@ -48,5 +48,6 @@
 void nullify_prechange_production(struct city *pcity);
 
 void check_city_migrations(struct player *pplayer);
+void calc_city_units_upkeep(const struct city *pcity);
 
 #endif  /* FC__CITYTURN_H */
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//server/diplomats.c freeciv-2.1.99svn15584.gold1//server/diplomats.c
--- freeciv-2.1.99svn15584.gold//server/diplomats.c	2009-04-13 20:57:06.054496582 +0200
+++ freeciv-2.1.99svn15584.gold1//server/diplomats.c	2009-04-13 21:45:15.390499956 +0200
@@ -472,6 +472,16 @@
   /* This may cause a diplomatic incident */
   maybe_cause_incident(DIPLOMAT_BRIBE, pplayer, pvictim, NULL);
 
+  /* update unit upkeep in the homecity of the victim */
+  if (pvictim->homecity > 0) {
+    /* update unit upkeep */
+    calc_city_units_upkeep(game_find_city_by_number(pvictim->homecity));
+  }
+  /* update unit upkeep in the new homecity */
+  if (pvictim->homecity > 0) {
+    calc_city_units_upkeep(game_find_city_by_number(gained_unit->homecity));
+  }
+
   /* Be sure to wipe the converted unit! */
   victim_tile = pvictim->tile;
   wipe_unit(pvictim);
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//server/unithand.c freeciv-2.1.99svn15584.gold1//server/unithand.c
--- freeciv-2.1.99svn15584.gold//server/unithand.c	2009-04-13 20:57:06.062496638 +0200
+++ freeciv-2.1.99svn15584.gold1//server/unithand.c	2009-04-13 21:32:55.838497178 +0200
@@ -398,11 +398,16 @@
   if (old_pcity) {
     /* Even if unit is dead, we have to unlink unit pointer (punit). */
     unit_list_unlink(old_pcity->units_supported, punit);
+    /* update unit upkeep */
+    calc_city_units_upkeep(old_pcity);
   }
 
   if (unit_alive) {
     unit_list_prepend(new_pcity->units_supported, punit);
 
+    /* update unit upkeep */
+    calc_city_units_upkeep(new_pcity);
+
     punit->homecity = new_pcity->id;
     send_unit_info(unit_owner(punit), punit);
   }
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//server/unittools.c freeciv-2.1.99svn15584.gold1//server/unittools.c
--- freeciv-2.1.99svn15584.gold//server/unittools.c	2009-04-13 20:57:29.822498512 +0200
+++ freeciv-2.1.99svn15584.gold1//server/unittools.c	2009-04-13 21:52:01.498495489 +0200
@@ -1338,6 +1338,9 @@
                          - game.info.upgrade_veteran_loss, 0);
   }
 
+  /* update unit upkeep */
+  calc_city_units_upkeep(game_find_city_by_number(punit->homecity));
+
   conn_list_do_buffer(pplayer->connections);
 
   unit_refresh_vision(punit);
@@ -1382,6 +1385,8 @@
     punit->homecity = 0; /* none */
   } else {
     punit->homecity = homecity_id;
+    /* update unit upkeep */
+    calc_city_units_upkeep(game_find_city_by_number(homecity_id));
   }
 
   if (hp_left >= 0) {
@@ -1579,6 +1584,9 @@
     server_remove_unit(punit);
   }
 
+  /* update unit upkeep */
+  calc_city_units_upkeep(game_find_city_by_number(punit->homecity));
+
   /* Finally reassign, bounce, or destroy all units that cannot exist at this
    * location without transport. */
   if (drowning) {
@@ -1827,6 +1835,9 @@
   packet->x = punit->tile->x;
   packet->y = punit->tile->y;
   packet->homecity = punit->homecity;
+  output_type_iterate(o) {
+    packet->upkeep[o] = punit->upkeep[o];
+  } output_type_iterate_end;
   packet->veteran = punit->veteran;
   packet->type = utype_number(unit_type(punit));
   packet->movesleft = punit->moves_left;
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to