bugfix(gui): Convert MessageDelayMS to logic frames so the setting takes effect

[bobtista]

Fixes #3131.

InGameUI::update computed the message timeout as m_messageDelayMS / LOGICFRAMES_PER_SECOND / 1000, dividing by both instead of converting milliseconds to logic frames. For positive values that reduces to floor(MessageDelayMS / 30000), which is wrong by a factor of about 900 and has no practical effect at any realistic setting. REAL_TO_INT_CEIL( ConvertDurationFromMsecsToFrames( ... ) ) is the conversion the codebase already uses for this, at ScriptEngine.cpp:6746, ScriptEngine.cpp:6803 and FXList.cpp:650.

Now converts through ConvertDurationFromMsecsToFrames, rounding partial frames up as that helper documents, so the timeout is the configured delay expressed in logic frames.

The shipped InGameUI.ini carries MessageDelayMS = 75000, an extra zero that never mattered while the conversion was broken. Once the conversion is correct that value would hold UI messages for 75 seconds, so InGameUI::validate() normalizes the known retail value to 7500 under ENABLE_GUI_HACKS, in the same spirit as the smudge particle validation.
TheSuperHackers/GeneralsGamePatch2#175 is the source data correction and can land whenever it is ready; once it does the validation is a no-op, since it only matches the exact retail value.

Predicted total message lifetime at 30 render / 30 logic FPS, including the existing alpha drain:

MessageDelayMS	total lifetime
3000	9.2 s (matches today's behaviour)
5000	10.0 s
7500	11.2 s
75000	75.4 s

Measured on MD_CHI01 before the validation was added, with the shipped MessageDelayMS = 75000 reaching the timeout unaltered, posting a message at logic frame 300 and logging the computed timeout and the message's age when it is removed:

	messageTimeout	message lifetime
before	2 frames	277 frames (9.2 s)
after	2251 frames	2263 frames (75.4 s)

Verified in game by instrumenting the message alpha and setting the delay to 3400 ms, which converts to 103 logic frames. The message stayed at full alpha through age 100 and only began draining after the timeout:

message age (logic frames)	alpha
85 / 90 / 95 / 100	255
105	254
110	249

Age 100 is the telling one: the existing fade amount becomes non-zero there, so on current main (where the timeout evaluates to 2 frames) the message is already draining by then. Holding to 103 shows the configured delay is what gates the fade.

Todo:

• Test that a UI message holds for the configured delay
• Replicate to Generals

[Mauller]

With the validate code "gui hack" check you could put this one for review independent of the data change.

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix UI message delay conversion and normalize retail value

🐞 Bug fix 🕐 10-20 Minutes

AI Description

• Converts configured message delays into rounded-up logic frames before message fading begins.
• Normalizes the known retail 75-second typo to 7.5 seconds under GUI hacks.
• Applies identical behavior to Generals and GeneralsMD.

Diagram

graph TD
  Config["UI config"] --> Parse["INI parser"] --> Validate{"Retail delay?"} -->|Yes| Normalize["Normalize 7500 ms"] --> Convert["Frame conversion"] --> Fade["Fade gate"]
  Validate -->|No| Convert

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Convert during INI parsing

• ➕ Stores the timeout directly in the unit consumed by update logic.
• ➕ Avoids repeating the conversion on every UI update.
• ➖ Changes the semantics implied by m_messageDelayMS.
• ➖ Requires normalization before conversion or a specialized parser.
• ➖ Creates a broader configuration representation change for a localized bug.

2. Rely on corrected source data

• ➕ Removes the runtime compatibility normalization.
• ➕ Keeps the canonical delay entirely data-driven.
• ➖ Does not work correctly with shipped retail data.
• ➖ Depends on an external data correction landing and being deployed.
• ➖ Still requires the engine-side unit conversion fix.

Recommendation: Keep the PR's approach: the established conversion helper provides correct rounding with minimal behavioral risk, while exact-value normalization makes the fix compatible with stock data. Correcting the source INI remains desirable, but should complement rather than replace the guarded compatibility path.

Files changed (4) +34 / -2

Bug fix (4) +34 / -2

InGameUI.hDeclare UI configuration validation +1/-0

Declare UI configuration validation

• Adds the validation method used to sanitize parsed InGameUI settings for Generals.

Generals/Code/GameEngine/Include/GameClient/InGameUI.h

InGameUI.cppCorrect Generals message delay timing +16/-1

Correct Generals message delay timing

• Validates InGameUI settings after parsing and normalizes the exact retail 75000 ms value to 7500 ms when GUI hacks are enabled. Converts MessageDelayMS to rounded-up logic frames before gating message fading.

Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp

InGameUI.hDeclare GeneralsMD UI configuration validation +1/-0

Declare GeneralsMD UI configuration validation

• Adds the validation method used to sanitize parsed InGameUI settings for GeneralsMD.

GeneralsMD/Code/GameEngine/Include/GameClient/InGameUI.h

InGameUI.cppCorrect GeneralsMD message delay timing +16/-1

Correct GeneralsMD message delay timing

• Mirrors the Generals fix by validating the retail delay after parsing and converting milliseconds to rounded-up logic frames before fading messages.

GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp

[greptile-apps]

Greptile Summary

This PR corrects UI message-delay conversion in both Generals variants and introduces a compatibility normalization for the shipped 75,000 ms value.

• Converts milliseconds to logic frames using the established duration helper and ceiling behavior.
• Adds validation after parsing each InGameUI definition.
• Replicates the behavior across Generals and Generals Zero Hour.
• The generic validation currently also overrides intentional 75-second settings supplied by mods or layered configuration.

Confidence Score: 4/5

The PR should not merge until the retail-data correction no longer overrides intentional 75-second delays from layered or mod-provided configuration.

The frame conversion itself follows the documented helper semantics, but the newly unconditional post-parse normalization breaks a valid configuration value in both game variants.

Files Needing Attention: Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp; GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp	Corrects frame conversion but applies the retail-value normalization to every parsed InGameUI configuration.
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp	Mirrors the conversion correction and the overly broad normalization in the Zero Hour implementation.
Generals/Code/GameEngine/Include/GameClient/InGameUI.h	Declares the new validation method used by the Generals parser.
GeneralsMD/Code/GameEngine/Include/GameClient/InGameUI.h	Declares the corresponding validation method for Zero Hour.

Prompt To Fix All With AI

### Issue 1
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp:935-938
**Intentional delays are overridden**

Every parsed `InGameUI` block that sets `MessageDelayMS = 75000` is silently rewritten to `7500` when GUI hacks are enabled. Because this parser also handles layered and mod-provided InGameUI files, an intentional 75-second delay becomes 7.5 seconds, causing messages to fade roughly 67.5 seconds earlier than configured. The same issue appears in `GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp:964-967`. Restrict this correction to known retail source data rather than applying it to every parsed value.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "bugfix(gui): Normalize retail message de..." | Re-trigger Greptile}

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

1. Negative delays keep messages forever 🐞 Bug ≡ Correctness

Description

InGameUI::update converts a negative m_messageDelayMS into a negative signed timeout and
compares it with an unsigned message age. Values such as -1000, which previously truncated to
zero, now become approximately -30 and are promoted to a huge unsigned threshold in both game
editions, preventing the fade and removal block from running during normal play.

Code

Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp[1866]

+	const int messageTimeout = REAL_TO_INT_CEIL( ConvertDurationFromMsecsToFrames( (Real)m_messageDelayMS ) );

Evidence

MessageDelayMS is parsed without a range constraint into a signed Int, while the helper
multiplies negative milliseconds by the frame conversion factor and the ceiling macro preserves
negative whole-frame results. The resulting signed timeout is compared against an expression derived
from two UnsignedInt frame values, so C++ converts the timeout to unsigned; the duplicated Zero
Hour implementation has the same data types and comparison.

Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp[787-792]
Generals/Code/GameEngine/Include/GameClient/InGameUI.h[899-905]
Core/GameEngine/Include/Common/GameCommon.h[71-81]
Core/Libraries/Include/Lib/BaseType.h[226-232]
Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp[1863-1872]
GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp[1897-1906]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Negative `MessageDelayMS` values can produce a negative timeout that is promoted to unsigned during the message-age comparison, effectively preventing messages from fading or being removed.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp[1864-1866]
- GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp[1898-1900]

## Recommended Fix
Clamp `m_messageDelayMS` to zero before converting it to logic frames in both implementations, or validate it unconditionally after parsing. Ensure the resulting timeout is nonnegative before it is compared with the unsigned message age.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Tickets: 🎫 MessageDelayMS uses an inverted unit conversion and has no practical effect 🎫 USA Comanche pilot does not spawn if helicopter crashes in under 1.5 seconds

Review mode: ⚖️ Balanced: This is a runtime behavior fix duplicated across two game variants, including configuration validation and timing conversion, so it warrants a complete review despite the small diff.

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[qodo-free-for-open-source-projects]

1. Negative delays keep messages forever 🐞 Bug ≡ Correctness

InGameUI::update converts a negative m_messageDelayMS into a negative signed timeout and
compares it with an unsigned message age. Values such as -1000, which previously truncated to
zero, now become approximately -30 and are promoted to a huge unsigned threshold in both game
editions, preventing the fade and removal block from running during normal play.

Agent Prompt

## Issue description
Negative `MessageDelayMS` values can produce a negative timeout that is promoted to unsigned during the message-age comparison, effectively preventing messages from fading or being removed.

## Fix Focus Areas
- Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp[1864-1866]
- GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp[1898-1900]

## Recommended Fix
Clamp `m_messageDelayMS` to zero before converting it to logic frames in both implementations, or validate it unconditionally after parsing. Ensure the resulting timeout is nonnegative before it is compared with the unsigned message age.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[greptile-apps]

Intentional delays are overridden

Every parsed InGameUI block that sets MessageDelayMS = 75000 is silently rewritten to 7500 when GUI hacks are enabled. Because this parser also handles layered and mod-provided InGameUI files, an intentional 75-second delay becomes 7.5 seconds, causing messages to fade roughly 67.5 seconds earlier than configured. The same issue appears in GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp:964-967. Restrict this correction to known retail source data rather than applying it to every parsed value.

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameClient/InGameUI.cpp
Line: 935-938

Comment:
**Intentional delays are overridden**

Every parsed `InGameUI` block that sets `MessageDelayMS = 75000` is silently rewritten to `7500` when GUI hacks are enabled. Because this parser also handles layered and mod-provided InGameUI files, an intentional 75-second delay becomes 7.5 seconds, causing messages to fade roughly 67.5 seconds earlier than configured. The same issue appears in `GeneralsMD/Code/GameEngine/Source/GameClient/InGameUI.cpp:964-967`. Restrict this correction to known retail source data rather than applying it to every parsed value.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[xezon]

I do not understand this comment.