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

Optimize aircraft repulsion a little #16490

Merged
merged 1 commit into from May 9, 2019

Conversation

@reaperrr
Copy link
Contributor

commented May 4, 2019

See commit description.

@pchote

This comment has been minimized.

Copy link
Member

commented May 4, 2019

Repulse is supposed to already do this, see:

// Repulsion only applies when we're flying!
var altitude = self.World.Map.DistanceAboveTerrain(CenterPosition).Length;
if (altitude != Info.CruiseAltitude.Length)
return WVec.Zero;

Can we identify why this has broken and fix it instead?

@reaperrr reaperrr force-pushed the reaperrr:fix-repulsing branch from 72d2b57 to d52ace9 May 4, 2019

@reaperrr

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2019

Huh, hadn't seen that. I can't reproduce the issue on bleed, either. Maybe in #16470 the repulsion triggers before the aircraft starts to descend.

Anyway, we can still turn this PR into a minor clean-up/perf optimization, by re-using the cruising boolean instead of doing another DAT check in GetRepulsion(), and checking for WVec.Zero in Repulse() makes more sense, too (most returns in GetRepulsion return exactly that, and the rest should never return a Z value other than zero either, so this saves us a WVec -> HorizontalLengthSquared conversion).

Repulsion performance optimizations
`cruising` is updated  on every position change,
so we can safely re-use it instead of
performing another DAT check.
Additionally, if repulsion force is
horizontally zero, it's guaranteed to be WVec.Zero,
so we can save that conversion too.

@reaperrr reaperrr force-pushed the reaperrr:fix-repulsing branch from d52ace9 to 93d4f29 May 4, 2019

@reaperrr reaperrr changed the title Disable aircraft repulsion when not at CruiseAltitude Optimize aircraft repulsion a little May 4, 2019

@matjaeck

This comment has been minimized.

Copy link
Contributor

commented May 4, 2019

It looks like something has fixed the weird repulsion behavior that I noticed while testing #16241 (https://imgur.com/a/x8HonbW / https://imgur.com/a/EwLyZfX, log).

@pchote
pchote approved these changes May 4, 2019

@pchote pchote added the PR: Needs +2 label May 4, 2019

@teinarss
Copy link
Contributor

left a comment

Looks good!

@reaperrr reaperrr merged commit c24398e into OpenRA:bleed May 9, 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
4 participants
You can’t perform that action at this time.