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

Move Warheads from the engine to Mods.Common #8635

Merged
merged 4 commits into from
Jul 7, 2015

Conversation

penev92
Copy link
Member

@penev92 penev92 commented Jul 4, 2015

Introduces IWarhead to enable moving Warhead and DamageWarhead outside of the engine and into OpenRA.Mods.Common.Warheads, where they belong.
Split into several commits to make reviewing easier.
This is the last step before Armor can be moved to Mods.Common and made upgradable.

@@ -40,7 +41,8 @@ public void Killed(Actor self, AttackInfo e)
if (self.World.SharedRandom.Next(100) > info.Chance)
return;

if (info.DeathType != null && e.Warhead != null && !info.DeathType.Intersect(e.Warhead.DamageTypes).Any())
var warhead = e.Warhead as DamageWarhead;
if (info.DeathType != null && e.Warhead != null && !info.DeathType.Intersect(warhead.DamageTypes).Any())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This (and the other code like it) need to be checking warhead != null to avoid a NRE if e.Warhead isn't a DamageWarhead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the warhead that killed the actor, so how can it not have dealt damage?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"this won't happen in practice" is no excuse for incorrect code :)

@Mailaender
Copy link
Member

Everything still shoots, gets damaged, spawns visceroids and deteriorate without concrete. ✅

@abcdefg30
Copy link
Member

Needs a rebase.

@penev92
Copy link
Member Author

penev92 commented Jul 6, 2015

Rebased.

@reaperrr
Copy link
Contributor

reaperrr commented Jul 7, 2015

Code looks good to me and warheads still work (including special cases like spawning visceroid and changing owner).
👍

reaperrr added a commit that referenced this pull request Jul 7, 2015
Move Warheads from the engine to Mods.Common
@reaperrr reaperrr merged commit bbf9596 into OpenRA:bleed Jul 7, 2015
@reaperrr
Copy link
Contributor

reaperrr commented Jul 7, 2015

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

Successfully merging this pull request may close these issues.

5 participants