fix cannon prediction bugs

[KyleAnthonyShepherd]

There have been some long standing bugs in the HaveFreeLineOfFire step of cannon weapons.

1. Errors in the calculations used to predict the trajectory of the cannon shot.
2. Too lenient of a check for determining trajectory intersections.

This Pull Request corrects those calculation errors, setting the proper linear and quadratic parabolic coefficients and using the weapon muzzle instead of aimpoint.

Image shows a cannon shot perfectly following the predicted path.

And this pull request adds in a DetectHit check along the "chord" of the parabola to mitigate instances of units thinking their shot will clear the top of a friendly unit, but instead clipping the close or far corner of the friendly unit.

Image shows an artillery unit correctly not firing and clipping the corner of a friendly unit.

Chord checks are replacing GetPointSurfaceDistance calls, so performance impact should be minimal.

Engine still builds successfully for me with these edits.

This is my first PR to the Spring/Recoil repo, so advice and feedback is greatly appreciated :)

[sprunk]

Would be good to measure the perf implications of running full collision queries per shot.

Also, check what happens if the muzzle is inside the target, like this (pic is for beamlaser but you get the idea):

[KyleAnthonyShepherd]

Compiled my side WITH this PR

Compiled my side WITHOUT this PR

Not sure the proper way to benchmark (and the engine compiled on my side seems very slow), but this suggests no performance impact, and suggests a slight improvement.

[KyleAnthonyShepherd]

Also, if the weapon muzzle is inside a friendly unit, the shot is correctly blocked and does not fire.

[lhog]

I see significant difference in Sim::Unit::Weapon occupation, does PR changes lie in the scope of that timer?

[sprunk]

Not sure the proper way to benchmark

Do something representative of cannons in a regular game, perhaps a bit over the top. Spawn 20 berthas and 200 stumpies per side and let them duke it out?

[lhog]

Not sure the proper way to benchmark

Do something representative of cannons in a regular game, perhaps a bit over the top. Spawn 20 berthas and 200 stumpies per side and let them duke it out?

I agree this should be tested at massive scale.

[KyleAnthonyShepherd]

Right, the screenshots of /debug I took was spawning in 1000 correap (BAR Tigers), and having them attack ground.

I can do a test of two large cannon weapon armies fighting.

And yes, it should lie in the scope of Sim::Unit::Weapon, as that calls HaveFreeLineOfFire which calls the files changed in the PR.

[sprunk]

Try something long-range too, eg 100 berthas firing above 500 solars. If that doesn't get worse either then the change is probably alright.

[KyleAnthonyShepherd]

armbrtha (BAR Basilica) currently has avoidfriendly = false, and so does not do friendly fire checks.
But for the purposes of benchmarking I can set that unitdef value to true.

[sprunk]

Yes, "bertha" stands for "something with a half-map range so that it generates a lot of checks along the trajectory".

[KyleAnthonyShepherd]

Testing with a long range, fast projectile weapon like bertha was a very good idea.
Revealed a "bug", where the more accurate linear and quadratic coefficients were also being fed into CGround::TrajectoryGroundCol, triggering false positive ground collision checks towards the end of the trajectory (when issued a ground attack command). The -10.0f factor on xzTargetDist is not enough to prevent this false positive behavior.

Thick red line is an added debug line, showing where the unit thinks the projectile will collide with the ground

[KyleAnthonyShepherd]

The -10.0f factor essentially tells the unit to ignore ground collisions in a 10 elmo radius cylinder around the target point.

So one possible solution is to tie the factor to a property of the projectile itself such as the horizontal velocity
Tell the unit to ignore ground collisions for the last X frames of projectile travel time.

A weapondef parameter to set the specific number of frames of end travel time to ignore could also address this issue
#870

[KyleAnthonyShepherd]

Unchanged engine

With PR

Bertha diagonally firing over a 50x50 square of armeyes, small 1x1 footprint buildings.
sim cur-%usage fluctuated between 20%-25% for both cases.

[lhog]

Somehow this new method of testing has 16x less load in Sim::Unit::Weapon compared to the previous test.

[KyleAnthonyShepherd]

Unchanged engine

With PR

2 12x12 blocks of correap (BAR Tiger) driving into each other.

I feel like the performance impact is smaller than the noise that /debug has.

I could just build a eval engine that puts HaveFreeLineOfFire in a loop that does it 10,000 times and measure wall clock time.

[KyleAnthonyShepherd]

Ok, did the 10k HaveFreeLineOfFire loop. Throwing in a std::chrono::system_clock::now(); before and after the loop
Base engine = 0.0282 seconds for 10,000 HaveFreeLineOfFire evaluations
With PR = 0.0444 seconds for 10,000 HaveFreeLineOfFire evaluations

So 57% longer to perform a friendly fire check.

[KyleAnthonyShepherd]

Found one optimization:
The obj->GetTransformMatrix(true); was expensive, and I was doing it two times for each check.
Doing it once, and feeding it into two separate CCollisionHandler::DetectHit brings the time down to
0.03175 for 10,000 HaveFreeLineOfFire evaluations.
So only a 12% performance cost over baseline.

[KyleAnthonyShepherd]

Doing:

1. const CMatrix44f objTransform = obj->GetTransformMatrix(true); before CCollisionHandler::DetectHit seems to provide a generic speedup.
2. Using a heuristic to choose which chord to check for collisions (instead of checking both). Only check the chord with the steepest downward slope.

Results in a net speedup compared to base engine:
Rerun wall clock times to account for different computer background activity
Base engine -> 0.0320 seconds for 10,000 HaveFreeLineOfFire evaluations
Optimized PR -> 0.0292 seconds for 10,000 HaveFreeLineOfFire evaluations
~9% speedup

[lhog]

Sounds good, thanks a lot for thorough inspection!

[KyleAnthonyShepherd]

Due to the more accurate trajectory parameters, fast projectiles that travel close to horizontal are triggering positive ground collision checks. Recent push sets the circle of ground collision to ignore from 10 elmos to projectileSpeedHorizontal * 3.0f.

I am still not super happy with this solution, as some units like the BAR sniper with very fast projectiles now have a tendency to fire at ground walls, as so much distance is not checked for collision ahead of time

[sprunk]

the #define looks very meek?

Suggested change

	go->SetColor(go->AddLine(pos, trajStartPos, 3, 0, GAME_SPEED), 1.0f, 0.0f, 0.0f, 1.0f);
	geometricObjects->SetColor(geometricObjects->AddLine(pos, trajStartPos, 3, 0, GAME_SPEED), 1.0f, 0.0f, 0.0f, 1.0f);

[KyleAnthonyShepherd]

Tuned the heuristic a little bit, to do the chord check based on the projectile vertical velocity at the test point.

Blue line in image shows the chord being checked for collision

[sprunk]

What if projectileSpeedHorizontal is 0? Think tremor or some other high-trajectory cannons with enough inaccuracy

[KyleAnthonyShepherd]

Good catch. :)
I'll be reviewing various zero conditions this week

[KyleAnthonyShepherd]

Example image of what this commit does, before I squash and merge in.

A. This commit now accurately predicts the black trajectory curve
B. For each friendly in the projectile path, it does a CCollisionHandler::DetectHit check (the blue line), from the point of the trajectory directly above the midpos of the friendly (indicated by green line if safe to fire, red line if not safe to fire), to one end of the trajectory curve.
Friendly 1 passes the check.
Friendly 2 fails the check, showing that the chord check is conservative, sometimes blocking shots that could have been fired without causing friendly fire.
Friendly 3 fails the check, showing the check works when the trajectory passes through the friendly.
Friendly 4 fails the check, showing a case where the previous check method would have passed a false positive (due to black trajectory curve not intersecting above the midpos, but does clip the corner of the collision box).
C. The CCollisionHandler::DetectHit check properly accounts for individual piece collision if the unit has that set, while the prior GetPointSurfaceDistance check does not.