[Freeciv-Dev] Re: (PR#8754) effects patch

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8754 >

Vasc and Per,

I only read the AI code bits, here are some comments:
1. in advmilitary.c:
+  if (sailing && get_city_bonus(pcity, EFT_SEA_DEFEND)) {
+    danger /= 2;
+  }
isn't get_bonus returning a real bonus which we can use in place of the
magic 2?

2. +      /* TODO: Case for Airport. -- Raahul */
Remove it.  Planes are not considered here and will never be.

3. In adjust_building_want_by_effects, please, PLEASE give as many
comments as possible, for each effect considered.  If it's a wag, say it.
If there is some logic behind, spell it out.

4. Same function, in EFT_CORRUPT_PCT (and maybe other places), can't we
use the amount instead of the magic numbers?

5. in ai_manage_buildings
+      if (pcity->ai.next_recalc > game.turn) {
+        continue; /* do not recalc yet */
+      } else {
+        pcity->ai.building_want[id] = 0; /* do recalc */
+      }
change to
+      if (pcity->ai.next_recalc > game.turn) {
+        continue; /* do not recalc yet */
+      }
+
+      pcity->ai.building_want[id] = 0;
it's more readable imo.

6. Changes to aidiplomat can go in separately, if effects aren't going in
immediately

7. Isn't the list in effects.h a good place to give explanations of each
effect?


G.