Skip to content

bugfix(veterancy): Disable audiovisual cues for ejected veteran pilots#2643

Merged
xezon merged 3 commits intoTheSuperHackers:mainfrom
Caball009:fix_pilot_promotion
May 3, 2026
Merged

bugfix(veterancy): Disable audiovisual cues for ejected veteran pilots#2643
xezon merged 3 commits intoTheSuperHackers:mainfrom
Caball009:fix_pilot_promotion

Conversation

@Caball009
Copy link
Copy Markdown

@Caball009 Caball009 commented Apr 22, 2026

The ejection of veteran pilots is accompanied with audiovisual promotion cues. This is not obvious because the position (and matrix) of the pilot objects is not set yet when it goes through the veterancy level change, so they take place around coordinates 0, 0, 0:

gen_zh_pilot_promotion.mp4

This PR disables the audiovisual cues / feedback.

TODO:

  • Replicate in Generals.

@Caball009 Caball009 added Minor Severity: Minor < Major < Critical < Blocker Fix Is fixing something, but is not user facing labels Apr 22, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 22, 2026

Greptile Summary

This PR fixes a bug where ejected veteran pilots triggered promotion audiovisual cues at world origin (0, 0, 0) because the object's matrix hadn't been set yet at the time of the veterancy level change. The fix passes FALSE to the provideFeedback parameter of setVeterancyLevel in both the Generals and Zero Hour ObjectCreationList.cpp files.

The implementation is correct and consistent with the existing function signature (Bool provideFeedback = TRUE). Note that the PR description includes an unchecked TODO "Replicate in Generals", but the fix is already present in Generals/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp — the checkbox appears to have been left unchecked accidentally.

Confidence Score: 5/5

Safe to merge — minimal, targeted change with no logic risk.

Both changed files receive the same two-line fix that is fully covered by the existing optional provideFeedback parameter. No logic branches are affected; gameplay behaviour is unchanged beyond suppressing the misplaced audiovisual cue. The fix is symmetric across Generals and Zero Hour.

No files require special attention.

Important Files Changed

Filename Overview
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp Passes FALSE to setVeterancyLevel to suppress audiovisual cues when a pilot object's position has not yet been initialized; comment is clear and date-compliant.
Generals/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp Identical fix applied to the Generals (vanilla) variant; parity with the Zero Hour file is maintained.

Sequence Diagram

sequenceDiagram
    participant OCL as ObjectCreationList
    participant Obj as New Pilot Object
    participant ET as ExperienceTracker
    participant AV as AudioVisual System

    OCL->>Obj: create pilot object
    note over Obj: position/matrix NOT set yet (0,0,0)
    OCL->>ET: setVeterancyLevel(v, FALSE)
    ET->>ET: update m_currentLevel
    note over ET: provideFeedback=FALSE → skip feedback
    ET--xAV: onVeterancyLevelChanged skipped
    note over AV: no cue spawned at wrong position
    OCL->>Obj: set position/matrix (later)
Loading

Reviews (3): Last reviewed commit: "Replicated in Generals." | Re-trigger Greptile

Copy link
Copy Markdown

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Makes sense. Are there maybe more code paths that need this fix?

Comment thread GeneralsMD/Code/GameEngine/Source/GameLogic/Object/ObjectCreationList.cpp Outdated
@xezon xezon added Bug Something is not working right, typically is user facing Gen Relates to Generals ZH Relates to Zero Hour and removed Fix Is fixing something, but is not user facing labels Apr 23, 2026
@Skyaero42
Copy link
Copy Markdown

Shouldn't we move it to the moment the position is set, so that the audiovisual cue is there?

@Caball009
Copy link
Copy Markdown
Author

Caball009 commented Apr 24, 2026

Shouldn't we move it to the moment the position is set, so that the audiovisual cue is there?

My first attempt was moving the code below to L1233, but the GR1 replay started to mismatch.

if (m_inheritsVeterancy && sourceObj && obj->getExperienceTracker()->isTrainable())
{
DEBUG_LOG(("Object %s inherits veterancy level %d from %s",
obj->getTemplate()->getName().str(), sourceObj->getVeterancyLevel(), sourceObj->getTemplate()->getName().str()));
VeterancyLevel v = sourceObj->getVeterancyLevel();
obj->getExperienceTracker()->setVeterancyLevel(v);
//In order to make things easier for the designers, we are going to transfer the unit name
//to the ejected thing... so the designer can control the pilot with the scripts.
TheScriptEngine->transferObjectName( sourceObj->getName(), obj );
}

Second attempt was adding the code below before the call to setVeterancyLevel:

if (mtx)
	obj->setTransformMatrix(mtx);
else
	obj->setOrientation(orientation);

obj->setPosition(&chunkPos);

I think GR1 replay didn't mismatch with this code, but that's no guarantee for other replays. It also feels inefficient because the position / orientation is set later on as well.

Perhaps we can go with attempt 1 but behind a non-retail compatibility macro.

Edit: I don't feel very strongly either way, because the promotion for ejected pilots is undesirable anyway, so the position doesn't need to be set. For me it's more important not to introduce new issues.

@githubawn
Copy link
Copy Markdown

Adding the cue seems to be against #226

For the player, the "unit" already have had a promotion. It got it in the vehicle.

@Caball009
Copy link
Copy Markdown
Author

Are there maybe more code paths that need this fix?

What do you mean?

@xezon
Copy link
Copy Markdown

xezon commented Apr 24, 2026

What do you mean?

Are there more calls to setVeterancyLevel in the code base that need to suppress feedback?

Copy link
Copy Markdown

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

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

Adding the cue seems to be against #226

For the player, the "unit" already have had a promotion. It got it in the vehicle.

This is a good point and I agree that the number of queues is too much.

@Caball009
Copy link
Copy Markdown
Author

Caball009 commented May 2, 2026

Are there more calls to setVeterancyLevel in the code base that need to suppress feedback?

There a couple of places where it might be desirable to surpress feedback, but I'm not sure:

exp->setVeterancyLevel(obj->getExperienceTracker()->getVeterancyLevel());

I'm not sure it needs to be included in this PR, though.

@Caball009
Copy link
Copy Markdown
Author

Replicated in Generals.

@xezon xezon changed the title fix(veterancy): Disable audiovisual cues for ejected veteran pilots bugfix(veterancy): Disable audiovisual cues for ejected veteran pilots May 3, 2026
@xezon xezon merged commit 7219a19 into TheSuperHackers:main May 3, 2026
17 checks passed
@Caball009 Caball009 deleted the fix_pilot_promotion branch May 3, 2026 18:24
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 Gen Relates to Generals Minor Severity: Minor < Major < Critical < Blocker ZH Relates to Zero Hour

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants