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 bogus order serialization. #14537

Merged
merged 3 commits into from Dec 21, 2017

Conversation

Projects
None yet
4 participants
@pchote
Member

pchote commented Dec 17, 2017

Fixes #14382 and applies some much needed code quality improvements to Order.Serialize.

The real fix and only behaviour change is 7220bb5. When the target actor is dead we have .Type == Invalid and .SerializableType == Actor which then corrupts the order deserialization.

I suggest reviewing by commit and with ?w=1. The combined diff view makes this look much worse than it really is.

@pchote pchote added this to the Next release milestone Dec 17, 2017

@GraionDilach

Code looks okay to me.

image
Also I can get results like ^on it while on current bleed I get a crash on every second run atleast.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 19, 2017

Member

Needs a rebase now.

Member

abcdefg30 commented Dec 19, 2017

Needs a rebase now.

@abcdefg30

Looks reasonable to me otherwise.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 21, 2017

Member

Rebased.

Member

pchote commented Dec 21, 2017

Rebased.

@abcdefg30 abcdefg30 merged commit ec4cf06 into OpenRA:bleed Dec 21, 2017

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.

Show comment
Hide comment
@abcdefg30
Member

abcdefg30 commented Dec 21, 2017

@pchote pchote deleted the pchote:fix-order-serialization branch Apr 28, 2018

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