Skip to content

Conversation

@Architector4
Copy link
Contributor

Ran cppcheck on the codebase and cleaned up most lints that it outputs.

The specific invocation I used:

cppcheck --project=compile_commands.json --file-filter=Source/* --check-level=exhaustive --enable=performance,information,missingInclude --suppress=missingIncludeSystem

(copy compile_commands.json from build dir to root of the project first of course)

Also includes some error handling fixups in AudioMan, some grammar fixes, and whatnot else Code Improvement™

bub fix 👍

Also includes some error handling fixups in AudioMan, some grammar
fixes, and whatnot else Code Improvement™
Copy link
Contributor

@HeliumAnt HeliumAnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly fine, just make it consistent. (Move semantics would technically be more universal, however that's only if the caller uses them and there's literally nothing in the project that would do that correctly)

@Architector4
Copy link
Contributor Author

Architector4 commented Dec 5, 2025

Mostly fine, just make it consistent. (Move semantics would technically be more universal, however that's only if the caller uses them and there's literally nothing in the project that would do that correctly)

I intentionally left/put pass-by-value & move code instead of const ref at these spots.

A const ref there would unavoidably cause a clone/reallocation anyway, since those are all assignments or lead to mutation, (like module ID lookup lowercasing the string).

Move semantics in these spots at least allow for possibility of avoiding a clone altogether, but aren't worse at all than const ref if the caller forgets to use move, except in visual inconsistency you pointed out.

For cases like this, I wonder if there's a compiler lint or a static analyzer which would highlight all the missed opportunities for moves. Then it'll all be truly optimal lol

@Architector4
Copy link
Contributor Author

For the record, I have to go sleep now; I'll fix the constness violation and whatnot else tomorrow lol

...at least the violation is not a very big deal since multiplayer is dead code anyway :v

Copy link
Contributor

@HeliumAnt HeliumAnt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gonna say ok on the move, just one small thing in the zipped modules.

@Causeless Causeless enabled auto-merge December 10, 2025 01:29
@Causeless Causeless dismissed HeliumAnt’s stale review December 10, 2025 01:32

Fixed the zippedModule

@Causeless Causeless added this pull request to the merge queue Dec 10, 2025
github-merge-queue bot pushed a commit that referenced this pull request Dec 10, 2025
cppcheck lints - change a lot of method params to use const ref, etc etc
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 10, 2025
@Architector4
Copy link
Contributor Author

meson update moment :D

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.

3 participants