bugfix(globaldata): Fix the handling of documents folder redirection by using SHGetKnownFolderPath()

[Mauller]

This PR improves folder redirection handling that can occur due to Group Policy or OneDrive redirecting the users documents folder.

This only works for non retail builds as it required a function not supported under VC6 as it only appeared in Vista onwards.
SHGetKnownFolderPath() has better handling of folder redirection built into it compared to the legacy SHGetSpecialFolderPath()

Generals and zero hour use different ways to obtain the game folder name which is why the code is different between them.

[greptile-apps]

Greptile Summary

This PR replaces the legacy SHGetSpecialFolderPath call (which is insensitive to OneDrive/Group Policy folder redirection) with SHGetKnownFolderPath for all compiler versions above VC6 (_MSC_VER >= 1300), improving documents-folder resolution for non-retail builds on modern systems.

• Both changed files gate the old code behind #if defined(_MSC_VER) && _MSC_VER < 1300 and place the new SHGetKnownFolderPath path in the corresponding #else block.
• CoTaskMemFree(pszPath) is correctly called unconditionally after the success-check block in both files (initialised to nullptr so it is a safe no-op on failure), resolving the issue raised in prior review threads.
• The preprocessor boundary (< 1300) is consistent across both the guard and the CoTaskMemFree placement — the off-by-one edge case raised in a prior thread is no longer present.
• All new code correctly uses nullptr instead of NULL throughout.
• Required headers (<shlobj.h>, <windows.h>) are already included via PreRTS.h in both translation units; no additional includes are needed.
• Minor: the new #else block in GeneralsMD/GlobalData.cpp uses tab indentation while the surrounding #if block uses 2-space indentation — purely cosmetic and does not affect correctness.

Confidence Score: 4/5

• This PR is safe to merge; the logic is correct and the previously-identified memory-leak issues have been resolved.
• Both files implement the same well-understood pattern correctly — pszPath is initialised to nullptr, CoTaskMemFree is unconditionally called after the success block, and the preprocessor guard is symmetrical. The only deduction is the minor tab/space indentation inconsistency in the GeneralsMD #else block, which has no runtime impact.
• No files require special attention.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/Common/GlobalData.cpp	Wraps legacy SHGetSpecialFolderPath in #if _MSC_VER < 1300 and adds a modern SHGetKnownFolderPath path for all other compilers. CoTaskMemFree is correctly placed outside the success-check block. Logic mirrors the existing code structure cleanly.
GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp	Same preprocessor guard + SHGetKnownFolderPath pattern as the Generals counterpart. Registry-based leaf name lookup is preserved. CoTaskMemFree is placed outside the success block correctly. New block uses tab indentation while the surrounding #if block uses 2-space indentation — minor inconsistency.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[parseGameDataDefinition / GlobalData constructor] --> B{_MSC_VER < 1300?}
    B -- Yes / VC6 --> C[SHGetSpecialFolderPath\nCSIDL_PERSONAL]
    C --> D{Success?}
    D -- No --> Z[m_userDataDir stays empty]
    D -- Yes --> E[Append backslash if needed]
    E --> F[Append leaf name + backslash]
    F --> G[CreateDirectory]
    G --> H[m_userDataDir = path]

    B -- No / modern compiler --> I[SHGetKnownFolderPath\nFOLDERID_Documents]
    I --> J[CoTaskMemFree pszPath]
    I --> K{SUCCEEDED and pszPath?}
    K -- No --> J
    K -- Yes --> L[AsciiString::translate from PWSTR]
    L --> M[Append backslash if needed]
    M --> N[Append leaf name + backslash]
    N --> O[CreateDirectory]
    O --> P[m_userDataDir = path]
    P --> J

Loading

_{Last reviewed commit: "bugfix(globaldata): ..."}

[Mauller]

Ah i didn't check building with VC6, will work on this further

[Mauller]

Updated by making the fix conditoinal so it only works in non VC6 builds

[Mauller]

Cleaned up the implementations by replicating the whole code block

[Mauller]

Removed controversial as this fix does not work in VC6 in the end anyway, so i had to make the fix conditional at compilation

[Skyaero42]

Can hardcoded path separators be an issue for (future) linux support?

[Mauller]

All of this will need altering to support linux in the future, the main functions for getting the documents folder are windows API.

[xezon]

Better use GetProcAddress(shell32module, "SHGetKnownFolderPath") to test the system for the function. This way the compiler condition is not required.

[Mauller]

Will this work with compiling in VC6?

[xezon]

I suggest move all this into a new function to tidy up this function:

TheWritableGlobalData->m_userDataDir = BuildUserDataPath();
CreateDirectory(TheWritableGlobalData->m_userDataDir.str(), nullptr);

[Mauller]

Yeah it makes sense

[xezon]

Simpler: !myDocumentsDirectory.endsWith("\\")

[Mauller]

Nice, i forgot we had those functions to use now.

[xezon]

You can probably consolidate the old string handling logic with the new logic.