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

Conditional TakeCover #15809

Merged
merged 1 commit into from Feb 24, 2019

Conversation

@TheChosenEvilOne
Copy link
Contributor

commented Nov 9, 2018

Made TakeCover conditional
PauseOnCondition stops remainingProneTime from decreasing
RequiresCondition sets remainingProneTime to 0
both stop the trait from triggering on damage

Closes #15167

@abcdefg30
Copy link
Member

left a comment

What was the issue with just letting TakeCover(Info) implement PausableConditionalTrait(Info)?

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

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2018

This would close #15167

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from afc6124 to b7e744c Nov 9, 2018

@pchote
Copy link
Member

left a comment

I'd prefer it if we could start with making Turreted conditional so that we don't need to duplicate all the plumbing here.

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from b7e744c to 8226e31 Nov 10, 2018

@TheChosenEvilOne

This comment has been minimized.

Copy link
Contributor Author

commented Nov 10, 2018

depends on #15811

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from 8226e31 to 6322518 Nov 10, 2018

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from 6322518 to d680473 Nov 21, 2018

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from d680473 to 5b322f8 Dec 19, 2018

@MustaphaTR
Copy link
Member

left a comment

Testcase works so 👍 . Tho comments below apply.

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

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from 5b322f8 to 4191737 Dec 19, 2018

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from 4191737 to 07326f3 Dec 19, 2018

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from 07326f3 to cd1c35d Dec 23, 2018

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from cd1c35d to 9ed74c3 Dec 26, 2018

@reaperrr

This comment has been minimized.

Copy link
Contributor

commented Feb 22, 2019

Looks good to me and testcase works.
LGTM once @MustaphaTR reaffirms his +1 and testcase is removed.

@TheChosenEvilOne TheChosenEvilOne force-pushed the TheChosenEvilOne:takecover-conditional branch from 9ed74c3 to d6104d2 Feb 22, 2019

@TheChosenEvilOne

This comment has been minimized.

Copy link
Contributor Author

commented Feb 22, 2019

removed testcase

@MustaphaTR
Copy link
Member

left a comment

👍 Looks good to me.

@reaperrr reaperrr merged commit 30103da into OpenRA:bleed Feb 24, 2019

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
Projects
None yet
6 participants
You can’t perform that action at this time.