Skip to content

bugfix(physics): Fix diagonal movement speed discrepancy #3003

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.

Sign up for GitHub

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

Open
xezon wants to merge 3 commits into
base: main
Choose a base branch
from
+63 −11
Open

bugfix(physics): Fix diagonal movement speed discrepancy #3003

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7be8457
bugfix(physics): Fix diagonal movement speed discrepancy
xezon Jul 22, 2026
6f4def0
Preserve legacy speeds for scripted movements
xezon Jul 29, 2026
19ada89
Polish comments
xezon Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Core/GameEngine/Include/Common/GameDefines.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@
#define PRESERVE_RETAIL_SCRIPTED_CAMERA (1) // Retain scripted camera behavior present in retail Generals 1.08 and Zero Hour 1.04
#endif

#ifndef PRESERVE_RETAIL_PHYSICS_FORWARD_SPEED
#define PRESERVE_RETAIL_PHYSICS_FORWARD_SPEED (1)
Comment thread
xezon marked this conversation as resolved.
#endif

#ifndef RETAIL_COMPATIBLE_CRC
#define RETAIL_COMPATIBLE_CRC (1) // Game is expected to be CRC compatible with retail Generals 1.08, Zero Hour 1.04
#endif
Expand Down
25 changes: 21 additions & 4 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -963,15 +963,22 @@ Real PhysicsBehavior::getForwardSpeed2D() const

Real dot = vx + vy;

Real speedSquared = vx*vx + vy*vy;
// DEBUG_ASSERTCRASH( speedSquared != 0, ("zero speedSquared will overflow sqrtf()!") );// lorenzen... sanity check

Real speed = (Real)sqrtf( speedSquared );
#if RETAIL_COMPATIBLE_CRC || PRESERVE_RETAIL_PHYSICS_FORWARD_SPEED
Real speed = (Real)sqrtf( vx*vx + vy*vy );

if (dot >= 0.0f)
return speed;

return -speed;
#else
// Inverse scale len by (1 + sqrt(2)) / 2 to adjust to the average of the former min/max movement speed.
Comment thread
xezon marked this conversation as resolved.
Outdated
// The inverse looks intuitively wrong, but it is correct, because the value returned by this function is
// used to determine the additional velocity needed to reach the target speed.
constexpr const Real DiagonalCompensation = 1.0f / 1.20710678f;
dot *= DiagonalCompensation;

@Mauller Mauller Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, the speed is not the dot product, the dot product tells you the difference in direction between the two vectors. If it goes negative then it means your vectors are going in opposite directions.

The speed of a vector is the magnitude of the vector.

The speed for 2D is:
speed = sqrtf( sqr(m_vel.x) + sqr(m_vel.y) ); which you can then scale with a constant

The speed for 3D is:
speed = sqrtf( sqr(m_vel.x) + sqr(m_vel.y) + sqr(m_vel.z) ); then the same can be scaled with a constant.

you still need to check the dot product and negate the speed if the dot product is negative.

@xezon xezon Jul 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dot is correct.

Chat Gippy

Here's a side-by-side comparison using a true velocity magnitude of 100 in each case.

Facing Moving dir vel Original Function Dot Product
East East (1.000, 0.000) (100.00, 0.00) 100.00 100.00
North North (0.000, 1.000) (0.00, 100.00) 100.00 100.00
45° 45° (0.707, 0.707) (70.71, 70.71) 70.71 100.00
East Northeast (1.000, 0.000) (70.71, 70.71) 70.71 70.71
45° East (0.707, 0.707) (100.00, 0.00) 70.71 70.71
30° 30° (0.866, 0.500) (86.60, 50.00) 79.06 100.00
60° 60° (0.500, 0.866) (50.00, 86.60) 79.06 100.00
15° 15° (0.966, 0.259) (96.59, 25.88) 93.54 100.00
75° 75° (0.259, 0.966) (25.88, 96.59) 93.54 100.00

This reveals that the original function is effectively applying a heading-dependent scale factor:

  • 0° / 90°: ×1.000

  • 15° / 75°: ×0.935

  • 30° / 60°: ×0.791

  • 45°: ×0.707

So if getForwardSpeed2D() is used in movement logic rather than just for display, the old code was inherently reducing the reported speed whenever the unit faced away from the world axes. That could explain why replacing it with the mathematically correct dot product changed the feel of movement.

@Mauller Mauller Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dot product is not correct for calculating the speed. The only relation the dot product has with the speed is the direction of the movement in relation to the orientation of the model/hull. So whether it is forwards or backwards etc.

in the code, Dir is the direction vector for the objects model/hull to tell which way it is facing and m_vel is the motion vector for the movement of the objects.

the dot product is used to tell the difference in angle between Dir and m_vel so we can determine if the object is moving backwards in relation to the direction it is facing. If the dot product is negative then the two vectors are facing in opposite directions and the speed will be negative relative to the objects orientation.

You have to workout the magnitude of m_vel to determine the speed of the object, which is the equivalent to using Pythagoras theorem to workout the hypotenuse of a triangle.

The flaw in the original code is that they used vy and vx which are intermediate products of calculating the dot product between Dir and m_vel. These should never be used outside of that calculation as they are meaningless outside of that context.

Since these intermediate products are not unit scaled they give the faster motion in the diagonal direction, but they also don't give the true speed either.

@xezon xezon Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem is speed = sqrtf( sqr(m_vel.x) + sqr(m_vel.y) ); does not account for object direction. If the object is facing sideways then it will not produce the same direction drag (or lack thereof). What the proposed solution does is eliminate the diagonal speed variance, but otherwise preserve the original average speed.

I tested it in game and it looked right, but I did not do a lab test. Maybe it needs a lab test.

@Mauller Mauller Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The m_vel value is already normalised, which means it correctly scales in all directions without the diagonal calculated vector magnitudes being larger than expected. The flaw in the original code is that they don't correctly calculate the speed since they use the intermediate dot product values which are not normalised values.

This function is just returning speed which is only a scalar value, it has no direction information within it apart from forwards and backwards.

Both calculations need to be done, speed = sqrtf( sqr(m_vel.x) + sqr(m_vel.y) ); and the dot product is used to determine if the speed is positive or negative.

Beyond this, to make the speeds, on average, closer to the original flawed speeds you can then scale the calculated speed just by multiplying it with a constant. This will scale in all directions due to m_vel already being normalised.

so finalSpeed = scalingValue * speed * dotProductDirection the dot product direction just being if it's positive or negative.

@Skyaero42 Skyaero42 Jul 29, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think that both @Mauller and I misinterpreted the meaning of final speed. It is not supposed to be the magnitude of the speed vector, but the component of the speed vector that faces forward.

This function is effectively a vector rotation that returns the x-component of the rotated vector.

image

Given a vector v = (vx, vy) in a coordinate frame (x, y).
Now we rotate the coordinate frame into (x', y') such that the x-axis lines up with the direction of the object, i.e. d` = (1, 0) by definition. In the (x,y) frame the direction vector d = (cos a, sin a)

Rotating a vector with angle a yields:

v` = (vx cos a + vy sin a, -vx sin a + vy cos a)

note that rotating the direction vector yields our desired result
d` = (cos a cos a + sin a sin a, - cos a sin a + sin a cos a) = (1, 0).

Now v` represents the velocity vector in the direction coordinate frame, where the x-direction is the forward/backward speed and y-direction represent the sideways movement.

So for this function to return the forward speed component, it has to return v`x = vx cos a + vy sin a. This equals v * d ('*' being the dot product here).

So solution (A) is correct.

That does leave the discussion about DiagonalCompensation in my other post that - while statistically may yield correct average speeds - it can have major consequences on the gameplay and could be highly controversial.

@xezon xezon Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

speedAvgs | {2.0779447383905580, 1.9905874710383378, 1.9912092029274284}

I think the explanation for this is the discrepancy between Acceleration and Braking (aka Deceleration). When Acceleration is higher, then top speeds will be reached quicker than low speeds and therefore in certain circumstances, such as the circling planes, the average speed of EA's speed (C) will be higher (or lower).

Locomotor RaptorJetLocomotor
  Acceleration = 120.0 ; in dist/(sec^2)
  Braking = 10.0 ; in dist/(sec^2)

@Mauller Mauller Jul 30, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So looking into this further, a bit like @Skyaero42 mentioned, it really comes down to what this function is meant to do and original intentions.

So i have not come across it being used in this way before, but when looking into it the use of the velocity vector and direction vector in a dot product is a bit of a special case. And it only works when the direction vector is properly normalised.

The dot product then gives the proportion of the speed in the direction of the direction vector. But only when |d| = 1 at all times.

So for the equation we start with v.x = |v||d| cosθ. If |d| = 1 at all times, then this simplifies to v.x = |v|cosθ which gives the speed in the direction of the direction vector in this case.

But with the way the original function was written, they appear to want to use the absolute speed rather than the component the speed in the direction of the object. The dot product is then only used to determine the direction of the sign of the speed.

The original calculation was wrong since they used the intermediate products of the dot product which are not normalised and cannot be used beyond calculating the dot product. So they cannot be used in the Pythagoras theorem to determine the speed.

So this function has two issues.

  1. The true intent of the function needs determining in a wider context.
  2. The original intent in the code and the name of the function can be interpreted in different ways.

And if it was meant to be using the component of the speed in the direction the object is facing, then this function also needs a debug check to make sure the direction vector is always normalised. The moment the direction vector is not normalised then the dot product cannot be used to determine the component of the speed in the direction the object is facing.

@xezon xezon Jul 30, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intent of the function is clear by its name: GetForwardSpeed2D. It aims to return forward speed (and backwards speed). Its usage is also quite clear; used to find the delta between current and goal speed, and then apply the delta to accelerate or brake an object with.

@Ui-shigure-z Ui-shigure-z Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it needs a test on helicopters to see what happened
hover locomotor is the only exception that oriention might be different with movement direction.
eg. 1 comanche and 1 battlemaster which has same speed moves parallelly towards same direction and BM is in comanche range.


return dot;
#endif
}

//-------------------------------------------------------------------------------------------------
Expand All @@ -989,12 +996,22 @@ Real PhysicsBehavior::getForwardSpeed3D() const

Real dot = vx + vy + vz;

#if RETAIL_COMPATIBLE_CRC || PRESERVE_RETAIL_PHYSICS_FORWARD_SPEED
Real speed = (Real)sqrtf( vx*vx + vy*vy + vz*vz );

if (dot >= 0.0f)
return speed;

return -speed;
#else
// Inverse scale len by (1 + sqrt(3)) / 2 to adjust to the average of the former min/max movement speed.
// The inverse looks intuitively wrong, but it is correct, because the value returned by this function is
// used to determine the additional velocity needed to reach the target speed.
constexpr const Real DiagonalCompensation = 1.0f / 1.36602540f;
dot *= DiagonalCompensation;

return dot;
#endif
}

//-------------------------------------------------------------------------------------------------
Expand Down
Loading