New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAM site stays after being sold during low-power #7442

Closed
deniz1a opened this Issue Feb 8, 2015 · 10 comments

Comments

Projects
None yet
8 participants
@deniz1a
Contributor

deniz1a commented Feb 8, 2015

sam

It was targeting a plane while looking like that. I had to press sell multiple times before it was sold properly. This is in release-20141029. Might be related to #6076.

@deniz1a deniz1a changed the title from Sam site stays after being sold to SAM site stays after being sold Feb 8, 2015

@Mailaender Mailaender added the Bug label Feb 8, 2015

@Mailaender Mailaender changed the title from SAM site stays after being sold to SAM site stays after being sold during low-power Feb 8, 2015

@pevers

This comment has been minimized.

Show comment
Hide comment
@pevers

pevers Feb 23, 2015

Contributor

Reproducable:

  1. Attack SAM site from enemy player that is powered down with AIRFORCE.
  2. Sell SAM site on enemy side.

Doesn't matter if the player have low-power or that he just powered the SAM site down, effect is the same.

screenshot from 2015-02-23 20 44 27

Contributor

pevers commented Feb 23, 2015

Reproducable:

  1. Attack SAM site from enemy player that is powered down with AIRFORCE.
  2. Sell SAM site on enemy side.

Doesn't matter if the player have low-power or that he just powered the SAM site down, effect is the same.

screenshot from 2015-02-23 20 44 27

@penev92

This comment has been minimized.

Show comment
Hide comment
@penev92

penev92 Feb 23, 2015

Member

Thanks. Now that we have a reproduce-case, maybe someone can fix it :)

Member

penev92 commented Feb 23, 2015

Thanks. Now that we have a reproduce-case, maybe someone can fix it :)

@jabbink

This comment has been minimized.

Show comment
Hide comment
@jabbink

jabbink Feb 24, 2015

Contributor

This behaviour is induced by the AttackFollow Activity, which the SAM site has every tick, even when powered down.

Subsequently https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs#L74 will return early without going to the next activity (being sold).

I changed that line to return NextActivity, and selling a powered down, attacking SAM site worked fine. I am not sure what other side effects this has as I guess there is a reason why a disabled actor in AttackFollow mode should not do anything else.

Contributor

jabbink commented Feb 24, 2015

This behaviour is induced by the AttackFollow Activity, which the SAM site has every tick, even when powered down.

Subsequently https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs#L74 will return early without going to the next activity (being sold).

I changed that line to return NextActivity, and selling a powered down, attacking SAM site worked fine. I am not sure what other side effects this has as I guess there is a reason why a disabled actor in AttackFollow mode should not do anything else.

@penev92

This comment has been minimized.

Show comment
Hide comment
@penev92

penev92 Feb 24, 2015

Member

First thing that comes to mind is an EMP-ed tank continuing to chase a target after the EMP effect wears off. But you are on a good path.
Consider also clearing the activity queue when a sell order is issued.

Member

penev92 commented Feb 24, 2015

First thing that comes to mind is an EMP-ed tank continuing to chase a target after the EMP effect wears off. But you are on a good path.
Consider also clearing the activity queue when a sell order is issued.

@obrakmann

This comment has been minimized.

Show comment
Hide comment
@obrakmann

obrakmann Mar 1, 2015

Contributor

I cannot reproduce this on -playtest at all. Is this still an issue, or has it been fixed by accident?

Contributor

obrakmann commented Mar 1, 2015

I cannot reproduce this on -playtest at all. Is this still an issue, or has it been fixed by accident?

@pevers

This comment has been minimized.

Show comment
Hide comment
@pevers

pevers Mar 1, 2015

Contributor

@obrakmann Thats strange as I can still reproduce this on the latest bleed.

Contributor

pevers commented Mar 1, 2015

@obrakmann Thats strange as I can still reproduce this on the latest bleed.

@pevers

This comment has been minimized.

Show comment
Hide comment
@pevers

pevers Mar 1, 2015

Contributor

I think I found out why it isn't always repoducible. In

https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Mods.Common/Traits/Sellable.cs#L46

All the activities are cancelled before selling the building. But the animation will take some time to sell, so a new attack activity can be placed in the queue blocking the eventually sell (after the animation). I have proposed another fix in #7544,

Contributor

pevers commented Mar 1, 2015

I think I found out why it isn't always repoducible. In

https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Mods.Common/Traits/Sellable.cs#L46

All the activities are cancelled before selling the building. But the animation will take some time to sell, so a new attack activity can be placed in the queue blocking the eventually sell (after the animation). I have proposed another fix in #7544,

@chrisforbes

This comment has been minimized.

Show comment
Hide comment
@chrisforbes

chrisforbes Mar 1, 2015

Member

Selling should take the building lock. I think we shouldn't be queuing any
attack stuff if the building is locked.

  • OR -

Is this not right? It would mean that capture in progress prevents firing
too. This would have balance implications.

On Mon, Mar 2, 2015 at 7:42 AM, Peter Evers notifications@github.com
wrote:

@obrakmann https://github.com/obrakmann Thats strange as I can still
reproduce this on the latest bleed.


Reply to this email directly or view it on GitHub
#7442 (comment).

Member

chrisforbes commented Mar 1, 2015

Selling should take the building lock. I think we shouldn't be queuing any
attack stuff if the building is locked.

  • OR -

Is this not right? It would mean that capture in progress prevents firing
too. This would have balance implications.

On Mon, Mar 2, 2015 at 7:42 AM, Peter Evers notifications@github.com
wrote:

@obrakmann https://github.com/obrakmann Thats strange as I can still
reproduce this on the latest bleed.


Reply to this email directly or view it on GitHub
#7442 (comment).

@pevers

This comment has been minimized.

Show comment
Hide comment
@pevers

pevers Mar 1, 2015

Contributor

@chrisforbes I think that would indeed mean that capturing a building would immediately disable attack moves which isn't preferred in my opinion

I outputted the queue activity in this situation:

cancelled all activity on sell
queue OpenRA.Mods.Common.Traits.AttackFollow+AttackActivity
queue OpenRA.Mods.Common.Activities.Sell

So preventing attack activities on disabled buildings may fix this bug and also improve a lot on efficiency I think. I'll try that out now.

Contributor

pevers commented Mar 1, 2015

@chrisforbes I think that would indeed mean that capturing a building would immediately disable attack moves which isn't preferred in my opinion

I outputted the queue activity in this situation:

cancelled all activity on sell
queue OpenRA.Mods.Common.Traits.AttackFollow+AttackActivity
queue OpenRA.Mods.Common.Activities.Sell

So preventing attack activities on disabled buildings may fix this bug and also improve a lot on efficiency I think. I'll try that out now.

@Phrohdoh

This comment has been minimized.

Show comment
Hide comment
@Phrohdoh

Phrohdoh Mar 12, 2015

Member

Closed with #7544.

Member

Phrohdoh commented Mar 12, 2015

Closed with #7544.

@Phrohdoh Phrohdoh closed this Mar 12, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment