[Freeciv-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-14 Thread guest

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

 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

if the creation of a unit is added the function does not have to be 
called on each turn - or are there buildings / events which change the 
upkeep of units (government change?) ... I have to think about this.

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


Re: [Freeciv-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-13 Thread Matthias Pfafferodt

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

reminder for me ...

  - Since the upkeep value is removed from struct unitgold,
  the entire unitgold vector can be simplified to an array
  (c99 dynamic array if possible) of unit pointers.
 
 I tried to change unitgold_vector to units_list but I did not get
 it to work. To get something out, I did not changed this part.
 How would a c99 dynamic array look like?

-- 
Matthias Pfafferodt - http://www.mapfa.de
Matthias.Pfafferodt at mapfa.de



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


Re: [Freeciv-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-13 Thread Matthias Pfafferodt

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
--- 

Re: [Freeciv-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-13 Thread Matthias Pfafferodt

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

version 3:

* remove city_unit_upkeep(); the functionality is provided by 
calc_city_units_upkeep()
* additional simplification (common/city.c:city_support)

not tested at all - only something that I checked before going to sleep ...

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/city.c freeciv-2.1.99svn15584.gold1//common/city.c
--- freeciv-2.1.99svn15584.gold//common/city.c	2009-04-13 20:57:03.182498213 +0200
+++ freeciv-2.1.99svn15584.gold1//common/city.c	2009-04-13 22:45:38.730498560 +0200
@@ -2152,50 +2152,14 @@
 }
 
 /**
-  Calculate upkeep of a given unit.
-**/
-void city_unit_upkeep(struct unit *punit, int *outputs, int *free_upkeep)
-{
-  struct unit_type *ut = unit_type(punit);
-  struct player *plr = unit_owner(punit);
-
-  assert(punit != NULL  ut != NULL 
-  free_upkeep != NULL  outputs != NULL);
-  memset(outputs, 0, O_COUNT * sizeof(*outputs));
-
-  /* set current upkeep on unit to zero */
-
-  output_type_iterate(o) {
-int cost = utype_upkeep_cost(ut, plr, o);
-if (cost  0) {
-  if (free_upkeep[o]  cost) {
-free_upkeep[o] -= cost;
-cost = 0;
-  } else {
-cost -= free_upkeep[o];
-free_upkeep[o] = 0;
-  }
-  outputs[o] = cost;
-}
-  } output_type_iterate_end;
-}
-
-/**
   Calculate upkeep costs.  This builds the pcity-usage[] array as well
   as setting some happiness values.
 **/
 static inline void city_support(struct city *pcity)
 {
-  int free_upkeep[O_COUNT];
   int free_unhappy = get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL);
 
-  output_type_iterate(o) {
-free_upkeep[o] = get_city_output_bonus(pcity, get_output_type(o), 
-   EFT_UNIT_UPKEEP_FREE_PER_CITY);
-  } output_type_iterate_end;
-
   /* Clear all usage values. */
-  memset(pcity-usage, 0, O_COUNT * sizeof(*pcity-usage));
   pcity-martial_law = 0;
   pcity-unit_happy_upkeep = 0;
 
@@ -,13 +2186,10 @@
   }
 
   unit_list_iterate(pcity-units_supported, this_unit) {
-int upkeep_cost[O_COUNT];
 int happy_cost = city_unit_unhappiness(this_unit, free_unhappy);
 
-city_unit_upkeep(this_unit, upkeep_cost, free_upkeep);
-
 output_type_iterate(o) {
-  pcity-usage[o] += upkeep_cost[o];
+  pcity-usage[o] += punit-upkeep[o];
 } output_type_iterate_end;
 pcity-unit_happy_upkeep += happy_cost;
   } unit_list_iterate_end;
diff '--exclude=*svn*' -ur freeciv-2.1.99svn15584.gold//common/city.h freeciv-2.1.99svn15584.gold1//common/city.h
--- freeciv-2.1.99svn15584.gold//common/city.h	2009-04-13 20:57:03.186497822 +0200
+++ freeciv-2.1.99svn15584.gold1//common/city.h	2009-04-13 22:45:38.730498560 +0200
@@ -472,7 +472,6 @@
 
 int city_population(const struct city *pcity);
 int city_unit_unhappiness(struct unit *punit, int *free_happy);
-void city_unit_upkeep(struct unit *punit, int *outputs, int *free_upkeep);
 bool city_happy(const struct city *pcity);  /* generally use celebrating instead */
 bool city_unhappy(const struct city *pcity);/* anarchy??? */
 bool base_city_celebrating(const struct city *pcity);
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-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-12 Thread guest

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

  - 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?
diff -ur freeciv-2.1.99svn15584.gold//common/packets.def freeciv-2.1.99svn15584.unit_upkeep//common/packets.def
--- freeciv-2.1.99svn15584.gold//common/packets.def	2009-03-23 20:47:57.0 +0100
+++ freeciv-2.1.99svn15584.unit_upkeep//common/packets.def	2009-04-12 11:57:22.082023611 +0200
@@ -747,6 +747,7 @@
   PLAYER owner;
   COORD x,y;
   CITY homecity;
+  UINT16 upkeep[O_MAX];
 
   UINT8 veteran;
   BOOL ai, paradropped;
diff -ur freeciv-2.1.99svn15584.gold//server/unittools.c freeciv-2.1.99svn15584.unit_upkeep//server/unittools.c
--- freeciv-2.1.99svn15584.gold//server/unittools.c	2009-03-23 20:47:57.0 +0100
+++ freeciv-2.1.99svn15584.unit_upkeep//server/unittools.c	2009-04-12 11:57:22.874025827 +0200
@@ -1827,6 +1827,7 @@
   packet-x = punit-tile-x;
   packet-y = punit-tile-y;
   packet-homecity = punit-homecity;
+  packet-upkeep = punit-upkeep;
   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


[Freeciv-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-12 Thread 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).


---
彼は陰険な奴である印象を受けた。

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


[Freeciv-Dev] (PR#40759) [patch] calc_city_units_upkeep

2009-04-11 Thread Matthias Pfafferodt

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

calculate the upkeep for all units of all cities at the start of the turn and 
save it in the unit struct

depends on ticket 40619

compile tested and also with a short game

diff -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-11 12:41:20.514129345 +0200
+++ freeciv-2.1.99svn15584.gold1//common/unit.c	2009-04-11 12:39:06.0 +0200
@@ -1373,6 +1373,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 -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-11 12:41:19.382132035 +0200
+++ freeciv-2.1.99svn15584.gold1//common/unit.h	2009-04-11 12:39:05.0 +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 -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-11 18:04:28.550131515 +0200
+++ freeciv-2.1.99svn15584.gold1//server/cityturn.c	2009-04-12 00:48:34.806125540 +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
@@ -117,6 +117,7 @@
 static void define_orig_production_values(struct city *pcity);
 static void update_city_activity(struct city *pcity);
 static void nullify_caravan_and_disband_plus(struct city *pcity);
+static void calc_city_units_upkeep(const struct city *pcity);
 
 static float city_migration_score(const struct city *pcity);
 static bool do_city_migration(struct city *pcity_from,
@@ -501,6 +502,33 @@
 }
 
 /**
+  Update upkeep needed for all units supported by the city
+**/
+static 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 +1677,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 +1756,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 +1782,6 @@
   struct cityimpr ci;
   struct unitgold_vector units;
   struct unitgold ug;
-  int free[O_COUNT], upkeep[O_COUNT];
 
   if (!pplayer) {
 return;
@@ -1783,16 +1804,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),
-