Re: Bug in acp-independent action code

[Lincoln Peters <sampln@sbcglobal.net>]

On Mon, 2004-06-14 at 16:31, Hans Ronne wrote:
> I have now figured out what is going on with your game. It is actually a
> quite interesting bug that raises several issues.
> 
> What happens is that the last build task before a builder runs out of ores
> fails if and only if all ores a consumed in the process. The reason for
> this is that in addition to setting unit-consumption-per-cp (which is what
> you are supposed to use with advanced units) to (u* ores 1), you have also
> set material-to-build to (u* ores 1).
> 
> The latter table is not a consumption table but rather sets a minimal
> amount of a unique "material" which is required to carry out any building.
> It is analogous to material-to-attack and material-to-fire, as opposed to
> consumption-per-attack and consumption-per-fire. I don't know if you
> remember the discussion about these tables some months ago. Basically, the
> materials-to-* tables are supposed to represent non-perishable "materials"
> such as guns, as opposed to ammo. I guess that in the building case, it
> might represent tools as opposed to raw materials.

I remember the discussion.  However, the last time I checked, I still
needed to set material-to-build (and other material-to-... tables)
because otherwise a unit could continue doing whatever it was doing,
even if it ran out of the required material(s).

> 
> Now, I never anticipated that somebody might use this table with the
> acp-independent units, and furthermore that the material would be the same
> as the one consumed per cp. Acp-independent units differ from normal units
> in that building material is consumed before the build task is executed, in
> run_construction. This would be risky unless we can be sure that the build
> task never fails, which is the normal case for acp-independent units (they
> don't use acps and they don't consume materials-per-build, which is what
> check_build_action tests for).
> 
> However, by using material-to-build and setting it to the same material as
> unit-consumption-per-cp, you have discovered a way to make do_build_task
> fail for acp-independent units. This is because run-construction uses up
> the ores, so there is nothing left when check_build_action checks the very
> same material in material-to-build.
> 
> So why does it matter if the build task fails? Now, here comes another
> interesting point. The task execution code has a safety valve that will
> kill any task that fails a certain number of times. There is also a fixed
> probability of killing a task that fails irrespective of how many times it
> failed previously. Finally, there is a fixed probability of putting the
> unit into reserve.
> 
> When a build task is killled in your game, building will proceed as normal
> after a new build task has been created, using the incomplete unit as
> target. However, one consequence of killing the task is that the run
> length, which is a task argument, is lost. Which is exactly what you see.

Indeed.

> 
> Here is what the task execution code looks like:
> 
>       case TASK_FAILED:
>         ++task->retrynum;
> 	DMprintf("failed try %d, ", task->retrynum);
> 	/* If a task fails, it might be because the task cannot be
> 	   completed, or just because conditions are temporarily
> 	   unfavorable, such as a passing unit blocking the way while
> 	   moving through.  So we need to retry a couple times at
> 	   least; the variables here control how hard to keep
> 	   trying. */
> 	/* (should be doctrine, since these affect human-run units too) */
> 	if (probability(g_ai_badtask_remove_chance())
> 	    || task->retrynum >= g_ai_badtask_max_retries()) {
> 	    pop_task(plan);
> 	    DMprintf("removed it");
> 	    /* We might be buzzing, so maybe go into reserve. */
> 	    if (probability(g_ai_badtask_reserve_chance())) {
> 		plan->reserve = TRUE;
> 	    	DMprintf(" and went into reserve");
> 	    }
> 	} else {
> 	    DMprintf("will retry");
> 	}
> 	break;
> 
> I have long had my doubts about this code. I think that if there is buzzing
> because the AI is trying to do something impossible or there is a bug in
> the game module, the correct thing to is to fix the bug. Moreover, I doubt
> that it is a good idea to have this code execute for human-controlled
> units. If their tasks fail, it would be better to prompt the human player
> for what to do.

I remember that, in space-civ.g, I set ai-badtask-reserve-chance to 0
for this exact reason.  I figured that, at least in space-civ.g, there
was virtually no chance that a non-acp-independent unit would fail at a
task due to temporary situation.

> 
> You will notice the gvars g_ai_badtask_remove_chance,
> g_ai_badtask_max_retries and g_ai_badtask_reserve_chance. I added them
> several years ago because I wanted to be able to turn off this piece of
> code (previously, hardcoded numbers were used).

Hard-coded numbers?  Sounds like a real pain.

> 
> So there are actually three problems at different levels and therefore also
> three solutions to your bug.
> 
> 1. The easiest fix is to get rid of material-to-build, at least until it is
> supported for acp-independent units.

I suppose I could re-configure the module so that only
non-acp-independent units (conjurers and necromancers) are affected by
material-to-build.  They need the material-to-build code because
otherwise they could continue to build after they exhaust their supply.

> 
> 2. Another fix would be to move part of the acp-independent build code into
> do_build_action, so that materials are consumed only when the action is
> executed (i.e. after check_build_action).
> This would make sense, and is something I have planned to do. However, it
> is not a trivial thing to do since the acp-independent build code differs a
> lot from the normal build code.

Sounds like the ideal solution, but as you say, not an easy solution. 
Might be another thing to add to the TODO list for v7.5.

> 
> 3. The third fix would be to get rid of the safety valve in task execution
> and allow units to retry forever with the failed task. Granted, tasks may
> be temporarily impossible, in which case going into reserve and retrying
> next turn make sense. However, as you can see in the above code, the unit
> goes into reserve only after it killed the task. I think it might make
> sense to instead save the task (whether a build task or a move task) and
> try again next turn when the conditions have changed.

In that case the cure may be worse than the disease.  Imagine the
frustration in the standard game if a battleship or carrier (or a
similarly expensive unit in another game) ran out of fuel because
something blocked its path to the supply point, and the player never
caught it because the plan was never canceled.

---
Lincoln Peters
<sampln@sbcglobal.net>

Are you sure the back door is locked?