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

Add DamageSource to Explodes #15304

Merged
merged 1 commit into from Jul 26, 2018

Conversation

Projects
None yet
4 participants
@MustaphaTR
Copy link
Member

MustaphaTR commented Jun 28, 2018

[10:27:27] | <MustaphaTR>  pchote, abcdefg30 , I think i already asked that but didn't get any  answer iirc: I have this change on my engine that should fix #9044, i  think seems like a obivious fix and i have that for a while now, didn't  see any regression or crash. Should i PR that to upstream, any reason  why this was e.Attacker? https://github.com/MustaphaTR/OpenRA/commit/1284f8945ad6eec3b8543d96cd07f3638b2edbed
[10:27:28] | <orabot> Title: Make actor itself the source of damage for Explodes weapon · MustaphaTR/OpenRA@1284f89 · GitHub
[10:27:30] | <orabot> Issue #9044 (open) by MustaphaTR: RA - Unit that destroyed the Demo Truck is counted as source of damage. \| http://bugs.openra.net/9044
[10:27:52] | <pchote> MustaphaTR: to attribute kills from demo trucks
[10:28:15] | <MustaphaTR> For statistics?
[10:28:18] | <pchote>  the idea being that if somebody shoots a demo truck and that then  destroys a bunch of stuff that the original attacker should get all the  credits
[10:28:29] | <pchote> IIRC the main argument was for bounties
[10:28:48] | <MustaphaTR> Well demo truck bounties are already broken.
[10:29:02] | <MustaphaTR> #14502
[10:29:02] | <orabot> Issue #14502 (open) by MustaphaTR: Dead actors can't get bounties. \| http://bugs.openra.net/14502
[10:29:16] | <pchote> tldr: bounties in general are broken ;)
[10:30:03] | <FRenzy>  (that reminds me that mines also seem to mess up with statistics, I'm  not entirely sure though and I'll have to test more in-depth)
[10:30:03] | <pchote> MustaphaTR: you can expose an enum in the trait info to set which actor should be treated as the source
[10:30:13] | <pchote> then default it to self, and override demo trucks to the killer
[10:33:12] | <MustaphaTR>  I was thiking why i fixed it in the first place, but i remember now, it  was Demo Gen's Demolitions ability which should only damage enemy  units, current setup messes up Stance checks in the weapon.
[10:33:28] | <pchote> right
@Mailaender

This comment has been minimized.

Copy link
Member

Mailaender commented Jul 10, 2018

Does this fix #9044?

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Jul 10, 2018

Does this fix #9044?

The code fixes it for regular units (at default settings) but explicitly enables the bleed behavior for demo trucks, because as the IRC log above explains, for demo trucks this bug was actually intentional.

I think we can still close #9044 after this is merged though, since this PR fixes the bug for all other units, while for demo trucks specifically it's a WONTFIX.

@pchote

pchote approved these changes Jul 26, 2018

Copy link
Member

pchote left a comment

Looks reasonable.

@pchote pchote merged commit 6073de5 into OpenRA:bleed Jul 26, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.