[PATCH] Spartan females won't chase ranged units (Infinite Chase Bug)

[Doménique]

As proposed the Spartan female citizen are to have defensive abilities. Currently they have none, despite their higher cost of 60 (versus 50) food units. So this patch gives these female citizen an attack ability, while setting their default stance to defensive to avoid undesirable/aggressive behaviour. Everything else remains unchanged, so a bot shouldn't mistake the females for potential soldiers and send them to war.

As for the attack ability itself, I could not find a predefined attack style in the design documents. I decided that either a small melee attack (e.g., a knife) or a small ranged attack (e.g., slinging stones that are usually available anywhere) would be most realistic. Because a melee attack requires close range, which is dangerous without any significant armour against true soldiers, I opted for the ranged attack. Compared to a soldier the range is reduced, as is the force of the attack (7 versus original of 17). A group of female citizens can now deal with a small amount of competition, as is the intend.

There is one strange problem with this patch. Because the female default stance is 'passive,' I set it to 'defensive.' A passive citizen would simply stand and wait to meet their fate, or flee. The attack ability is never triggered, so setting the stance to defensive gives the intended behaviour. However, doing so results in an error via the console:

WARNING: JavaScript warning: simulation/components/UnitAI.js line 2681
reference to undefined property this.heldPosition
ERROR: JavaScript error: simulation/components/UnitAI.js line 2681
TypeError: this.heldPosition is undefined
  (73,37,"Ranged")@simulation/components/UnitAI.js:2681
  ([object Array])@simulation/components/UnitAI.js:2744
  ([object Array])@simulation/components/UnitAI.js:2770
  ([object Array])@simulation/components/UnitAI.js:3641
  ([object Object])@simulation/components/UnitAI.js:775
  ([object Object],[object Object])@simulation/helpers/FSM.js:274
  ([object Object])@simulation/components/UnitAI.js:2334
ERROR: Script message handler OnRangeUpdate failed

Manually setting the stance to defensive does work without issues. I cannot figure out whether this is a problem with a support unit which suddenly gets ranged attack capabilities, or that it uncovers a bug elsewhere. Just to be complete, setting the stance to aggressive in the template file also works (but does not have the effect I was looking for). Anyway, this means the patch and/or something else needs a little work before it can be applied without problems.

[Doménique]

Patch that gives Spartan female a small ranged attack ability

[fcxSanya]

There is one strange problem with this patch. <...>

Looked at this quickly, so I may be wrong somewhere, but this is what I found: heldPosition property is set by SetHeldPosition function, which is called by SwitchToStance (which is like a wrapper to SetStance with additional functionality), I think this is what happening when you change stances manually. But in Init there is SetStance itself is used (this.SetStance(this.template.DefaultStance)), so it is not calling SetHeldPosition. I propose to try to replace SetStance to SwitchToStance in Init (or to find what is the reason to don't use it there).

[Doménique]

I think I have found the reason for the errors by jotting down this long text :) First of all, the goal for the female citizens is to defend as best as they can, or flee. In principle, this is what the defensive stance does.

As the errors and fcxSanya indicate, this.heldPosition (an object with elements x,y) is undefined, causing trouble. This would be set if SwitchToStance could be used within Init. Doing so causes all kinds of errors, I guess because within Init a lot of things are not yet defined. One such thing unavailable is the current position, normally retrieved in SwitchToStance. So even if SwitchToStance could be used, the call to SetHeldPosition cannot be given valid position arguments. However, just temporarily setting the held position (this.SetHeldPosition(0,0)) in Init does make the errors go away. The behaviour is still not what I want, because units remain essentially not defensive.

So why is this.heldPosition troublesome for the defensive stance? All the other stances give no trouble when set as default (even though up till now only aggressive and passive have been used as defaults; passive for support units, aggressive for everything else). The defensive stance differs from the other stances in one way: it is the only stance for which the property respondHoldGround (in UnitAI.js) is set to true. Setting it to false makes the errors go away, but that's not the goal.

To summarise, I assume the problem lies somewhere in wanting to hold ground and needing the position to hold. It is the only time this.heldPosition is used within the UnitAI code. So this position refers to either the position the unit itself decided to hold (dynamically), or as it currently is set up, the position where the unit is at the moment the player decides to give the defensive stance command. At the start of a game this is thus not yet set, hence the errors once the need to hold position arises (some nearby swordsmen in my test map). A test with no nearby enemies confirms this.

So the solution is to set this.heldPosition appropriately. Initially it can be set to x=0, y=0, but it needs dynamic updating at some point. I would propose to set the position to hold to a (move command) destination, farm field, bushes, or current position if at rest. I have not implemented such a thing yet, perhaps someone has a better idea?

[fcxSanya]

Replying to dvangennip:

Initially it can be set to x=0, y=0 <...>

This sounds not any better than undefined (even worse since it is misleading and can cause incorrect behaviour).

Another random idea: modify that specific place where this.heldPosition is used (UnitAI.js:2681) or general GetHeldPosition function (and use it in UnitAI.js:2681 then) to check if this.heldPosition is defined and use the default value (entity's current position) if it is not (which I expect will be correct behaviour in this case). There is one place with check whether this.heldPosition is defined (in WalkToHeldPosition), it is used to "Return to our original position" after a combat, it should not be broken by proposed change.

Still it needs to be evaluated more and tested.

[fcxSanya]

Replying to fcxSanya:

Another random idea: modify that specific place where this.heldPosition is used (UnitAI.js:2681) or general GetHeldPosition function (and use it in UnitAI.js:2681 then) to check if this.heldPosition is defined and use the default value (entity's current position) if it is not (which I expect will be correct behaviour in this case).

Attached a simple patch for this. When it is applied, your patch looks working without errors.

Michael added different actor code for Spartan female melee attack in r12792. I will point him to this ticket, maybe he will have some design-related comments.

[michael]

I think the proper stance would probably be "Stand Ground" and then we make them unable to switch to a different, more aggressive stance. Thoughts?

[Doménique]

Replying to michael:

I think the proper stance would probably be "Stand Ground" and then we make them unable to switch to a different, more aggressive stance. Thoughts?

I agree, support units should definitely not be able to scale up to aggressive/offensive stances. As for the usefulness of this ticket, I've seen the template for female support citizens has been given a melee attack, and the Spartan female actor includes a knife to do so :) So I made a second version of my patch to reflect these changes. The proposed changes/additions are rather small now. I'm not sure how I can disable the more offensive stances for an entity? The entity docs seem not to cover this, perhaps because it's not possible yet?

P.S.1. The entity docs refer to the correct UnitAI->DefaultStance option as stand while it should be standground. Perhaps this could be corrected.

P.S.2. Sorry for the late reply, I had trouble to log in to trac (not sure why).

[Doménique]

Patch that enables Spartan females to defend themselves

[fcxSanya]

Replying to dvangennip:

P.S.2. Sorry for the late reply, I had trouble to log in to trac (not sure why).

We have issues due to a Trac bug from time to time. Philip fixes them when someone asks him.

[Kieran P]

Partially fixed by [12924] (committed both patches). According to Michael, additional changes may be needed to fix this. Moving to Alpha 13.

[michael]

Committed this to SVN. It still isn't quite right, because the Spartan female doesn't fight back automatically--she has to be ordered to do so manually by the player. 'Standground' should be the unitAI default stance she has, based on how standground works for other units, but it doesn't seem to be working. And changing her unitAI default stance to 'Defensive' gives undesirable aggressive behavior. We only want them to fight back when attacked (while most other females run away).

[sanderd17]

I don't know what changed, but it seems to work now.

[sanderd17]

As it works now (I don't know when it was fixed), we can close this issue.

[Kieran P]

I disagree. The original bug may be fixed, but they intended behaviour isn't. Look at Michaels comment (comment 10). Things still aren't working as desired. Don't be so hasty to "CLOZ AL DA BUGZ!" :-)

[sanderd17]

As you can read on IRC, I discussed this with historic_bruno. The thing mentioned by Michael is fixed (confirmed by me, hbruno and scythetwirler). Females fight back automatically.

But I was going to re-open it because there was a new issue. Because they won't chase ranged units. But then again, this would require us to fix the infinite chase first.

[elexis]

Pushing due to lack of activity.

[elexis]

Backlogging due to lack of progress.

[Imarok]

Move tickets to Simulation as UI & Simulation got some sub components.