feat(icu4c): Add ICU4C support to UTF8 string operations by CryoTheRenegade · Pull Request #3187 · TheSuperHackers/GeneralsGameCode

CryoTheRenegade · 2026-08-21T16:24:08Z

Replaces the hand-rolled WWLib UTF-8 converter with ICU4C, and links ICU into the engine so later code can use the rest of the suite.

AsciiString::translate / UnicodeString::translate now convert through ICU instead of 7-bit ASCII. Invalid UTF-8 still falls back to the original one-byte-to-one-wide-unit behavior so legacy CP1252 data is preserved. LAN player names are truncated on a UTF-8 code-point boundary instead of chopping mid-sequence.

ICU is selected in this order:

find_package(ICU) from vcpkg / system (C and C++ APIs)
Windows SDK icu.lib + /DELAYLOAD:icu.dll on modern MSVC
VC6 LoadLibrary of OS icu.dll, with Win32 CP_UTF8 if that DLL is missing

WWLib/IcuSupport.h is the engine include for linked ICU. vcpkg now depends on icu on all platforms.

In the future we can also use ICU's baked in bidirectional string support for languages that require them

qodo-free-for-open-source-projects · 2026-08-23T21:54:55Z

PR Summary by Qodo

Add ICU4C-backed UTF-8 conversions and link ICU into the engine

✨ Enhancement 🐞 Bug fix ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Replace WWLib UTF-8 conversion routines with ICU4C-backed transcoding.
• Add cross-toolchain ICU discovery/linking and expose it via core_icu.
• Fix LAN player-name truncation to respect UTF-8 code-point boundaries.

Diagram

graph TD
CMakeIcu["cmake/icu.cmake"] --> CoreIcu(["core_icu (INTERFACE)"]) --> Engine(["GameEngine targets"])
Engine --> Utf8["WWLib/utf8.cpp"] --> ICU{{"ICU4C"}}
Utf8 --> IcuHdr["WWLib/IcuSupport.h"]
Engine --> Strings["Ascii/Unicode translate"] --> Utf8
Engine --> Net["LAN name truncation"] --> Utf8
subgraph Legend
direction LR
_mod(["Module/Target"]) ~~~ _file["Source/File"] ~~~ _ext{{"External lib"}}
end

High-Level Assessment

The following are alternative approaches to this PR:

1. Use OS APIs per-platform (Win32 + iconv) instead of ICU

➕ Potentially smaller dependency footprint than ICU
➕ Leverages native platform behavior/tooling
➖ Inconsistent behavior across platforms/locales
➖ iconv availability/behavior varies; added portability work
➖ Harder to expose richer i18n features later (collation, IDNA, break iterators)

2. Adopt a lightweight UTF-8 library (e.g., utf8cpp/utf8proc) for conversion only

➕ Much smaller than ICU; simpler build integration
➕ Good for strict UTF-8 validation and iteration
➖ Does not provide the broader ICU suite the PR explicitly wants to expose
➖ May still require separate solutions for collation/normalization/IDNA later

3. Remove dynamic-load/delay-load complexity; require ICU at build-time everywhere

➕ Simpler runtime behavior and fewer fallback branches
➕ More predictable conversion results
➖ Breaks legacy toolchains (VC6) and some Windows environments without ICU readily available
➖ Contradicts stated compatibility goals for older builds

Recommendation: The chosen approach (ICU-first with well-scoped Windows fallbacks) best matches the stated goals: consistent UTF conversion semantics and future access to ICU’s i18n features, while keeping VC6 and older Windows scenarios functional. Reviewers should focus on verifying fallback correctness (invalid UTF-8 handling, null-termination rules, and length preflights) and ensuring the CMake selection order matches the intended deployment environments.

Files changed (13) +837 / -305

Enhancement (3) +676 / -233

AsciiString.cppRoute wide-to-UTF-8 translation through ICU-backed converter +14/-5

Route wide-to-UTF-8 translation through ICU-backed converter
• Updates AsciiString::translate to use Wide_To_Utf8_* conversion with explicit bounds checking against MAX_LEN. Behavior now relies on ICU-backed UTF-8 conversion where available.
Core/GameEngine/Source/Common/System/AsciiString.cpp

IcuSupport.hAdd engine-facing ICU4C include gateway header +58/-0

Add engine-facing ICU4C include gateway header
• Introduces a single header that conditionally includes Windows SDK ICU headers or full ICU headers (and optional i18n/C++ APIs) based on compile definitions.
Core/Libraries/Source/WWVegas/WWLib/IcuSupport.h

utf8.cppReplace hand-rolled UTF-8 codec with ICU/Win32-backed implementation +604/-228

Replace hand-rolled UTF-8 codec with ICU/Win32-backed implementation
• Rewrites UTF-8 <-> wide conversion to prefer ICU APIs (linked, Windows SDK delay-load, or runtime-loaded icu.dll) and fall back to Win32 CP_UTF8 when needed. Adds size/int safety checks and preflight logic for buffer sizing.
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp

Bug fix (2) +29 / -17

UnicodeString.cppRoute UTF-8-to-wide translation through ICU-backed converter with legacy fallback +16/-15

Route UTF-8-to-wide translation through ICU-backed converter with legacy fallback
• Updates UnicodeString::translate to use Utf8_To_Wide_* conversion and preserve legacy non-UTF-8 data via 1:1 byte-to-wide casting when input is invalid UTF-8.
Core/GameEngine/Source/Common/System/UnicodeString.cpp

GameInfo.cppTruncate LAN player names on UTF-8 code-point boundaries +13/-2

Truncate LAN player names on UTF-8 code-point boundaries
• Replaces character-by-character truncation with logic that avoids cutting UTF-8 continuation bytes, preventing malformed UTF-8 sequences in LAN option formatting.
Core/GameEngine/Source/GameNetwork/GameInfo.cpp

Refactor (1) +23 / -29

ThreadUtils.cppUse shared UTF conversion helpers for GameSpy thread utilities +23/-29

Use shared UTF conversion helpers for GameSpy thread utilities
• Switches Win32/WWLib conversion usage to the shared Utf8_To_Wide / Wide_To_Utf8 helpers and simplifies newline/carriage-return stripping logic.
Core/GameEngine/Source/GameNetwork/GameSpy/Thread/ThreadUtils.cpp

Documentation (1) +9 / -26

utf8.hUpdate UTF conversion API documentation to reflect ICU backing +9/-26

Update UTF conversion API documentation to reflect ICU backing
• Replaces the prior hand-rolled codec documentation with notes describing ICU-backed conversion behavior, fallback modes, and error semantics.
Core/Libraries/Source/WWVegas/WWLib/utf8.h

Other (6) +100 / -0

CMakeLists.txtInclude ICU build integration module +1/-0

Include ICU build integration module
• Adds inclusion of cmake/icu.cmake so ICU configuration is evaluated as part of the top-level build.
CMakeLists.txt

CMakeLists.txtLink core_icu into the engine interface libraries +1/-0

Link core_icu into the engine interface libraries
• Extends the engine’s public interface link set to include core_icu, making ICU availability/flags propagate to engine consumers.
Core/GameEngine/CMakeLists.txt

CMakeLists.txtExport ICU linkage from WWLib via core_icu +5/-0

Export ICU linkage from WWLib via core_icu
• Adds IcuSupport.h to the WWLib sources and links core_icu publicly so dependents can include ICU through the engine entry header.
Core/Libraries/Source/WWVegas/WWLib/CMakeLists.txt

icu.cmakeAdd core_icu target and toolchain-specific ICU selection +74/-0

Add core_icu target and toolchain-specific ICU selection

• Creates an INTERFACE core_icu target that prefers find_package(ICU), then Windows SDK icu.lib with delay-load, else runtime-load system icu.dll on Windows; non-Windows builds require ICU. Exports compile definitions indicating available ICU APIs.

cmake/icu.cmake

vcpkg-lock.jsonLock ICU dependency version in vcpkg manifest +18/-0

Lock ICU dependency version in vcpkg manifest
• Adds ICU to the vcpkg lockfile with a pinned version and git-tree entry, ensuring reproducible builds.
vcpkg-lock.json

vcpkg.jsonAdd ICU to vcpkg dependencies +1/-0

Add ICU to vcpkg dependencies

• Adds the icu port to the vcpkg dependency list so ICU is installed for supported builds.

vcpkg.json

qodo-free-for-open-source-projects · 2026-08-23T21:58:36Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~LoadSystemIcu data race~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

LoadSystemIcu() uses unsynchronized static state (attempted/available), so concurrent
first-use calls can race and cause undefined behavior while deciding whether ICU is available.
GameSpy Buddy/Peer worker threads perform UTF conversions, making concurrent first-use plausible in
real execution.

Code

Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[R48-51]
+bool LoadSystemIcu()
+{
+    static bool attempted = false;
+    static bool available = false;

Evidence
The new LoadSystemIcu() writes/reads shared static bool state without any synchronization, and
ICU availability checks can be reached from multiple concurrently-running GameSpy worker threads
that perform UTF conversions.
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[48-67]
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[583-590]
cmake/icu.cmake[47-56]
Core/GameEngine/Source/GameNetwork/GameSpy/Thread/ThreadUtils.cpp[37-84]
Core/GameEngine/Source/GameNetwork/GameSpy/Thread/BuddyThread.cpp[258-319]
Core/GameEngine/Source/GameNetwork/GameSpy/Thread/PeerThread.cpp[1153-1199]

Agent prompt

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

## Issue description
`LoadSystemIcu()` caches initialization in two plain `static bool` variables that are read/written without synchronization. If multiple threads hit UTF conversion at startup, this creates a C++ data race (UB) during the first ICU availability check.
### Issue Context
- Windows SDK ICU builds compile with `RTS_HAS_ICU_WINSDK`, and `IcuIsAvailable()` calls `LoadSystemIcu()`.
- GameSpy Buddy/Peer thread functions call `WideCharStringToMultiByte()` / `MultiByteToWideCharSingleLine()`, which call into `Wide_To_Utf8_*` / `Utf8_To_Wide_*` and can trigger the availability check.
### Fix Focus Areas
- Implement a one-time init guard (e.g., `std::once_flag`/`call_once` for modern toolchains; Win32 `InitOnceExecuteOnce` or `InterlockedCompareExchange` for older toolchains) and make `available` publication safe.
- Ensure the DLL load attempt is executed exactly once, and all threads observe the same stable result.
### Fix Focus Areas (paths and lines)
- Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[48-67]
- Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[583-587]
- Core/GameEngine/Source/GameNetwork/GameSpy/Thread/ThreadUtils.cpp[37-84]

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

2. ~~ICU disabled by system32-only load~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

In the Windows-SDK ICU configuration, IcuIsAvailable() only returns true if LoadSystemIcu() can
load System32\icu.dll, so ICU will be treated as unavailable even when icu.dll is present via
normal DLL search order (e.g., delay-loaded from the game directory). This forces silent fallback to
Win32 conversion and can also load a different DLL than the one intended by /DELAYLOAD:icu.dll.

Code

Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[R59-65]

+    char systemIcuPath[MAX_PATH];
+    const UINT systemDirectoryLength = GetSystemDirectoryA(systemIcuPath, MAX_PATH);
+    if (systemDirectoryLength > 0 && systemDirectoryLength <= MAX_PATH - sizeof("\\icu.dll"))
+    {
+        memcpy(systemIcuPath + systemDirectoryLength, "\\icu.dll", sizeof("\\icu.dll"));
+        available = LoadLibraryA(systemIcuPath) != nullptr;
+    }

Evidence
CMake explicitly configures delay-load of icu.dll, but the runtime availability check ignores the
normal DLL search path by loading only from the system directory and gating ICU usage on that
result.
cmake/icu.cmake[47-56]
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[59-65]
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[583-587]

Agent prompt

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

## Issue description
`LoadSystemIcu()` hardcodes loading `%SystemRoot%\\System32\\icu.dll`. When building with the Windows SDK ICU import lib + `/DELAYLOAD:icu.dll`, ICU might legitimately be provided next to the executable (or elsewhere on the DLL search path), but `IcuIsAvailable()` will still return false and the engine will never call ICU.
### Issue Context
- Windows SDK mode is enabled by `cmake/icu.cmake`, which sets `/DELAYLOAD:icu.dll`.
- `IcuIsAvailable()` uses `LoadSystemIcu()` for `RTS_HAS_ICU_WINSDK`.
### Fix Focus Areas
- Change the availability probe to respect the standard DLL search order (e.g., `LoadLibraryA("icu.dll")` / `GetModuleHandleA("icu.dll")`), optionally preferring the EXE directory if that is the intended distribution model.
- Keep the existing Win32 CP_UTF8 fallback only when `icu.dll` truly cannot be loaded.
### Fix Focus Areas (paths and lines)
- Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[59-65]
- Core/Libraries/Source/WWVegas/WWLib/utf8.cpp[583-587]
- cmake/icu.cmake[47-56]

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

3. ~~Found ICU lib path unused~~ ✓ Resolved 🐞 Bug ☼ Reliability

Description

cmake/icu.cmake calls find_library(RTS_ICU_WINSDK_LIB ...) but then links core_icu against the
literal icu name instead of the found path, so configuration can succeed while linking fails in
environments where icu.lib isn’t on the default linker search path. The status message also
implies the found path is being used when it is not.

Code

cmake/icu.cmake[R52-55]

+        set(RTS_ICU_WINSDK TRUE)
+        target_link_libraries(core_icu INTERFACE icu delayimp)
+        target_link_options(core_icu INTERFACE "/DELAYLOAD:icu.dll")
+        target_compile_definitions(core_icu INTERFACE RTS_HAS_ICU RTS_HAS_ICU_WINSDK RTS_HAS_ICU_I18N)

Evidence

The script records the result of find_library() into RTS_ICU_WINSDK_LIB but does not consume it
when specifying link libraries, so the probe can become a false positive for successful linking.

cmake/icu.cmake[48-57]

Agent prompt

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

## Issue description
In the Windows SDK fallback, CMake probes `RTS_ICU_WINSDK_LIB` but does not use it for linking. This can break builds where the SDK ICU library is discoverable by `find_library()` but not by the toolchain’s default linker search paths.
### Issue Context
This affects the `elseif(WIN32 AND NOT IS_VS6_BUILD AND NOT MINGW)` branch.
### Fix Focus Areas
- Replace `target_link_libraries(core_icu INTERFACE icu delayimp)` with `target_link_libraries(core_icu INTERFACE ${RTS_ICU_WINSDK_LIB} delayimp)`.
- Consider also updating the status message to reflect the actual link inputs.
### Fix Focus Areas (paths and lines)
- cmake/icu.cmake[48-57]

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

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

CryoTheRenegade · 2026-08-25T00:04:27Z

/agentic_review

qodo-free-for-open-source-projects · 2026-08-25T00:06:55Z

Code review by qodo was updated up to the latest commit 0240a44

CryoTheRenegade · 2026-08-26T18:09:03Z

@greptileai review this

greptile-apps · 2026-08-26T18:14:03Z

Greptile Summary

The PR replaces the shared UTF-8 conversion implementation with ICU4C-backed platform paths and updates engine string operations to use it.

Adds ICU discovery and linking for package-managed, Windows SDK, and legacy dynamic-loading configurations.
Routes AsciiString, UnicodeString, and GameSpy conversions through the shared ICU-backed functions.
Truncates LAN player names on UTF-8 code-point boundaries.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
cmake/icu.cmake	Adds the shared ICU interface target and selects package, Windows SDK, dynamic-loading, or required non-Windows behavior.
Core/Libraries/Source/WWVegas/WWLib/utf8.cpp	Replaces hand-written transcoding with ICU-backed implementations and Win32 fallback paths while preserving malformed-input signaling.
Core/GameEngine/Source/Common/System/AsciiString.cpp	Converts wide strings to UTF-8 through the shared API and guards the destination-size limit.
Core/GameEngine/Source/Common/System/UnicodeString.cpp	Converts valid UTF-8 through the shared API while retaining the legacy byte-wise fallback for invalid input.
Core/GameEngine/Source/GameNetwork/GameInfo.cpp	Truncates serialized LAN player names without retaining partial UTF-8 sequences.
Core/GameEngine/Source/GameNetwork/GameSpy/Thread/ThreadUtils.cpp	Moves GameSpy text conversion to the shared UTF-8 API and preserves single-line normalization.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Build[CMake ICU selection] --> Package[System or vcpkg ICU]
  Build --> WinSDK[Windows SDK ICU]
  Build --> Dynamic[Legacy dynamic icu.dll]
  Dynamic --> Win32[Win32 UTF-8 fallback]
  Package --> Shared[WWLib UTF-8 conversion API]
  WinSDK --> Shared
  Dynamic --> Shared
  Win32 --> Shared
  Shared --> Strings[AsciiString and UnicodeString]
  Shared --> GameSpy[GameSpy string helpers]
  GameSpy --> LAN[UTF-8-safe LAN name truncation]

_{Reviews (2): Last reviewed commit: "fix(icu): Use InterlockedIncrement for t..." | Re-trigger Greptile}

Use vcpkg or the Windows SDK C API on modern builds, keep VC6 on runtime LoadLibrary, and only probe system icu.dll for the delay-loaded SDK path.

Preserve bobtista's original change comments and append the ICU conversion notes instead of replacing them.

Publish the availability result with InterlockedCompareExchange, search the normal DLL path instead of System32 only, and pass the CMake-found icu.lib into the link line.

Avoid InterlockedCompareExchange, whose VC6 and later SDK signatures disagree, so utf8.cpp compiles on both toolchains.

chatgpt-codex-connector · 2026-09-01T19:21:02Z

To use Codex here, create a Codex account and connect to github.

CryoTheRenegade · 2026-09-01T19:22:25Z

@codex review this

chatgpt-codex-connector · 2026-09-01T19:25:36Z

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: c27b37a22f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

CryoTheRenegade marked this pull request as ready for review August 23, 2026 21:53

qodo-free-for-open-source-projects Bot reviewed Aug 23, 2026

View reviewed changes

Comment thread Core/Libraries/Source/WWVegas/WWLib/utf8.cpp Outdated

Comment thread Core/Libraries/Source/WWVegas/WWLib/utf8.cpp Outdated

Comment thread cmake/icu.cmake

CryoTheRenegade added 5 commits August 30, 2026 19:30

feat(os): Add UTF8 support using ICU

5f8cb9f

feat(icu): Link the full ICU4C suite across toolchains

de83891

Use vcpkg or the Windows SDK C API on modern builds, keep VC6 on runtime LoadLibrary, and only probe system icu.dll for the delay-loaded SDK path.

docs(icu): Keep prior UTF-8 comments alongside the ICU notes

55a3042

Preserve bobtista's original change comments and append the ICU conversion notes instead of replacing them.

fix(icu): Probe icu.dll once and link the found SDK import lib

d165e00

Publish the availability result with InterlockedCompareExchange, search the normal DLL path instead of System32 only, and pass the CMake-found icu.lib into the link line.

fix(icu): Use InterlockedIncrement for the VC6 icu.dll probe

c27b37a

Avoid InterlockedCompareExchange, whose VC6 and later SDK signatures disagree, so utf8.cpp compiles on both toolchains.

CryoTheRenegade force-pushed the feat/icu4c-utf8 branch from 0240a44 to c27b37a Compare August 31, 2026 01:31

Navigation Menu

Uh oh!

feat(icu4c): Add ICU4C support to UTF8 string operations - #3187

feat(icu4c): Add ICU4C support to UTF8 string operations#3187
CryoTheRenegade wants to merge 5 commits into
TheSuperHackers:mainfrom
CryoTheRenegade:feat/icu4c-utf8

CryoTheRenegade commented Aug 21, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 23, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 23, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CryoTheRenegade commented Aug 25, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 25, 2026

Uh oh!

CryoTheRenegade commented Aug 26, 2026

Uh oh!

greptile-apps Bot commented Aug 26, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

CryoTheRenegade commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

CryoTheRenegade commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 23, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CryoTheRenegade commented Aug 25, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 25, 2026

Uh oh!

CryoTheRenegade commented Aug 26, 2026

Uh oh!

greptile-apps Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

CryoTheRenegade commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

CryoTheRenegade commented Aug 21, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 23, 2026 •

edited

Loading

greptile-apps Bot commented Aug 26, 2026 •

edited

Loading