This is the mail archive of the xconq7@sources.redhat.com mailing list for the Xconq project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Fixing repair


>I'm pretty sure the code in ai.c is wrong, but without it units would
>get very easily distracted from the task of going back for repair (you
>could see their tasks flicker between some offensive thingie and
>repair).  Of course it created (or exposed) a different problem -
>units staying in passive state - as noted in the comment, so I'm sure
>there is a better solution.  If the repair code can work like the
>resupply code, then we should be OK.

Your patch makes sense. I would, however, put it before the
check_current_target call and also check for resupply tasks and rearm tasks
at the same time, just as in ai_react_to_action. In fact, what we really
need to do is to just have ai_decide_plan call ai_react_to_action before
the to_decide_plan function pointer:

ai_decide_plan(Side *side, Unit *unit)
{
    void (*fn)(Side *side, Unit *unit);

    /* This is the replanning code from ai_react_to_action_result.
    It needs to be called at least once each turn even in the absence of any
    hostile actions, to make sure that peace garrisons are maintained. Not
    sure if this is the best place, though. */

-    /* First check that the unit's target (if it has one) is valid. */
-    check_current_target(unit);
-   /* Deal with tactical emergencies. */
-    defensive_reaction(unit);
-    offensive_reaction(unit);
+   ai_react_to_action(side, unit);

    fn = all_ai_ops[side->aitype].to_decide_plan;
    if (fn)
      (*fn)(side, unit);
}

That would solve all problems and avoid unnecessary code duplication. It
would also avoid the passive plan trap (see below).

In retrospect, it is easy to see how this bug originated. I wrote
ai_react_to_action in order to improve ai performance. It is executed
whenever a hostile unit does something within a certain radius from our
unit (u_ai_tactical_range, defaults to 4 hexes). It does two things: first,
makes sure any nearby undefended city is garrisoned (important in Civ type
games) and second, drops the current task and attacks the enemy unit.
Without this code, the ai used to wait a full turn before reacting to an
enemy. Way too slow.

Step two was that I also added this code to ai_decide_plan. The reason was
that I wanted it to execute at least once each turn, to ensure that city
garrisons never fall below the minimal limit. The real purpose of
ai_decide_plan is to reassing passive units, but it was a convenient place
to stick this code since it is executed at the start of each turn. Kind of
a hack.

Step three was that Stan found the same bug as you did, and fixed it in
ai_react_to_action by adding checks for resupply, rearm and repair tasks.
Unfortunately, this fix was never propagated to the duplicated code in
ai_decide_plan.

This illustrates the perils of code duplication. Obviously, I should just
have called ai_react_to_action from within ai_decide_plan in the first
place.

As for units getting trapped in the passive state, this is a long-standing
problem that should be fixed. However, I think the real culprit here is
ai_adjust_plan which forces units with too many plan execs into reserve
mode. The specific case you worried about will not happen if we call
ai_react_to_action in ai_decide_plan, since passive units will then fall
through and still execute to_decide_plan instead of just returning.

The fix in do_repair_task also seems logical. I guess this is what Stan
meant with his comment /* and transport repairs */ ...

A final comment about the network code. You asked if we need a
net_set_repair_task similar to net_set_resupply_task. The answer is no,
unless you would either want to add a manual command for setting a repair
task, or add repair code to the mplayer. The only reason for having
net_set_resupply_task right now is the manual do_return command.

This illustrates the fact that a lot of low level ai code (including the
resupply and repair code) really lives in the kernel (plan.c and task.c)
rather than in ai.c or the mplayer. It has two consequences. First, we
don't need network functions for this code since it runs the same way on
all computers. Second, this code is executed for all units, whether we like
it or not. Kind of a problem with human controlled units in some cases.

Hans

Hans Ronne

hronne@pp.sbbs.se



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]