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

Properly end capturing during an owner change #15643

Merged
merged 1 commit into from Oct 6, 2018

Conversation

Projects
None yet
4 participants
@abcdefg30
Member

abcdefg30 commented Sep 25, 2018

Fixes the capturing process not properly ending like seen in this image:
capturing

Reproduction case: Capture an oil derrick that is owned by another player. That other play surrenders while you attempt to capture their oil derrick. The ownership changes back to "neutral" and the capturing activity is cancelled. However, the bar still progresses.

The fix: As soon as ownership returns to "neutral", the actor's generation is increased which means the target cached in ExternalCapturesActor becomes invalid. However, because we check if the type is still an Actor we never reach EndCapture on ExternalCapturable.
I changed ExternalCapturable to cache the Building trait so we no longer risk a crash by attempting to query the trait of a dead actor, so that the check in ExternalCapturesActor can be removed.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Sep 25, 2018

Contributor

OpenRA.Mods.Common/Traits/ExternalCapturable.cs(57,18): error CS0414: Warning as Error: The private field `OpenRA.Mods.Common.Traits.ExternalCapturable.self' is assigned but its value is never used

Contributor

reaperrr commented Sep 25, 2018

OpenRA.Mods.Common/Traits/ExternalCapturable.cs(57,18): error CS0414: Warning as Error: The private field `OpenRA.Mods.Common.Traits.ExternalCapturable.self' is assigned but its value is never used

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Sep 25, 2018

Member

Ops, fixed.

Member

abcdefg30 commented Sep 25, 2018

Ops, fixed.

@MustaphaTR

Works as described.

@pchote

pchote approved these changes Oct 6, 2018

@pchote pchote merged commit 01d340d into OpenRA:bleed Oct 6, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment