Skip to content

C++23 Compiler Compatible#715

Closed
Racc-Boi wants to merge 7 commits intosmartcmd:mainfrom
Racc-Boi:main
Closed

C++23 Compiler Compatible#715
Racc-Boi wants to merge 7 commits intosmartcmd:mainfrom
Racc-Boi:main

Conversation

@Racc-Boi
Copy link

@Racc-Boi Racc-Boi commented Mar 6, 2026

Description

This PR modernizes the codebase to support the C++23 standard, ensuring that the project can be compiled with modern toolchains while maintaining compatibility with legacy libraries and build environments.

Changes

  • Modernized Sorting Logic: Removed inheritance from std::binary_function in DistanceChunkSorter.h and DirtyChunkSorter.h.
  • Keyword Deconfliction: Renamed variables/methods named requires to requirements to avoid conflict with the C++20 requires keyword.
  • Standard Library Updates: Replaced the custom byte typedef with uint8_t to resolve ambiguity with std::byte.
  • Strict Type Safety: Implemented const_cast for string literals passed to legacy SDK functions (PIX, 4J Storage) and fixed std::wstring initialization in Screen.cpp.
  • Modernized Randomness: Replaced deprecated std::random_shuffle in Villager.cpp with std::shuffle using a proper Mersenne Twister engine.
  • Control Flow: Refactored MakeTextureFromResource to remove goto in favor of structured SUCCEEDED logic.

Previous Behavior

Compiling with C++23 resulted in several fatal errors:

  • Older C++ standards allowed string literals to be assigned directly to non-const char* pointers. However, C++23 strictly forbids this, which is why we must now explicitly use const char* or safely cast them for legacy APIs.
  • The removal of deprecated standard library features (std::binary_function and std::random_shuffle) caused build failures.
  • The introduction of the reserved keyword requires in C++20 triggered syntax errors where older code used it as an identifier.

Root Cause

C++17 and C++20 standards removed several legacy features and introduced new keywords. Modern compilers strictly enforce the const-correctness of string literals (treating them strictly as const char[]) to prevent undefined behavior from attempting to modify read-only memory.

New Behavior

  • The project compiles cleanly in C++23.
  • Sorting and shuffling logic remains functionally identical but uses standard-compliant methods.
  • String literal assignments are now fully type-safe.

Fix Implementation

  • DistanceChunkSorter.h / DirtyChunkSorter.h: Removed the base class as in C++11 and later, standard algorithms only require the operator() and do not need the legacy typedefs provided by binary_function.
  • Villager.cpp: Replaced the C-style random_shuffle with std::shuffle as the modern standard equivalent and provides better randomness via std::mt19937.
  • UIScene_LoadOrJoinMenu.cpp: Applied const_cast<char*> to string literals to satisfy the compiler's strict string rules while maintaining the specific pointer types required by legacy 4J library functions.

AI Use Disclosure

No AI was used to write the code in this PR. All changes were manually implemented based on technical documentation regarding C++ standard deprecations and MSVC linker behavior.

Related Issues

N/A

Images showing random still works fine:
image
image

@void2012
Copy link
Collaborator

void2012 commented Mar 6, 2026

This one is huge and oh boy I waited for PR like this. Just make sure we don't drop C++14 yet until we resolve the XBOX SDK situation.

Consider collaborating with @ModMaker101 of #630 ?

@void2012 void2012 added priority: EXTREME good PR Exceptionally good Pull Request labels Mar 6, 2026
@void2012
Copy link
Collaborator

void2012 commented Mar 6, 2026

I see that you mechanically replaced all requires with requirements, but this broke many comments grammar and meaning. Please do not do that to comments.

@ModMaker101
Copy link
Contributor

Should we finish #630 before we do this or what? @void2012

@void2012
Copy link
Collaborator

void2012 commented Mar 6, 2026

This PR scope seems smaller, so it's rather vice versa. Yours is fundamentally larger.

@ModMaker101
Copy link
Contributor

Well, I really don't want to resolve all those conflicts...

@Racc-Boi
Copy link
Author

Racc-Boi commented Mar 6, 2026

I'll fix the grammar requirements when I'm back at my pc later for the types to uint8_t is requirements I had just use ctrl+h so that's my bad I apologise

@Racc-Boi
Copy link
Author

Racc-Boi commented Mar 6, 2026

This one is huge and oh boy I waited for PR like this. Just make sure we don't drop C++14 yet until we resolve the XBOX SDK situation.

Consider collaborating with @ModMaker101 of #630 ?

I wouldn't mind collaboration especially if it makes things easier

@ModMaker101
Copy link
Contributor

There's not much I would need collaboration with right this second as most of it is small repetitive work that I have tooling that does it for me.

@Racc-Boi
Copy link
Author

Racc-Boi commented Mar 6, 2026

There's not much I would need collaboration with right this second as most of it is small repetitive work that I have tooling that does it for me.

My thought is helping with things such as modernising the particle engine such as how it uses new to spawn them and likely iterates through vectors of raw Particle* to delete them we can replace with std::unique_ptr to guarantee no accidental memory leaks and using std::erase_if which i believe should provide a massive vector optimization compared to the manual loop erasures or like std::string_view with tolower to have zero allocation overhead

@Racc-Boi Racc-Boi marked this pull request as draft March 6, 2026 20:59
@Racc-Boi Racc-Boi marked this pull request as ready for review March 7, 2026 17:02
@Racc-Boi Racc-Boi marked this pull request as draft March 7, 2026 17:14
@void2012
Copy link
Collaborator

void2012 commented Mar 7, 2026

Someone send me a simpler fix so sorry,I'm closing this one

@void2012 void2012 closed this Mar 7, 2026
@Racc-Boi
Copy link
Author

Racc-Boi commented Mar 7, 2026

we can do a simpler fix by defining what byte should be and disabling strict chars but doing that isnt safe memory wise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good PR Exceptionally good Pull Request priority: EXTREME

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants