Idle military people

Bug #530646 reported by SirVer
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Low
Raul Ferriz

Bug Description

Full description here, only tracked here to be markes as show stopper for rc1

https://sourceforge.net/tracker/?func=detail&aid=2961814&group_id=40163&atid=427221

Tags: military
SirVer (sirver)
Changed in widelands:
status: New → Confirmed
importance: Undecided → Critical
milestone: none → rc1
Revision history for this message
Timowi (timo-wingender) wrote :

This bug was caused by #531037 A pop_task was done which caused the soldiers to go home. With a militarysite it worked. But with a headquarter the soldiers got lost and were not removed form the global worker list.
So there seems to be some wrong handling on this case. But I think this is no problem. The AI work again so it will no be likely to attack a headquarter with soldiers and the defend task work now. Close this bug?

Revision history for this message
SirVer (sirver) wrote :

I can't follow Timowi. If this bug can't/won't reappear in game, close it. If not, leave it open.

Revision history for this message
Timowi (timo-wingender) wrote :

It wont't appear in the same situation. Even the situation will not appear in the game any more. But the bug is there.

I try to explain it again.

This bug is a difference between soldiers from a militarysite and a headquarter(and i think from a warehouse too). Bug #531037 only triggered this one. Bug #531037 was a wrong pop_task inside task_defense. This causes a different behavior for the "two types of soldiers": Soldiers from a militarysite return to their building. Soldiers without a militarysite disappear but exist in the worker list of their building.

I had a look at the code again and I think it's the following problem:
A soldier launched from a militarysite has a task "buildingwork" and then task "attack" or "defense". Soldiers launched from a headquarter (or warehouse?) only have task "defense". If pop_task is called in the first case task "buildingwork" takes the solider home (or to a warehouse). In the second case the solider is forever at his last position without any task (But he is a worker of his building).

Either calling pop_task without being home or something with the sending of soldiers from a headquarter is wrong. I am not completely familiar with the task system but I think pop_task should no be called if the worker is not at his building.

I can imagine at least on situation where this could happen: If you attack near a headquarter and the headquarter disappears while some soldiers defend it they would disappear because of this bug.

Timowi (timo-wingender)
Changed in widelands:
importance: Critical → Low
Revision history for this message
Nicolai Hähnle (nha) wrote : Re: [Widelands-dev] [Bug 530646] Re: Idle military people

Am Monday 08 March 2010 11:10:33 schrieb Timowi:
> I had a look at the code again and I think it's the following problem:
> A soldier launched from a militarysite has a task "buildingwork" and then
> task "attack" or "defense". Soldiers launched from a headquarter (or
> warehouse?) only have task "defense". If pop_task is called in the first
> case task "buildingwork" takes the solider home (or to a warehouse). In the
> second case the solider is forever at his last position without any task
> (But he is a worker of his building).
>
> Either calling pop_task without being home or something with the sending
> of soldiers from a headquarter is wrong. I am not completely familiar
> with the task system but I think pop_task should no be called if the
> worker is not at his building.

Keep in mind that's it's been a long time since I was really deep in that
code. However: The case of an empty task stack is supposed to be handled by
init_auto_task. Unless the soldier has 0 hit points left, this should be
handled by Worker::init_auto_task which should start the gowarehouse task.
However, it seems that the gowarehouse task then incorporates the worker
without checking whether it is actually at the warehouse's position. That is
bound to cause problems later on.

I suggest to add a check in gowarehouse_update which, instead of incorporating
the worker, will zero out her location if she's not actually at the warehouse
position. What do you think?

Revision history for this message
Nicolai Hähnle (nha) wrote :

Am Monday 08 March 2010 22:32:41 schrieb Nicolai Hähnle:
> Am Monday 08 March 2010 11:10:33 schrieb Timowi:
> > I had a look at the code again and I think it's the following problem:
> > A soldier launched from a militarysite has a task "buildingwork" and then
> > task "attack" or "defense". Soldiers launched from a headquarter (or
> > warehouse?) only have task "defense". If pop_task is called in the first
> > case task "buildingwork" takes the solider home (or to a warehouse). In
> > the second case the solider is forever at his last position without any
> > task (But he is a worker of his building).
> >
> > Either calling pop_task without being home or something with the sending
> > of soldiers from a headquarter is wrong. I am not completely familiar
> > with the task system but I think pop_task should no be called if the
> > worker is not at his building.
>
> Keep in mind that's it's been a long time since I was really deep in that
> code. However: The case of an empty task stack is supposed to be handled by
> init_auto_task. Unless the soldier has 0 hit points left, this should be
> handled by Worker::init_auto_task which should start the gowarehouse task.
> However, it seems that the gowarehouse task then incorporates the worker
> without checking whether it is actually at the warehouse's position. That
> is bound to cause problems later on.
>
> I suggest to add a check in gowarehouse_update which, instead of
> incorporating the worker, will zero out her location if she's not actually
> at the warehouse position. What do you think?

Come to think of it, just pushing a return to building task should be better
than zeroing the location.

Revision history for this message
SirVer (sirver) wrote :

I set this to triaged since a suitable solution was proposed. Someone can start on it then.

Changed in widelands:
status: Confirmed → Triaged
Revision history for this message
Raul Ferriz (raul.ferriz) wrote :

Related to this bug:
There are some invisible defenders on warehouse baseflag, I am trying to find why are they invisible and frozen. But I don't have a lot of time to investigate, so progress is slow.

O the other hand, I don't see why there is a problem with warehouses and militarysites with defending soldiers. When a defender must go home, because injuries or because there are not anymore defenders, defense task should move soldier to baseflag and building location.
Can anyone enlight me?

Revision history for this message
Timowi (timo-wingender) wrote :

I've seen on normal militarysites that some defenders come out and get back in just a they are at the flag. But they come out again a bit later. So it's hardly to see. I think it's a problem with the path finding. I think the task defense do something similar (as the pop_task) if they don't find a way. The soldiers who come out first seem to prevent following soldiers for finding a path on the first try. So they break the task.

The problem are not the cases where the task defense(/attack?) takes care of taking them home but the cases where the task is pop without the soldier being home. Like in Bug #531037: There were a pop_task() called without the soldier being at home. Possible that there are other places where this happens.

Revision history for this message
Raul Ferriz (raul.ferriz) wrote :

Mmm ... then pop_task on attack/defend task only should be called:
a) when home building is destroyed / bulldozed
b) when attacker/defender is on home
c) when attacker is on new conquered home
d) when a fatal fail is acheived (for example when soldier can not find a path to home).

Now, I pop_task also is called:
a) when defender could not find a way to attacker soldier.
b) when attacker can not reach attacking building baseflag.

So the last two cases should be fixed.
a) First case simply seek another soldier, if no soldiers can be achieved, then step to our home and check again (start_task_move home only with a single step).
b) Return home and cancel attack when soldier is inside building.

I will fix them as I have time, but until late this night I think I will not be able to fix them.

Happy hacking!

Changed in widelands:
status: Triaged → In Progress
assignee: nobody → raul.ferriz (raul.ferriz)
Revision history for this message
Raul Ferriz (raul.ferriz) wrote :

I have done that modifications, and on my tests seems that this is not fixed.
But I found the reason for defenders disapearing, and fixed it. The fix for preventing a crash when player bulldozes a being attacked building (with defender at baseflag) was the responsible. I changed that code, to only check for location of building, and not also for the immovable at soldier's position.
I will commit and push all changes, because I think that changes introduced, seeking another attacker when first wasn't able to be reached is a good improvement.

I need that someone test this, I do not have enough time to test very hardly, but on submitted savegame that is fixed.

Note: That fix PREVENTS dissaperaing of soldiers, but on savegames that soldiers are frozen, I did not found a way to easily fix this. I think that asking checking that all invisible soldiers should be at same position that his home will be enough, when this is not true, then change soldiers position to put inside home building.

Changed in widelands:
status: In Progress → Fix Committed
tags: added: military
Revision history for this message
Raul Ferriz (raul.ferriz) wrote :

Fixed on trunk, revision 5012

Revision history for this message
SirVer (sirver) wrote :

Releasd in build15-rc1

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.