Skip to content

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

Open
xezon wants to merge 1 commit into
TheSuperHackers:mainfrom
xezon:xezon/fix-diagonal-movement-speed
Open

bugfix(physics): Fix diagonal movement speed discrepancy#3003
xezon wants to merge 1 commit into
TheSuperHackers:mainfrom
xezon:xezon/fix-diagonal-movement-speed

Conversation

@xezon

@xezon xezon commented Jul 22, 2026

Copy link
Copy Markdown

This change fixes the diagonal movement speed discrepancy.

The new 2d and 3d speeds are scaled to the average of the former min and max speeds.

@xezon xezon added Bug Something is not working right, typically is user facing Controversial Is controversial Major Severity: Minor < Major < Critical < Blocker Unit AI Is related to unit behavior Gen Relates to Generals ZH Relates to Zero Hour NoRetail This fix or change is not applicable with Retail game compatibility labels Jul 22, 2026
@greptile-apps

greptile-apps Bot commented Jul 22, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces compatibility-gated replacements for the 2D and 3D forward-speed calculations.

  • Adds a physics-specific retail-preservation macro.
  • Uses scaled vector dot products to compensate diagonal movement speed when compatibility behavior is disabled.

Confidence Score: 3/5

This PR should not merge until a normal intended build configuration actually enables the diagonal-speed correction.

Both compatibility macros are enabled in current builds, so preprocessing retains the old forward-speed calculations and excludes the behavior this PR is intended to introduce.

Core/GameEngine/Include/Common/GameDefines.h and GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp

Important Files Changed

Filename Overview
Core/GameEngine/Include/Common/GameDefines.h Adds a preservation flag defaulting to enabled, which prevents current builds from selecting the new physics behavior.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/PhysicsUpdate.cpp Adds compensated 2D and 3D forward-speed formulas, but both remain excluded by the active compatibility flags.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
Core/GameEngine/Include/Common/GameDefines.h:91
**Compatibility flags disable correction**

In every current build, this flag and `RETAIL_COMPATIBLE_CRC` both default to enabled. The conditions in both forward-speed functions therefore select the original retail calculations, so the new 2D and 3D diagonal compensation branches are not compiled and movement behavior remains unchanged.

Reviews (1): Last reviewed commit: "bugfix(physics): Fix diagonal movement s..." | Re-trigger Greptile

Comment thread Core/GameEngine/Include/Common/GameDefines.h
@gamezerve

Copy link
Copy Markdown

Why is the horizontal movement speed being increased? In retail, units already move at their locomotor-defined speed when traveling horizontally or vertically. Wouldn't slowing down diagonal movement be the more appropriate solution? Also, changing movement speeds will inevitably alter the timing of scripted in-game cinematics.

@xezon

xezon commented Jul 22, 2026

Copy link
Copy Markdown
Author

Why is the horizontal movement speed being increased? In retail, units already move at their locomotor-defined speed when traveling horizontally or vertically. Wouldn't slowing down diagonal movement be the more appropriate solution?

Because then on average the game unit movements will be around 20% slower than originally, noticably making the game play with less pace.

Also, changing movement speeds will inevitably alter the timing of scripted in-game cinematics.

That is a fair point we probably need to think about.

// 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

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.

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.

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

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

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.

@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.

So similar to what i put in the original issue

This should be the actual fix for this.

Real PhysicsBehavior::getForwardSpeed2D() const
{
	//Determine the direction of the velocity relative to the direction the unit is facing using the dot product
	const Coord3D *dir = getObject()->getUnitDirectionVector2D();
	Real vx = m_vel.x * dir->x;
	Real vy = m_vel.y * dir->y;
	Real dot = vx + vy;

	//Return the speed of the unit - Magnitude of the velocity is the speed
	if (dot >= 0.0f)
		return (Real)sqrtf( sqr(m_vel.x) + sqr(m_vel.y) ) * scalingValue;

	//Negative dot product means the unit is moving in reverse
	return -(Real)sqrtf( sqr(m_vel.x) + sqr(m_vel.y) ) * scalingValue;
}

And for 3D it is the same

Real PhysicsBehavior::getForwardSpeed3D() const
{
	//Determine the direction of the velocity relative to the direction the unit is facing using the dot product
	Vector3 dir = getObject()->getTransformMatrix()->Get_X_Vector();
	Real vx = m_vel.x * dir.X;
	Real vy = m_vel.y * dir.Y;
	Real vz = m_vel.z * dir.Z;
	Real dot = vx + vy + vz;

	//Return the speed of the unit - Magnitude of the velocity is the speed
	if (dot >= 0.0f)
		return (Real)sqrtf( sqr(m_vel.x) + sqr(m_vel.y) + sqr(m_vel.z) ) * scalingValue;

	//Negative dot product means the unit is moving in reverse
	return -(Real)sqrtf( sqr(m_vel.x) + sqr(m_vel.y) + sqr(m_vel.z) ) * scalingValue;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something is not working right, typically is user facing Controversial Is controversial Gen Relates to Generals Major Severity: Minor < Major < Critical < Blocker NoRetail This fix or change is not applicable with Retail game compatibility Unit AI Is related to unit behavior ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Locomotor makes units move faster diagonally

3 participants