Skip to content

bugfix: Prevent cases where weapons would partially fire and require reloading without actually firing a shot #3416

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
Stubbjax wants to merge 3 commits into
base: main
Choose a base branch
from
+79 −2
Open

bugfix: Prevent cases where weapons would partially fire and require reloading without actually firing a shot #3416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
352445c
bugfix: Prevent cases where weapons would partially fire and require …
Stubbjax Oct 1, 2026
503ed60
chore: Add coverage for linked turrets
Stubbjax Oct 6, 2026
c9b4c66
refactor: Add convenience functions for handling leech ranges
Stubbjax Oct 7, 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
5 changes: 5 additions & 0 deletions Generals/Code/GameEngine/Include/GameLogic/Weapon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,11 @@ class Weapon : public MemoryPoolObject,
Bool isWithinAttackRange(const Object *source, const Object *target) const;
Bool isWithinAttackRange(const Object *source, const Coord3D* pos) const;

/** return true if the target is within attack range or the weapon has leech range, false otherwise.
*/
Bool isWithinEffectiveAttackRange(const Object* source, const Object* target) const;
Bool isWithinEffectiveAttackRange(const Object* source, const Coord3D* pos) const;

Bool isTooClose(const Object *source, const Object *target) const;
Bool isTooClose(const Object *source, const Coord3D *pos) const;

Expand Down
13 changes: 12 additions & 1 deletion Generals/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,7 @@ Bool outOfWeaponRangeObject( State *thisState, void* userData )
// victim->getID(), victim->getTemplate()->getName().str()));
return true;
}
if (!weapon->hasLeechRange() && !weapon->isWithinAttackRange(obj, victim))
if (!weapon->isWithinEffectiveAttackRange(obj, victim))
{
//CRCDEBUG_LOG(("outOfWeaponRangeObject() - object %d (%s) is out of range for attacking %d (%s)",
// obj->getID(), obj->getTemplate()->getName().str(),
Expand Down Expand Up @@ -5084,6 +5084,17 @@ StateReturnType AIAttackFireWeaponState::update()
return STATE_FAILURE;
}

#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 28/09/2026 The weapon may have gone out of range since we entered this
// state, so we check the range again to avoid partially firing the weapon.
Bool inRange = m_att->isAttackingObject()
? weapon->isWithinEffectiveAttackRange(obj, victim)
: weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());

if (!inRange)
return STATE_FAILURE;
#endif

/**
this is the weird case where we have multi turrets, and turret 'a' wants
to fire, but someone has changed the current weapon to be one not on him.
Expand Down
18 changes: 18 additions & 0 deletions Generals/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2137,6 +2137,24 @@ Bool Weapon::isWithinAttackRange(const Object *source, const Object *target) con
return false;
}

//-------------------------------------------------------------------------------------------------
Bool Weapon::isWithinEffectiveAttackRange(const Object* source, const Object* target) const
{
if (hasLeechRange())
return true;

return isWithinAttackRange(source, target);
}

//-------------------------------------------------------------------------------------------------
Bool Weapon::isWithinEffectiveAttackRange(const Object* source, const Coord3D* pos) const
{
if (hasLeechRange())
return true;

return isWithinAttackRange(source, pos);
}

//-------------------------------------------------------------------------------------------------
Bool Weapon::isTooClose(const Object *source, const Object *target) const
{
Expand Down
5 changes: 5 additions & 0 deletions GeneralsMD/Code/GameEngine/Include/GameLogic/Weapon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,6 +632,11 @@ class Weapon : public MemoryPoolObject,
Bool isWithinAttackRange(const Object *source, const Object *target) const;
Bool isWithinAttackRange(const Object *source, const Coord3D* pos) const;

/** return true if the target is within attack range or the weapon has leech range, false otherwise.
*/
Bool isWithinEffectiveAttackRange(const Object* source, const Object* target) const;
Bool isWithinEffectiveAttackRange(const Object* source, const Coord3D* pos) const;

Bool isTooClose(const Object *source, const Object *target) const;
Bool isTooClose(const Object *source, const Coord3D *pos) const;

Expand Down
22 changes: 21 additions & 1 deletion GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ Bool outOfWeaponRangeObject( State *thisState, void* userData )
// victim->getID(), victim->getTemplate()->getName().str()));
return true;
}
if (!weapon->hasLeechRange() && !weapon->isWithinAttackRange(obj, victim))
if (!weapon->isWithinEffectiveAttackRange(obj, victim))
{
//CRCDEBUG_LOG(("outOfWeaponRangeObject() - object %d (%s) is out of range for attacking %d (%s)",
// obj->getID(), obj->getTemplate()->getName().str(),
Expand Down Expand Up @@ -5240,6 +5240,17 @@ StateReturnType AIAttackFireWeaponState::update()
return STATE_FAILURE;
}

#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 28/09/2026 The weapon may have gone out of range since we entered this
// state, so we check the range again to avoid partially firing the weapon.
Bool inRange = m_att->isAttackingObject()
? weapon->isWithinEffectiveAttackRange(obj, victim)
: weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());

if (!inRange)
return STATE_FAILURE;
Comment on lines +5246 to +5251

@coderabbitai coderabbitai Bot Oct 7, 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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

set -eu
git diff --unified=35 b0c29eba834e7030e5c4d5f86f985e8ff799c92c c9b4c668130f08dc8d3fc34637c77397c51c7ab1 -- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
printf '\n--- reviewed source 5190-5365 ---\n'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp | sed -n '5190,5365p'
printf '\n--- selection references/declarations ---\n'
rg -n -F -- 'chooseBestWeaponForTarget' GeneralsMD/Code/GameEngine/Source

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 18347

🏁 Script executed:

set -eu
git diff --unified=35 b0c29eba834e7030e5c4d5f86f985e8ff799c92c c9b4c668130f08dc8d3fc34637c77397c51c7ab1 -- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp | sed -n '5190,5365p'
rg -n -F -- 'chooseBestWeaponForTarget' GeneralsMD/Code/GameEngine/Source

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 18269

🏁 Script executed:

git diff --unified=35 b0c29eba834e7030e5c4d5f86f985e8ff799c92c c9b4c668130f08dc8d3fc34637c77397c51c7ab1 -- GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp; nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp | sed -n '5190,5365p'; rg -n -F -- 'chooseBestWeaponForTarget' GeneralsMD/Code/GameEngine/Source

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 18269

🏁 Script executed:

printf '%s\n' '--- WeaponSet::chooseBestWeaponForTarget ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp | sed -n '770,1005p'
printf '%s\n' '--- AIStates position target caller ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp | sed -n '5430,5560p'
printf '%s\n' '--- Object delegation ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp | sed -n '1470,1500p'

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 17117

🏁 Script executed:

printf '%s\n' '--- WeaponSet::chooseBestWeaponForTarget ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp | sed -n '770,1005p'
printf '%s\n' '--- AIStates position-target caller ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/AI/AIStates.cpp | sed -n '5430,5560p'
printf '%s\n' '--- Object delegation ---'
nl -ba GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp | sed -n '1470,1500p'

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 17117

🏁 Script executed:

printf '%s\n' '--- linked turret configuration references ---'
rg -n -i -- 'areTurretsLinked|turretslinked|linked.?turret' GeneralsMD
printf '%s\n' '--- effective weapon range configuration references ---'
rg -n -i -- 'AttackRange\s*=' GeneralsMD/Data GeneralsMD/Code 2>/dev/null | head -80

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 4292

🏁 Script executed:

printf '%s\n' '--- linked turret configuration references ---'
rg -n -i -- 'areTurretsLinked|turretslinked|linked.?turret' GeneralsMD
printf '%s\n' '--- weapon range configuration examples ---'
rg -n -i -- 'AttackRange[[:space:]]*=' GeneralsMD/Data GeneralsMD/Code 2>/dev/null | head -80

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 4280

🏁 Script executed:

printf '%s\n' '--- root directories ---'
find . -maxdepth 2 -type d | sort | head -100
printf '%s\n' '--- repository-wide linked turret assignments ---'
rg -n -i -- 'TurretsLinked|LinkedTurret' .
printf '%s\n' '--- candidate data files ---'
rg --files | rg -i '\.(ini|xml|json)$' | head -100

Repository: TheSuperHackers/GeneralsGameCode

Length of output: 2205

Let linked position attacks reach the per-slot range checks.

For an unlocked position attack, chooseBestWeaponForTarget(nullptr, ...) selects PRIMARY_WEAPON without checking range. If that weapon is out of range while another linked slot is in range, this guard returns before the linked-slot loop can fire it. Skip the current-weapon guard for linked position attacks and let the per-slot checks decide.

🐛 Suggested fix 
-	Bool inRange = m_att->isAttackingObject()
-		? weapon->isWithinEffectiveAttackRange(obj, victim)
-		: weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());
-
-	if (!inRange)
-		return STATE_FAILURE;
+	Bool isLinkedPositionAttack =
+		!m_att->isAttackingObject() && obj->getAI()->areTurretsLinked();
+	if (!isLinkedPositionAttack)
+	{
+		Bool inRange = m_att->isAttackingObject()
+			? weapon->isWithinEffectiveAttackRange(obj, victim)
+			: weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());
+
+		if (!inRange)
+			return STATE_FAILURE;
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Bool inRange = m_att->isAttackingObject()
? weapon->isWithinEffectiveAttackRange(obj, victim)
: weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());
if (!inRange)
return STATE_FAILURE;
Bool isLinkedPositionAttack =
!m_att->isAttackingObject() && obj->getAI()->areTurretsLinked();
if (!isLinkedPositionAttack)
{
Bool inRange = m_att->isAttackingObject()
? weapon->isWithinEffectiveAttackRange(obj, victim)
: weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());
if (!inRange)
return STATE_FAILURE;
}

#endif

/**
this is the weird case where we have multi turrets, and turret 'a' wants
to fire, but someone has changed the current weapon to be one not on him.
Expand Down Expand Up @@ -5318,6 +5329,15 @@ StateReturnType AIAttackFireWeaponState::update()
Weapon *weapon = obj->getWeaponInWeaponSlot( (WeaponSlotType)slot );
if ( weapon )
{
#if !RETAIL_COMPATIBLE_CRC
// TheSuperHackers @bugfix Stubbjax 28/09/2026 The weapon may have gone out of range since we entered this
// state, so we check the range again to avoid partially firing the weapon.
Bool inRange = weapon->isWithinEffectiveAttackRange(obj, getMachineGoalPosition());

if (!inRange)
continue;
#endif

if ( weapon->fireWeapon(obj, getMachineGoalPosition()) ) //fire() returns 'reloaded'
obj->releaseWeaponLock(LOCKED_TEMPORARILY);// unlock, 'cause we're loaded

Expand Down
18 changes: 18 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Weapon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2323,6 +2323,24 @@ Bool Weapon::isWithinAttackRange(const Object *source, const Object *target) con
return false;
}

//-------------------------------------------------------------------------------------------------
Bool Weapon::isWithinEffectiveAttackRange(const Object* source, const Object *target) const
{
if (hasLeechRange())
return true;

return isWithinAttackRange(source, target);
}

//-------------------------------------------------------------------------------------------------
Bool Weapon::isWithinEffectiveAttackRange(const Object* source, const Coord3D* pos) const
{
if (hasLeechRange())
return true;

return isWithinAttackRange(source, pos);
}

//-------------------------------------------------------------------------------------------------
Bool Weapon::isTooClose(const Object *source, const Object *target) const
{
Expand Down
Loading