Skip to content
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

Fix resupply anim continuing if docked actor dies during resupply #16788

Merged
merged 1 commit into from Jul 18, 2019

Conversation

@reaperrr
Copy link
Contributor

commented Jul 16, 2019

Fixes #16785.

Resetting repairing/rearming at the end of every tick should be fine, for as long as the actor being resupplied is still alive and resupplying, they should be updated before the next ITick.

@fruestueck
Copy link
Contributor

left a comment

Tested both mods.

@teinarss
Copy link
Contributor

left a comment

LGTM

@pchote

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

ResupplyTick gives the actor that is being resupplied, so wouldn't it be cleaner to track that and check if it has died?

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

commented Jul 18, 2019

ResupplyTick gives the actor that is being resupplied, so wouldn't it be cleaner to track that and check if it has died?

The problem is that activities tick before traits. Example:
Tick 100: during activity tick (ResupplyTick), actor is still alive
Tick 100: it later dies during trait ITick
Tick 101: as the actor already died last tick, ResupplyTick won't tick again

So during Tick 100 ResupplyTick would still tell the host that the resupplying actor is alive, while during Tick 101 ResupplyTick won't even tick anymore and therefore not be able to tell the host that the actor died.

I guess I could cache the actor being repaired if that's what you meant, but then I'd have to add a null check to prevent crashes and null the cache when it dies, which doesn't sound much cleaner to me.

@pchote

This comment has been minimized.

Copy link
Member

commented Jul 18, 2019

Ok. I expect this trait to be rewritten as part of the docking support anyway, so its not worth spending a lot of time going over what-ifs here.

@pchote
pchote approved these changes Jul 18, 2019

@pchote pchote merged commit 83a607e into OpenRA:bleed Jul 18, 2019

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
Projects
None yet
4 participants
You can’t perform that action at this time.