Skip to content
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.

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

Laser patriot shoots at units outside of its range #370

Open
roossienb opened this issue Mar 4, 2025 · 13 comments
Open

Laser patriot shoots at units outside of its range #370

roossienb opened this issue Mar 4, 2025 · 13 comments
Assignees
Labels
Bug Something is not working right Major Severity: Minor < Major < Critical < Blocker USA Affects USA faction

Comments

@roossienb
Copy link

Laser patriots sometimes shoot at a unit, far beyond its range. May be an issue with patriot target communication.
Example:

  • Tractor gets shots twice before it is in patriot range
  • Once the tractor is shot, the remaining shots of one of the patriots hit the (out of range) Jarmen Kell.

Image

Could be related to issue #110

@xezon
Copy link

xezon commented Mar 4, 2025

I am impressed that we did not track this earlier. After all it is one of the popular bugs.

@xezon xezon added Bug Something is not working right Major Severity: Minor < Major < Critical < Blocker USA Affects USA faction labels Mar 4, 2025
@dmgreeny
Copy link

dmgreeny commented Mar 4, 2025

It happens with all patriot systems, the laser one is with the furthest reach, thats why its so noticeable.

@DevGeniusCode DevGeniusCode added the ExecutableMaybe Is perhaps game code related label Mar 4, 2025
@AdrianeYves
Copy link

What about the lag the assist system produced, is it tracked?

@xezon
Copy link

xezon commented Mar 5, 2025

We have reduced the lag with data changes, but perhaps there is more we can do in code.

@roossienb
Copy link
Author

roossienb commented Mar 5, 2025

So I think I understand what is happening. In the INIZH the assist behaviour states:

    AssistingClipSize = 4                       ; How many shots to make when asked by someone of my kind who has a RequestAssistRange weapon
    AssistingWeaponSlot = SECONDARY             ; And the weapon to use.  Should have huge range and no natural clip.
    LaserFromAssisted = PatriotBinaryDataStream ; Stream to draw from the requestor to me
    LaserToTarget = PatriotBinaryDataStream     ; Stream to draw from me to the target
  End

When asked for assistance, it will shoot an AssistingClipSize amount of projectiles from its secondary weapon (which is the patriot, but with extended range). Somehow, once the target is destroyed, the assistance is not reset and there may be projectiles left in the secondary weapon. Those projectiles need to be fired first, before it can switch back to the primary weapon (with reduced range).

This also explains the self-damage bug range in issue 27 in the game patch issue tracker. The patriot that is shooting at the damaged patriot is also requesting assistance from that same patriot. The damaged patriot hence switches to its secondary -long range - weapon but cannot fire its projectiles, because it is prohibited to shoot itself. It then uses the range finding of the secondary weapon to find another target and shoot it.

It is more obvious for laser patriots, because they fire instant. Due to the slow speed, it is more likely that a normal patriot fires all four projectiles from its secondary weapon before the target is destroyed.

I'm not very familiar with all the INI stuff and what logic is and is not in there, but I think @xezon is right that the assistance logic may be handled in the code. I'll see if I can dig into it more.

@roossienb
Copy link
Author

roossienb commented Mar 5, 2025

Logging my search

When an assist request comes in and the turret is free, the AssistingWeaponSlot (secondary weapon on patriots) is temporary locked, giving it preference over its primary weapon. According to the comment it lasts one clip size or until the target is destroyed. There are edge cases where I can think of this not working. Such as a manual stop command before a clip is fired, or if the target is the assisting turret.

https://github.com/TheSuperHackers/GeneralsGameCode/blob/0a05454d8574207440a5fb15241b98ad0b435590/Generals/Code/GameEngine/Source/GameLogic/Object/Update/AssistedTargetingUpdate.cpp#L95C1-L97C75

The temporary lock is released at some point. Investigating when this is and should be called.

https://github.com/TheSuperHackers/GeneralsGameCode/blob/0a05454d8574207440a5fb15241b98ad0b435590/Generals/Code/GameEngine/Source/GameLogic/Object/WeaponSet.cpp#L1030C1-L1050C2

Partial fix

static void makeAssistanceRequest( Object *requestOf, void *userData )

Here the assisting turret is called upon to fire on the target. However it is possible that the target (victim) is the same as the assisting turret being called upon. If the assisting turret is also the victim, it should not go into assist mode. This can be resolved by adding the following code

	// Don't ask ourselves (can't believe I forgot this one)
	if( requestOf == requestData->m_requestingObject )
		return;

        // -- NEW CODE TO BE ADDED
	// Don't ask ourselves if we are the target (community fix)
	if( requestOf == requestData->m_victimObject )
		return;

Once we have a compiling version I can test this and then create a PR

Still to figure out:

  • If the target is destroyed, make sure all assisting turrets reset their weapon to primary
  • If a stop command is given, make sure the turret switches to its primary weapon

@roossienb
Copy link
Author

roossienb commented Mar 5, 2025

@xezon @DevGeniusCode This is definitely an executable issue. Could you change the label from maybe to definite?
Also, if you like, you can assign me to this problem

And can you elaborate on the lag? Is it a performance issue, or is it delay until the assist is requested?

@roossienb
Copy link
Author

roossienb commented Mar 5, 2025

If the target is destroyed, make sure all assisting turrets reset their weapon to primary

A fix for this is likely to be found in AIUpdate.cpp

here

void AIUpdateInterface::privateAttackObject( Object *victim, Int maxShotsToFire, CommandSourceType cmdSource )

and here

void AIUpdateInterface::privateForceAttackObject( Object *victim, Int maxShotsToFire, CommandSourceType cmdSource )

In both routines nothing happens if the victim is no longer there. But if the victim is gone, a weapons lock reset is required to ensure the turret switches from secondary (extended range) to primary weapon

Proposal is to change in both routines

	if (!victim) 
	{
		// Hard to kill em if they're already dead.  jba
		return;
	}

to

	if (!victim) 
	{
                // Reset weapons lock to fix patriot turret extended range bug (community fix)
                getObject()->releaseWeaponLock(LOCKED_TEMPORARILY);
                
		// Hard to kill em if they're already dead.  jba
	        return;
	}

@xezon xezon removed the ExecutableMaybe Is perhaps game code related label Mar 5, 2025
@xezon
Copy link

xezon commented Mar 5, 2025

Good luck :-)

@roossienb
Copy link
Author

If a stop command is given, make sure the turret switches to its primary weapon

If the player keeps spamming the stop command for the turret, while it receives an assist request, it will not fire at the target at all. But it will remain in the secondary weapon state.

I'm not sure if a reset is needed, or that the AIUpdate code is run anyways and will releases the weapon lock (after the above patch) because the victim is death. This will require testing of the above patch.

In case a weapon lock release is required at stop command it probably goes here:

void AIGroup::groupIdle(CommandSourceType cmdSource)

@LyreZ61

This comment has been minimized.

@roossienb
Copy link
Author

A reminder for myself (and others): Fixing this will break compatibility with non-patched versions.

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 Major Severity: Minor < Major < Critical < Blocker USA Affects USA faction
Projects
None yet
Development

No branches or pull requests

6 participants