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

Remove WithPermanentInjury #17264

Merged
merged 3 commits into from Jan 16, 2020
Merged

Remove WithPermanentInjury #17264

merged 3 commits into from Jan 16, 2020

Conversation

@reaperrr
Copy link
Contributor

reaperrr commented Oct 21, 2019

Although I took a slightly different approach than suggested for TS cyborgs, I think this closes #17261.

IsTraitDisabled now also disables the speed/damage modifiers on TakeCover, plus the trait makes infantry go prone permanently if a negative duration is set (this can also be triggered without damage by simply enabling the trait with no DamageTriggers defined).

Note: I don't have the motivation (and spare time) for a more sophisticated approach, so please no scope-creeping or requests for a larger TakeCover/*Multiplier trait refactor or anything of that sort.
If this approach is rejected for some reason, it would most likely just mean no update from me on this matter for at least another year or so, as I have other priorities and only took this on as it seemed fairly simple.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Dec 30, 2019

#17522 was merged.

@reaperrr reaperrr force-pushed the reaperrr:remove-wpi branch from 283c6fe to 3119901 Dec 30, 2019
@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Dec 30, 2019

Rebased.

Copy link
Member

pchote left a comment

LGTM, just a couple of minor points:

[FieldLoader.Require]
[Desc("Damage types that trigger prone state. Defined on the warheads.")]
[Desc("Damage types that trigger prone state. Defined on the warheads.",
"If Duration is negative (permanent), you can leave this empty to trigger prone state immediately.")]

This comment has been minimized.

Copy link
@pchote

pchote Jan 11, 2020

Member

What is our use case for immediate permanent prone?

This comment has been minimized.

Copy link
@reaperrr

reaperrr Jan 12, 2020

Author Contributor

If a modder wants to trigger permanent prone state through other means than damage (deploy, for example).

OpenRA.Mods.Common/Traits/Infantry/TakeCover.cs Outdated Show resolved Hide resolved
@reaperrr

This comment has been minimized.

Copy link
Contributor Author

reaperrr commented Jan 12, 2020

Updated (and rebased).

Copy link
Member

pchote left a comment

Confirming that my 👍 still applies.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Jan 13, 2020

Needs a rebase again.

reaperrr added 3 commits Oct 21, 2019
Negative ProneTime now activates prone state
permanently as soon as the trait is enabled.
This is more in line with original behavior.
Also allows us to remove WithPermanentInjury.
@reaperrr reaperrr force-pushed the reaperrr:remove-wpi branch from 9342e18 to d798eaf Jan 16, 2020
@abcdefg30 abcdefg30 merged commit aa63696 into OpenRA:bleed Jan 16, 2020
2 checks passed
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.

Copy link
Member

abcdefg30 commented Jan 16, 2020

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

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.