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 MinRange support to AffectsShroud #17449

Merged
merged 3 commits into from Dec 23, 2019
Merged

Conversation

@pchote
Copy link
Member

pchote commented Dec 10, 2019

This is then used to remove the overlap between the two vision traits on RA actors. Testing against release-20191117 with 100 Yaks shows a tick time improvement from ~50 to ~30ms. This plus #17397 should give the hotfix better vision performance than release-20190314.

This shouldn't conflict with #17383, and may give us a much smaller (possibly negligible) improvement on top of it. In any case, the minimum range support may be useful for modders.

This also fixes the bogus vision definition for the allied tech center in RA to avoid having a minrange > maxrange.

pchote added 3 commits Dec 10, 2019
This brings a significant perf saving by reducing
the number of evaluated tiles.
@pchote pchote added this to the Next Release milestone Dec 10, 2019
{
if ((map.CenterOfCell(c) - projectedPos).HorizontalLengthSquared <= limit)
var dist = (map.CenterOfCell(c) - projectedPos).HorizontalLengthSquared;
if (dist <= maxLimit && (dist == 0 || dist > minLimit))

This comment has been minimized.

Copy link
@pchote

pchote Dec 10, 2019

Author Member

Ideally, this would have been written as if (dist < maxLimit && dist >= minLimit), but we have many actors with ranges on the cell boundary and this would have changed their vision footprints.

This comment has been minimized.

Copy link
@abcdefg30

abcdefg30 Dec 19, 2019

Member

Do we want to put a comment here explaining this?

Copy link
Contributor

reaperrr left a comment

Looks good to me

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Dec 15, 2019

@tovl do you have any thoughts on how this will interact or interfere with #17383?

@tovl

This comment has been minimized.

Copy link
Contributor

tovl commented Dec 15, 2019

I think adding support for minimum sight ranges is a good modding enhancement regardless of it's effect on performance and I don't see any direct conflicts with #17383 because that doesn't really touch upon the actual sight range calculation part.

The performance gains when added on top of #17383 would probably be a lot less though.

@abcdefg30 abcdefg30 merged commit b1f7c5c into OpenRA:bleed Dec 23, 2019
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 Dec 23, 2019

@pchote pchote deleted the pchote:shroud-minrange branch Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.