[Freeciv-Dev] Re: [PATCH] aiunit.c ai_military_findvictim() cleanup (PR#1264)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Feb 20, 2002 at 03:16:36PM -0600, Mike Kaufman wrote:
> On Wed, Feb 20, 2002 at 09:49:53PM +0100, Raimar Falke wrote:
> > On Wed, Feb 20, 2002 at 08:49:26PM +0100, Petr Baudis wrote:
> > > > > +          benefit = (benefit + 40) * punit->hp / 
> > > > > unit_type(punit)->hp;
> > > > 
> > > > What is that 40?
> > > 
> > > Just magic value of city. It's used as this very frequently (I'm not sure 
> > > that
> > > equal value for city walls is absolute coincidence ;).
> > > 
> > > This would be nice to have as some #define or at least mentioned in 
> > > README_AI.
> > > Someone promised to extend it. Who was it? Raahul?
> > > 
> > > > > +        SET_BEST(99999);
> > > > ...
> > > > > +      if (map_has_special(x1, y1, S_HUT) && best < 99999
> > > > > +          && could_unit_move_to_tile(punit, punit->x, punit->y, x1, 
> > > > > y1) != 0
> > > > > +          && !is_barbarian(unit_owner(punit))
> > > > > +          && punit->ai.ai_role != AIUNIT_ESCORT
> > > > > +          && punit->ai.charge == 0 /* Above line doesn't seem to 
> > > > > work. :( */
> > > > > +          && punit->ai.ai_role != AIUNIT_DEFEND_HOME) {
> > > > > +        SET_BEST(99998);
> > > > > +      }
> > > > 
> > > > These 9999* are ugly.
> > > 
> > > And what shall we do with them..?
> > 
> > Change the SET_BEST to a SET_BEST_IF_BETTER (this will remove the
> > "best < 99999"). Add
> 
> disagree. putting the test there cuts out a bunch of useless tests and a
> couple of function calls.

I don't see any function calls:

old:
+#define SET_TARGET(new_best) \
+  do { best = (new_best); *dest_x = x1; *dest_y = y1; } while (0)

new:
+#define SET_TARGET(new_best) \
+  do { if(new_best>best){best = (new_best); *dest_x = x1; *dest_y = y1; }} 
while (FALSE)

old:
+          if (desire > best && ai_fuzzy(pplayer, 1)) {
+            freelog(LOG_DEBUG, "Better than %d is %d (%s)",
+                          best, desire, unit_type(pdef)->name);
+            SET_TARGET(desire);

new:
+          if (ai_fuzzy(pplayer, 1)) {
+            freelog(LOG_DEBUG, "Better than %d is %d (%s)",
+                          best, desire, unit_type(pdef)->name);
+            SET_TARGET(desire);

old:
+      /* ...or tiny pleasant hut here! */
+      if (map_has_special(x1, y1, S_HUT) && best < 99999
+          && could_unit_move_to_tile(punit, punit->x, punit->y, x1, y1) != 0
+          && !is_barbarian(unit_owner(punit))
+          && punit->ai.ai_role != AIUNIT_ESCORT
+          && punit->ai.charge == 0 /* Above line doesn't seem to work. :( */
+          && punit->ai.ai_role != AIUNIT_DEFEND_HOME) {
+        SET_TARGET(99998);
+      }

new:
+      /* ...or tiny pleasant hut here! */
+      if (map_has_special(x1, y1, S_HUT)
+          && could_unit_move_to_tile(punit, punit->x, punit->y, x1, y1) != 0
+          && !is_barbarian(unit_owner(punit))
+          && punit->ai.ai_role != AIUNIT_ESCORT
+          && punit->ai.charge == 0 /* Above line doesn't seem to work. :( */
+          && punit->ai.ai_role != AIUNIT_DEFEND_HOME) {
+        SET_TARGET(99998);
+      }

Maybe SET_TARGET should be renamed in CONSIDER_TARGET.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Premature optimization is the root of all evil."
    -- D. E. Knuth in "Structured Programming with go to Statements"