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 crash with dead cargo. #17016

Merged
merged 1 commit into from Sep 13, 2019

Conversation

@tovl
Copy link
Contributor

commented Aug 30, 2019

Fixes #17015
Fixes #16405
Fixes #15261

@pchote pchote added this to the Next Release milestone Aug 30, 2019
@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Think it would be better to add the dead check here in the AddFrameEndTask, because right now with this fix its possible to have a transport filled with dead actors(?).
https://github.com/OpenRA/OpenRA/blob/bleed/OpenRA.Mods.Common/Activities/RideTransport.cs#L54

@tovl tovl force-pushed the tovl:deadcargo branch from 3e3cd08 to bb2e411 Sep 6, 2019
@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

Added the check on both ends, just in case.

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Sep 6, 2019

Out of curiosity: Do we actually remove passengers dying inside the transport from blocking space?

@tovl

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

As far as I can tell, we don't.

@teinarss

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2019

Out of curiosity: Do we actually remove passengers dying inside the transport from blocking space?

With the check in place in the FrameEndTask there should be no cases where the cargo is dead, or am i missing some other case?

@abcdefg30

This comment has been minimized.

Copy link
Member

commented Sep 7, 2019

Cargo dying inside the transport. I have to retest if this is still possible to do with negative self healing.

@pchote
pchote approved these changes Sep 8, 2019
Copy link
Member

left a comment

Looks reasonable, but I haven't tested the various edge cases.

@pchote pchote added the PR: Needs +2 label Sep 8, 2019
Copy link
Member

left a comment

Cargo dying inside a transport is removed as well.

@abcdefg30 abcdefg30 merged commit 46c0b4c into OpenRA:bleed Sep 13, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.