beyond-all-reason / spring

A powerful free cross-platform RTS game engine
https://beyond-all-reason.github.io/spring/
Other
200 stars 98 forks source link

TrajectoryHeight MissileLauncher too timid about firing with terrain in the way. #869

Closed GoogleFrog closed 1 year ago

GoogleFrog commented 1 year ago

Here is a unit firing with a completely clear line of fire, the terrain is in the background, not in the way in any way. image

I then built out the terrain. Weirdly the original shooting unit continues to shoot, but no new unit can be told to shoot. image

Giving the shooting unit two wait orders interrupts it, and it can no longer shoot. image This quirk isn't that important, I was mostly firing a consistent way to reproduce the bug I care about, which is that the MissileLaunchers should shoot if they have a clear line of fire according to their TrajectoryHeight. This may mess up homing MissileLaunchers, so perhaps there should be a parameter for how much leeway they leave around their trajectory.

GoogleFrog commented 1 year ago

This came up investigating why the tank here won't fire. image

It's trajectory height gives it a clear shot, but the small amount of terrain below the Crab stops it. More generally the issue hurts units with trajectory firing onto highground.

KyleAnthonyShepherd commented 1 year ago

What I am running into testing this is that TrajectoryHeight MissileLauncher seem feature-incomplete. TrajectoryHeight MissileLauncher has a hard time targeting anything too high. And there is no accommodation for missiles with acceleration.

Probably just need to copy over more Cannon code.

GoogleFrog commented 1 year ago

Which parts are incomplete? I am by default happy with how they aim and fire in cases where they actually aim and fire. There is a bunch of weird missile behaviour so something seemingly minor could end up having an important effect. Anything is fine as an option, we just shouldn't have forced balance changes for things like projectile physics and aiming.

KyleAnthonyShepherd commented 1 year ago

armsptk had an issue in BAR, where its missiles would accelerate up to a speed, but the trajectory prediction code assumes the missile is traveling at full speed the whole time, causing unnecessary terrain collisions.

And the way TrajectoryHeight is done, to get the nice curved missile paths, essentially puts super high gravity on the missiles when fired at targets at short horizontal ranges (so a missile fired at a 45 degree angle upward TrajectoryHeight =1 can curve fast enough to be at a 45 degree angle downward at the target). So in the vs crab screenshot, that short of range is putting super high gravity on the missile (to get it to arc enough when shot at nearby targets), but that super high gravity makes the unit assume it cannot fire the missile high enough to reach the target.

So some option will be needed to clarify what is the intended behavior when firing at targets with a much greater elevation difference than horizontal difference.

GoogleFrog commented 1 year ago

image The unit in the screenshot (tankriot) has no trouble shooting very high. This is a screenshot after I told the unit to ignore terrain while aiming.

I am happy with how the ZK Recluse missile acceleration causes the peak of its trajectory to shift closer to Recluse than the target. But it is also unable to shoot more than ~200 elmos above itself. That is about the effective height of its trajectory so perhaps that makes sense. It seems ok to me and I haven't heard complaints about it.

KyleAnthonyShepherd commented 1 year ago

https://github.com/beyond-all-reason/spring/blob/BAR105/rts/Sim/Weapons/MissileLauncher.cpp#L76 https://github.com/beyond-all-reason/spring/blob/BAR105/rts/Sim/Weapons/Cannon.cpp#L88

Comparing the ground collision check code, and it looks like cannons do not check collision for the last 10 elmos of horizontal distance xzTargetDist - 10.0f while MissileLauncher is checking the full distance. So it might be getting false positives at the very end of the trajectory.

And from this image (Gave BAR armsptk the same weapon parameters as ZK tankriot), the predicted trajectory [green arrows from /debugtraceray], is not accurate to the real trajectory. image

sprunk commented 1 year ago

cannons do not check collision for the last 10 elmos of horizontal distance, while MissileLauncher is checking the full distance

This could be a weaponDef tag (defaulting to 10 for Cannons, 0 for MissileLauncher etc for backward compatibility), which would help #870.

KyleAnthonyShepherd commented 1 year ago

image image

Missiles with trajectoryheight are weird. You can get interesting trajectories with them, depending on trajectoryheight, maxspeed, initial speed, and acceleration. But I am pretty sure there is no simple equation to describe the trajectory. So it makes predicting their path, and checking for friendly fire and ground collisions difficult.

KyleAnthonyShepherd commented 1 year ago

image Ok, figuring out how to predict trajectoryheight missiles. Green arrows is current erroneous predicted path (triggering false positives on friendlies, and terrain when aiming upwards) And red line is current fixed prediction path. I'll make the PR once I am sure the new method does not murder performance.

sprunk commented 1 year ago

eeb94e348dec92352a091402ed63a0a329edb074