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 Creates/RevealsShroudMultiplier #15707

Merged
merged 2 commits into from Nov 10, 2018

Conversation

Projects
None yet
4 participants
@MustaphaTR
Copy link
Member

MustaphaTR commented Oct 14, 2018

Sole Survivor's Range Crates increased units' vision ranges too, considering all the units with 10 different range for 10 level, it is really unnecessarily long thing to code if i were to use conditions on RevealShroud traits. Multipliers can do the job way easier.

TESTCASE makes RA Infantry get Sight bonus when you have radar and adds deploying Gap Generator to effect a larger area.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Oct 14, 2018

Just throwing in an idea here, not a request (yet): In the longer run, I think it might be better to convert GainsExperience's Conditions to stacked conditions and make the modifier/multiplier traits (not just this one, also speed, firepower etc.) define an array of values and cycle through them via stacked conditions, to reduce the number of traits necessary for veterancy.

Show resolved Hide resolved OpenRA.Mods.Common/Traits/CreatesShroud.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Traits/CreatesShroud.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Traits/RevealsShroud.cs Outdated

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:shroud-multipliers branch 2 times, most recently from d25fba1 to 025fbaa Oct 16, 2018

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 16, 2018

Updated.

@abcdefg30
Copy link
Member

abcdefg30 left a comment

The testcases don't work on my end.

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 18, 2018

First code was working, but i guess i messed something up trying to do your requests, will take a look what is wrong.

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:shroud-multipliers branch 2 times, most recently from af7dce8 to dfdd734 Oct 18, 2018

Show resolved Hide resolved OpenRA.Mods.Common/Traits/CreatesShroud.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Traits/CreatesShroud.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Traits/AffectsShroud.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Traits/CreatesShroud.cs Outdated
Show resolved Hide resolved OpenRA.Mods.Common/Traits/RevealsShroud.cs Outdated

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:shroud-multipliers branch from dfdd734 to 9e69e64 Nov 4, 2018

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Nov 4, 2018

Updated.

@pchote
Copy link
Member

pchote left a comment

LGTM after one fix and the testcase should of course be removed. The testcase also causes problems wrt #15419, but that is out of scope of the immediate goal here.

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:shroud-multipliers branch from 9e69e64 to 3aa520c Nov 5, 2018

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Nov 5, 2018

Updated.

1 similar comment
@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Nov 5, 2018

Updated.

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:shroud-multipliers branch from 3aa520c to e404aa6 Nov 10, 2018

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Nov 10, 2018

Updated.

@abcdefg30 abcdefg30 merged commit becfc15 into OpenRA:bleed Nov 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MustaphaTR MustaphaTR deleted the MustaphaTR:shroud-multipliers branch Nov 10, 2018

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Nov 10, 2018

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