Fix collisions with long dtime, in particular with bouncing

[kno10]

Closes #15027

When there is lag in the server (generating map chunks, spawning structures, mobs...) and it misses some steps and we have a large dtime, entities can bounce badly on beds. Seen for example in Voxelibre and Mineclonia.

The main issue is that speed_f is the speed at the end of the movement, not at collision time. For a large timestep, this speed can be much too large (consider a mob just over a bed, accelerated by gravity for a long timestep).

This patch (A) introduces aspeed_f, which is the average movement speed not the maximum (B) on collision, the speed is updated according to the estimated dtime until collision.

For the villager example above, in particular (B) means a much smaller velocity when bouncing.

For further detail, see #15027

[sfan5]

It's hard to tell which are cosmetic changes and which are functional changes that fix the bug. Is it aspeed_f?

[kno10]

This patch (A) introduces aspeed_f, which is the average movement speed not the maximum (B) on collision, the speed is updated according to the estimated dtime until collision.

So yes, aspeed_f is the most important part of it. The old code used the speed at the end of the movement, not the average speed during the time step. But further down, aspeed is updated not with the full dtime, but with nearest_dtime

aspeed_f = *speed_f + accel_f * 0.5f * nearest_dtime;
*pos_f += truncate(aspeed_f * nearest_dtime, 100.0f);

This could probably be one line, but the important thing is that the average speed during the partial time nearest_dtime is correct, instead of the acceleration over the full dtime as before (which caused a very high impact speed on bouncy materials with lag), if nearest_dtime is much smaller than dtime.

[sfan5]

Can you separate the fix and the style changes into two commits?

[kno10]

This would be really nice to have in 5.9.1 or 5.9.2 IMHO.

[appgurueu]

Thought: This is still some kind of linear approximation of a quadratic function, just now with the average speed, isn't it?

We could make axisAlignedCollision take acceleration into account properly: If acceleration on an axis is zero, the current code is fine. Otherwise, exactly solve the quadratic equation for the first point in time >= 0 where the faces touch. (We might want doubles here?)

So for example if we're checking for an X axis collision, we look at the current velocity. Say the X component is > 0. So we want to check whether our +X face smashes into another the -X face of another box. Now we have the distance between those faces and we want to find the point in time at which they touch; we then check whether the YZ projections of the AABBs overlap. That's what the code is essentially currently doing for all three axes, except it doesn't take acceleration into account: It's just dtime = distance / std::abs(speed.Y); when it should be $vt + \frac{1}{2}at = d$ solved for $t$ where $d$ is the distance between the faces solved for $t$.

If we do this, some extra optimizations might be needed to not hurt performance, as axisAlignedCollision is pretty hot.

[kno10]

I did not previously check axisAlignedCollision, and this fix here is quite urgent IMHO - there are plenty of complaints in minetest games such as Voxelibre and Mineclonia about mobs bouncing (on beds) through roofs.
Hence I opened #15236 to discuss this part. But there also is #15227 which further complicates things when considering two moving objects (although it should be possible to compute this similarly by using their relative positions and relative speeds, which should reduce this to the base case again).

[SmallJoker]

Did the villager drop test + 6 seconds server delay.

ACTION[Server]: singleplayer executed local t0 = os.time() while os.time() < t0+6 do end
WARNING[Server]: collisionMoveSimple: maximum step interval exceeded, lost movement details!

Works. As far as I can tell, there's no noticeable difference for the client-side physics because its step size is small enough.

[appgurueu]

I will review this properly in a couple days when the feature freeze starts (till then I will prioritize reviewing feature PRs; since this is a bug fix it can be merged during feature freeze).

[Zughy]

@appgurueu reminder

[appgurueu]

Fixes the issue it claims to fix. The math should at least be less bogus than before, on average, now. This seems fine as a quick fix for 5.10.0.

---

I was hoping there would be a simpler fix lurking here (by simply using the current speed before applying acceleration in the linear approximation), but I tried to implement it and it's not as simple (specifically the "no collisions" case poses a major problem; we do want to apply acceleration there, but we don't know whether by doing so we overshoot because there will be an early collision). I think ultimately I'll implement https://github.com/minetest/minetest/pull/15029/files?diff=split&w=1#r1786903169.

[appgurueu]

This is somewhat nonsensical, but I see no better way of doing it yet short of implementing the proper solution. (I do get that it tries to compensate for the fact that we already added half of the dtime worth of acceleration, but overall the math doesn't check out, even for a linear approximation.)

[kno10]

The 0.5 comes from adding the average acceleration over the interval nearest_dtime.
So it's fine as long as nearest_dtime is accurate (which it is not entirely, as discussed elsewhere).
As we only simulate up to nearest_dtime in this iteration, we have to update the average speed to match this segment only. We could iterate multiple times here to approximate this numerically (recheck the collision with the new aspeed until the change is tiny), but I believe this will be fine and is better fixed by improving the axisAlignedCollision function.
But when nearest_dtime is much smaller than dtime, then this version will be more accurate than not updating aspeed_f (because the initial aspeed_f then was not very accurate).

[kno10]

@grorp ok, reverted back to const v3f vec

[kno10]

Improved version #15408 - please consider that for 5.11