Bug in acp-independent action code

Bug in acp-independent action code

[Lincoln Peters]

It would appear that, if an acp-independent unit executes a build task
that depends on a certain material (as per unit-consumption-per-cp), and
it runs out of the material before it can complete the build task, it
will often abort the task and go into reserve, whereas it should go into
reserve but keep the plan.

It took me a while to pin this bug down, since an acp-independent unit
with no task assigned automatically resumes building any incomplete
units that occupy it.  Consequently, it will build at least one unit
despite the bug, and so it initially appeared to be a bug in the code
that handles the number of units built as the result of a single build
task (e.g. you tell it to build 99 knights and it only builds 1).

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

Air is water with holes in it.

Re: Bug in acp-independent action code

[Hans Ronne]

>It would appear that, if an acp-independent unit executes a build task
>that depends on a certain material (as per unit-consumption-per-cp), and
>it runs out of the material before it can complete the build task, it
>will often abort the task and go into reserve, whereas it should go into
>reserve but keep the plan.
>
>It took me a while to pin this bug down, since an acp-independent unit
>with no task assigned automatically resumes building any incomplete
>units that occupy it.  Consequently, it will build at least one unit
>despite the bug, and so it initially appeared to be a bug in the code
>that handles the number of units built as the result of a single build
>task (e.g. you tell it to build 99 knights and it only builds 1).

This I could not reproduce. If I tell an acp-independent unit (a city in
the advances game) to build e. g. 3 colonizers it does exactly that, and
then prompts for a new build task. I have not seen it go into reserve or
abort the plan, even though it is limited by unit-consumption-per-cp and
runs out of material each turn (as it should).

What game did this happen in, and do you have a saved game where the bug
can be seen?

Hans

Re: Bug in acp-independent action code

[Lincoln Peters]

On Thu, 2004-06-10 at 23:38, Hans Ronne wrote:
> This I could not reproduce. If I tell an acp-independent unit (a city in
> the advances game) to build e. g. 3 colonizers it does exactly that, and
> then prompts for a new build task. I have not seen it go into reserve or
> abort the plan, even though it is limited by unit-consumption-per-cp and
> runs out of material each turn (as it should).
> 
> What game did this happen in, and do you have a saved game where the bug
> can be seen?

It was in the new version of knights.g (now called red-wizard.g).  In
the new version, it is possible to capture the lairs of certain types of
monsters (goblins, kobolds, orcs) and make them churn out troops for
you.  Their productive capacity, however, is limited to a single unit
each (e.g. an orc lair can only produce orcs), so I usually set them to
build 99 units at a time.

Since the module is more than twice the size of the original knights.g,
I'll send it directly to you in a separate e-mail.

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

Many a man who thinks he's going on a maiden voyage with
a woman finds out later that it was just a shake-down cruise.

Re: Bug in acp-independent action code

[Hans Ronne]

>On Thu, 2004-06-10 at 23:38, Hans Ronne wrote:
>> This I could not reproduce. If I tell an acp-independent unit (a city in
>> the advances game) to build e. g. 3 colonizers it does exactly that, and
>> then prompts for a new build task. I have not seen it go into reserve or
>> abort the plan, even though it is limited by unit-consumption-per-cp and
>> runs out of material each turn (as it should).
>>
>> What game did this happen in, and do you have a saved game where the bug
>> can be seen?
>
>It was in the new version of knights.g (now called red-wizard.g).  In
>the new version, it is possible to capture the lairs of certain types of
>monsters (goblins, kobolds, orcs) and make them churn out troops for
>you.  Their productive capacity, however, is limited to a single unit
>each (e.g. an orc lair can only produce orcs), so I usually set them to
>build 99 units at a time.
>
>Since the module is more than twice the size of the original knights.g,
>I'll send it directly to you in a separate e-mail.

OK. I'll have a look at it.

Hans

Re: Bug in acp-independent action code

[Hans Ronne]

>Since the module is more than twice the size of the original knights.g,
>I'll send it directly to you in a separate e-mail.

Thanks. I can now reproduce the bug. It seems that acp-independent units,
at least in your game, have a 20% chance of resetting the run length to 1
for each unit that is completed. Not an easy bug to catch.

Hans

Re: Bug in acp-independent action code

[Hans Ronne]

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.

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.

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.

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

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.

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.

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.

Hans

Re: Bug in acp-independent action code

[Lincoln Peters]

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?

Re: Bug in acp-independent action code

[Eric McDonald]

On Mon, 2004-06-14 at 17:31, Hans Ronne wrote:

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

I certainly favor this option the most. The more common/unified code
that we can make, the easier it is to maintain/fix/expand.

Eric

Re: Bug in acp-independent action code

[Hans Ronne]

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

In that case something else must be wrong with your module. None of the
other games with acp-independent units use material-to-build.

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

check_build_action checks that the supply needed by build_step_consumption
is available, so if the unit-consumption-per-build tables are set
correctly, material-to-build should not be needed. If it is, we may have a
bug in the build code.

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

There is another safety valve in execute_plan that cancels the plan it
after 1000 failed attempts. And then there is code in the mplayer that will
put the unit in reserve if things don't work out. The question is how many
of these safety valves we need and where they should be. I would prefer one
only, so that errors are handled in a predictable way.

Hans

Re: Bug in acp-independent action code

[Lincoln Peters]

On Tue, 2004-06-15 at 00:14, Hans Ronne wrote:
> >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).
> 
> In that case something else must be wrong with your module. None of the
> other games with acp-independent units use material-to-build.

I didn't explain that very well.  I meant that I needed to set
material-to-build so that non-acp-independent units would function
properly.  I can try setting it up so that material-to-build *only*
affects non-acp-independent units.

> 
> >> 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.
> 
> check_build_action checks that the supply needed by build_step_consumption
> is available, so if the unit-consumption-per-build tables are set
> correctly, material-to-build should not be needed. If it is, we may have a
> bug in the build code.

Most likely, yes.

> 
> >> 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.
> 
> There is another safety valve in execute_plan that cancels the plan it
> after 1000 failed attempts. And then there is code in the mplayer that will
> put the unit in reserve if things don't work out. The question is how many
> of these safety valves we need and where they should be. I would prefer one
> only, so that errors are handled in a predictable way.

Of course, if a unit doesn't re-plan before it's re-tried and failed
1,000 times, the game will most likely be over before the unit can do
anything useful.

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

Do not underestimate the power of the Force.

New Wreck-Type Options

[Elijah Meeks]

I was waiting until I had a working framework for what
I hope is a fun little, civilization-like game that's
taking taking longer than I thought to put together,
but I wanted to thank Eric for these new wreck-type
options and the capture protection options.  They add
immeasurably to Xconq.  Bravo.


		
__________________________________
Do you Yahoo!?
Yahoo! Mail Address AutoComplete - You start. We finish.
http://promotions.yahoo.com/new_mail 

Re: New Wreck-Type Options

[Eric McDonald]

On Tue, 2004-06-15 at 22:08, Elijah Meeks wrote:
> I was waiting until I had a working framework for what
> I hope is a fun little, civilization-like game that's
> taking taking longer than I thought to put together,
> but I wanted to thank Eric for these new wreck-type
> options and the capture protection options.  They add
> immeasurably to Xconq.  Bravo.

You're quite welcome. Usefulness is the mother of invention.

With apologies to Interplay:
Xconq. For game designers. By game designers.

Eric

P.S. If anyone is using the new 'destruction-result' unit property, then
be aware that there is a subtle bug lurking in it. I encountered the bug
with it last night. It seems to work right most of the time, but after
like 40 turns of Wreckreation (being played by the AI), it did encounter
a bogus result (a nil list instead of one of the 'vanish' or 'table'
keywords). I will try to finish tracking down this bug tonight. If you
are playing Wreckreation and encounter a warning, "Unknown destruction
result encountered.", you can safely ignore it and continue playing
without ill side effects.

Re: Bug in acp-independent action code

[Hans Ronne]

>I didn't explain that very well.  I meant that I needed to set
>material-to-build so that non-acp-independent units would function
>properly.  I can try setting it up so that material-to-build *only*
>affects non-acp-independent units.

I see. But there is no reason why non-acp-independent units should require
material-to-build to be set, either. This is the relevant code in
check_build-action:

        if (totsup < um_to_build(builder->type, m)) {
           return A_ANY_NO_MATERIAL;
        }
        if (totsup <  build_step_consumption(builder->type, newunit->type,
m, newunit->cp)) {
           return A_ANY_NO_MATERIAL;
        }

build_step-consumption checks um_consumption_per_build. So you should not
need to set um_material_to_build. If this is indeed required, this is a bug
somewhere.

>> There is another safety valve in execute_plan that cancels the plan it
>> after 1000 failed attempts. And then there is code in the mplayer that will
>> put the unit in reserve if things don't work out. The question is how many
>> of these safety valves we need and where they should be. I would prefer one
>> only, so that errors are handled in a predictable way.
>
>Of course, if a unit doesn't re-plan before it's re-tried and failed
>1,000 times, the game will most likely be over before the unit can do
>anything useful.

Not really, since the game will wait for the unit to complete its 1000
failed attempts before proceeding to the nct turn, But uf many units do
this, it ill slow done the game.

Hans