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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer check Carryable IsInWorld when Carryall is killed. #15030

Merged
merged 1 commit into from Apr 10, 2018

Conversation

Projects
None yet
3 participants
@JordanBergin
Copy link
Contributor

JordanBergin commented Apr 8, 2018

This PR concerns issue #14336, it seems the Carryable in a Carryall is always going to have IsInWorld == false since it's removed from the world when it's loaded, so Kill isn't called on the Carryable when the Carryall dies.

I have removed checking IsInWorld when the Carryall dies (I'm fairly confident this is the correct thing to do, as a Cargo doesn't perform this check when it dies and will call Kill on its passengers) and you can now build a Mammoth Mk II again if it's destroyed in a CarryAll.

I assume this would have also been affecting other things that are notified on kill such as exp gain and scores at the end of the game.

P.S. I'm new here (OpenRA, Github, and C#) so any advice/criticism is welcome! 馃槃

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Apr 9, 2018

Looks good. Only one thing I noticed is that the MKII seems to be killed at the location it was lifted off ground (flying debris there).

@JordanBergin

This comment has been minimized.

Copy link
Contributor Author

JordanBergin commented Apr 9, 2018

Good spot, I'll take a look at that this evening.

@JordanBergin

This comment has been minimized.

Copy link
Contributor Author

JordanBergin commented Apr 9, 2018

Edit - sorted my mess, ignore this comment :)

@JordanBergin JordanBergin force-pushed the JordanBergin:CarryAllDeath branch from c2dddfc to ddec89e Apr 9, 2018

@JordanBergin

This comment has been minimized.

Copy link
Contributor Author

JordanBergin commented Apr 9, 2018

Following the example in INotifyKilled.Killed in the Cargo class I've added the logic to ensure the position of the Carryable reflects that of the Carryall when it dies.

Checked the original fix is still working and the debris of the Carryable now flies off from the correct location when the Carryall is killed.

@abcdefg30
Copy link
Member

abcdefg30 left a comment

Looks good to me. 馃憤 Can you squash the commits, please?

@JordanBergin JordanBergin force-pushed the JordanBergin:CarryAllDeath branch 2 times, most recently from db30fa8 to 0badd54 Apr 9, 2018

No longer check Carryable IsInWorld when Carryall is killed. Also the鈥
鈥 Carryable's position is updated to Carryall's position when the Carryall is killed.

@JordanBergin JordanBergin force-pushed the JordanBergin:CarryAllDeath branch from 0badd54 to 40f853c Apr 9, 2018

@JordanBergin

This comment has been minimized.

Copy link
Contributor Author

JordanBergin commented Apr 9, 2018

Squashed (I think!). Cheers 馃槂

@Smittytron
Copy link
Contributor

Smittytron left a comment

Works

@abcdefg30 abcdefg30 merged commit 0226c06 into OpenRA:bleed Apr 10, 2018

2 checks passed

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

@JordanBergin JordanBergin deleted the JordanBergin:CarryAllDeath branch Apr 10, 2018

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Apr 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.