Skip to content

Project modernization#630

Merged
void2012 merged 13 commits intosmartcmd:mainfrom
ModMaker101:feat/project-modernization
Mar 8, 2026
Merged

Project modernization#630
void2012 merged 13 commits intosmartcmd:mainfrom
ModMaker101:feat/project-modernization

Conversation

@ModMaker101
Copy link
Contributor

Description

This replaces all C-style casts in the codebase with their appropriate C++-style equivalent. This improves type safety.

Changes

Previous Behavior

The codebase relied on C-style cases like crazy. The C-style casts lack type safety and can performant multiple types of conversions implicitly.

Root Cause

The original code used C-style cases because they were new/didn't exist when this was written. This over time can make it harder to program especially when it in C++.

New Behavior

Most C=style casts now use their C++ equivalent.

Fix Implementation

  • Went through the codebase and replaced most of the C-style casts with C++ ones.

AI Use Disclosure

No.

@ModMaker101
Copy link
Contributor Author

Dang it why is the boat commit there. Oh well.

@ModMaker101
Copy link
Contributor Author

Ready to merge.

@EnderActually
Copy link
Contributor

Dang it why is the boat commit there. Oh well.

Should have probably made a separate fork to prevent that.

@void2012
Copy link
Collaborator

void2012 commented Mar 5, 2026

Please keep the old boat code because we try to achieve feature accuracy

@void2012 void2012 marked this pull request as draft March 5, 2026 23:52
@ModMaker101
Copy link
Contributor Author

That boat code is from this Pr #615 its a fix I just made a new branch based of my boat fix branch on accident

@ModMaker101 ModMaker101 marked this pull request as ready for review March 6, 2026 00:06
@ModMaker101 ModMaker101 marked this pull request as draft March 6, 2026 00:06
@ModMaker101
Copy link
Contributor Author

If you don't mind me asking why did you make the PR a draft?

@void2012
Copy link
Collaborator

void2012 commented Mar 6, 2026

There are examples where int is used in place of where size_t is used, especially for object sizes, array subscripts and std::wstring::find_* functions. Could you please work on this, too? For the sake of the name of this PR. This may also hunt some obscure bugs.

@void2012
Copy link
Collaborator

void2012 commented Mar 6, 2026

Also, I think it makes sense to get rid off NULL, std::shared_ptr(new(...)) in favor of std::make_shared and other kinds of modernizations that can be done (semi-)automatically with clang-tidy(although some of those should be checked manually because sometimes it breaks the code. I think all of it can be done in this PR.

@ModMaker101
Copy link
Contributor Author

Alright I'll work on it.

@void2012
Copy link
Collaborator

void2012 commented Mar 6, 2026

I enabled all modernize-* checks in .clang-tidy but today plan to add even more checks for aggressive standard conforming refactoring

ModMaker101 and others added 6 commits March 6, 2026 13:59
- Replaced most instances of `NULL` with `nullptr`.
- Replaced most `shared_ptr(new ...)` with `make_shared`.
- Removed the `nullptr` macro as it was interfering with the actual nullptr keyword in some instances.
@ModMaker101
Copy link
Contributor Author

@void2012 You want me to work on renaming methods to match our current style?

@ModMaker101
Copy link
Contributor Author

@void2012 We should probably merge this PR ASAP because in the future if I were to add more "modernization" it could break existing PRs/fixes which could lead to more friction. I have already implemented int -> size_t, more static_cast to replace C-style ones, replaced NULL with nullptr in most cases, and a few other things I was going to start on before recognizing that this could lead to friction. Would it be possible to review this PR and merge ASAP.

@ModMaker101 ModMaker101 marked this pull request as ready for review March 8, 2026 02:28
@ModMaker101
Copy link
Contributor Author

@smartcmd if you could also review this that would be helpful.

@void2012 void2012 merged commit a9be52c into smartcmd:main Mar 8, 2026
1 check passed
@codeHusky
Copy link
Collaborator

codeHusky commented Mar 8, 2026

Getting reports that this breaks pausing and tab menu. confirm i can't make a new world without the game crashing on latest release. prob gonna have to revert this, was not tested
image

codeHusky added a commit that referenced this pull request Mar 8, 2026
codeHusky added a commit that referenced this pull request Mar 8, 2026
This code was not tested and breaks in Release builds, reverting to restore
functionality of the nightly. All in-game menus do not work and generating
a world crashes.

This reverts commit a9be52c.
@ModMaker101
Copy link
Contributor Author

Must be an issue with nullptr somewhere

@ModMaker101
Copy link
Contributor Author

I'll take a look

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants