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 production of dummy actors from producers without Exit trait #16416

Merged
merged 4 commits into from Apr 22, 2019

Conversation

@MustaphaTR
Copy link
Member

commented Apr 15, 2019

The Conditional Exit PR broke this because it is now passing exit.Info instead of exit, which when exit is null is crashing. DoProduction has necessary checks in it that passing null there do not crash the game.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

DoProduction has necessary checks in it that passing null there do not crash the game.

It only checks for self.OccupiesSpace != null, so it technically works (because Produce will only give a null Exit if OccupiesSpace is null) but is extremely fragile.

Can you please add an explicit exitInfo != null check to that case in DoProduction?

@MustaphaTR

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2019

Updated.

@pchote

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

I tested this by removing the Exit trait from D2k's conyards, but the dummy upgrade actors stay on "Ready" rather than being produced.

@MustaphaTR

This comment has been minimized.

Copy link
Member Author

commented Apr 21, 2019

The OccupiesSpace check is for the producer not producee i just noticed. I think the best course of action would be to change that because i don't think it would work fine if you try to build a real actor from player actor.

@pchote
pchote approved these changes Apr 22, 2019

@pchote pchote added the PR: Needs +2 label Apr 22, 2019

@reaperrr reaperrr merged commit 2463b2e into OpenRA:bleed Apr 22, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MustaphaTR MustaphaTR deleted the MustaphaTR:dummy-production branch Apr 22, 2019

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