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

Fix enabled trait checks #14477

Merged
merged 2 commits into from Dec 11, 2017

Conversation

Projects
None yet
5 participants
@RoosterDragon
Member

RoosterDragon commented Dec 7, 2017

Fixes some bad uses of Exts.IsTraitEnabled.

Fixes some of #14445.

@penev92 penev92 added this to the Next release milestone Dec 7, 2017

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 9, 2017

Member

This doesn't seem to work, am I missing something obvious?

My testcase is:

diff --git a/mods/d2k/rules/infantry.yaml b/mods/d2k/rules/infantry.yaml
index b83312be0b..36622c31a1 100644
--- a/mods/d2k/rules/infantry.yaml
+++ b/mods/d2k/rules/infantry.yaml
@@ -98,7 +98,11 @@ thumper:
        Mobile:
                Speed: 43
                RequiresCondition: undeployed
+       GrantConditionOnTerrain:
+               Condition: enable-deploy
+               TerrainTypes: Sand, Spice, Dune, SpiceSand
        GrantConditionOnDeploy:
+               RequiresCondition: enable-deploy
                DeployedCondition: deployed
                UndeployedCondition: undeployed
                Facing: 128

which should disable the thumper deploy when it is not on sand. The no-deploy cursor shows as expected, so the problem appears to be with the commandbar changes in this PR. Note that this depends on #14482 to fix the invisible thumper regression. I plan on PRing this testcase as a polish fix once this and #14482 are merged.

Member

pchote commented Dec 9, 2017

This doesn't seem to work, am I missing something obvious?

My testcase is:

diff --git a/mods/d2k/rules/infantry.yaml b/mods/d2k/rules/infantry.yaml
index b83312be0b..36622c31a1 100644
--- a/mods/d2k/rules/infantry.yaml
+++ b/mods/d2k/rules/infantry.yaml
@@ -98,7 +98,11 @@ thumper:
        Mobile:
                Speed: 43
                RequiresCondition: undeployed
+       GrantConditionOnTerrain:
+               Condition: enable-deploy
+               TerrainTypes: Sand, Spice, Dune, SpiceSand
        GrantConditionOnDeploy:
+               RequiresCondition: enable-deploy
                DeployedCondition: deployed
                UndeployedCondition: undeployed
                Facing: 128

which should disable the thumper deploy when it is not on sand. The no-deploy cursor shows as expected, so the problem appears to be with the commandbar changes in this PR. Note that this depends on #14482 to fix the invisible thumper regression. I plan on PRing this testcase as a polish fix once this and #14482 are merged.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Dec 10, 2017

Member

Last time i checked GrantConditionOnDeploy wasn't conditionable.

Member

MustaphaTR commented Dec 10, 2017

Last time i checked GrantConditionOnDeploy wasn't conditionable.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Dec 10, 2017

Member

Ah, Derp. On closer inspection we don't have any conditional IIssueDeployOrder traits.

This works as expected when I make one conditional, so 👍

Member

pchote commented Dec 10, 2017

Ah, Derp. On closer inspection we don't have any conditional IIssueDeployOrder traits.

This works as expected when I make one conditional, so 👍

@pchote

pchote approved these changes Dec 10, 2017

@pchote pchote added the PR: Needs +2 label Dec 10, 2017

@abcdefg30 abcdefg30 merged commit 70ffa99 into OpenRA:bleed Dec 11, 2017

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.

Show comment
Hide comment
@abcdefg30
Member

abcdefg30 commented Dec 11, 2017

@RoosterDragon RoosterDragon deleted the RoosterDragon:fix-enabled-trait-checks branch Dec 11, 2017

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