Skip to content

Conversation

@DexrnZacAttack
Copy link
Collaborator

@DexrnZacAttack DexrnZacAttack commented Nov 19, 2025

This will allow for IDEs to enforce the naming system, and we can add a GitHub action to verify that the naming system is being followed.

This .clang-tidy enforces:

constexpr int CONST_GLOBAL_VAR = 0;
int gGlobalVar = 0;

class Something {
    static constexpr int CONST_CLASS_MEMBER = 0;
    static int sStaticClassMember;

public:
    int mPublicField;
protected:
    int mProtectedField;
private:
    int mPrivateField;
}

@GRAnimated
Copy link
Owner

LGTM! I'm a little worried about static names that we have from Java Edition (such as LevelRenderer::allChanged and similar from b1.2) but I don't mind if we change them to make this repo consistent with itself.

@GRAnimated
Copy link
Owner

If Bagietas doesn't suggest any changes, you'll be good to merge. Otherwise, make those changes and then merge.

@DexrnZacAttack
Copy link
Collaborator Author

LGTM! I'm a little worried about static names that we have from Java Edition (such as LevelRenderer::allChanged and similar from b1.2) but I don't mind if we change them to make this repo consistent with itself.

We can add // NOTIDY to avoid changing them if needed.

@Bagietas
Copy link
Collaborator

Later today I will see if there's anything else we can enforce here.

@Bagietas
Copy link
Collaborator

Bagietas commented Nov 20, 2025

We can add // NOTIDY to avoid changing them if needed.

I think if we decide for that enforcement, we should keep it consistent and apply to every name.

@DexrnZacAttack
Copy link
Collaborator Author

We can add // NOTIDY to avoid changing them if needed.

I think if we decide for that enforcement, we should keep it consistent and apply to every name.

Sounds good.

After this is merged, I will implement an actions thing to make sure there's no clang-tidy issues, and then I'll fix all the issues that clang-tidy reports (in another PR)

@DexrnZacAttack
Copy link
Collaborator Author

I wonder if we should switch to the more common m_ style, I think 4J used it because I remember seeing some kind of assert or something iirc that contained it

@Bagietas
Copy link
Collaborator

I wonder if we should switch to the more common m_ style, I think 4J used it because I remember seeing some kind of assert or something iirc that contained it

That's what I suggested long time ago but GRA was against it, don't remember what was the reason.

@DexrnZacAttack
Copy link
Collaborator Author

I wonder if we should switch to the more common m_ style, I think 4J used it because I remember seeing some kind of assert or something iirc that contained it

That's what I suggested long time ago but GRA was against it, don't remember what was the reason.

Shouldn't be too hard to switch in the future should we decide otherwise.

@Bagietas
Copy link
Collaborator

Bagietas commented Nov 21, 2025

What I suggest to add:

readability-inconsistent-declaration-parameter-name
readability-misleading-indentation
readability-math-missing-parentheses

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