Skip to content

docs: Add code standard guidelines for #include #1229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NoxMax wants to merge 2 commits into
base: master
Choose a base branch
from
Open

docs: Add code standard guidelines for #include #1229

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faaf59c
Update C++ code standards for includes and whitespace
NoxMax Jul 14, 2026
c4c979c
Bundled library bracket exception
NoxMax Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion docs/cpp-code-standards.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ if (a == b)

Trailing whitespace is not allowed.

You should also not have unneeded spaces within a bracket.
You should also not have unneeded spaces within a bracket.

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Clarify which delimiters this rule covers.

“Bracket” is ambiguous here, while the examples only demonstrate parentheses. Specify parentheses, square brackets, and braces to make the rule unambiguous.

Proposed wording 
-You should also not have unneeded spaces within a bracket.
+You should also avoid unneeded spaces inside parentheses, square brackets, and braces.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
You should also not have unneeded spaces within a bracket.
You should also avoid unneeded spaces inside parentheses, square brackets, and braces.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/cpp-code-standards.md` at line 56, Update the bracket-spacing rule in
the C++ standards documentation to explicitly cover parentheses, square
brackets, and braces, rather than using the ambiguous term “bracket.” Keep the
existing guidance about avoiding unnecessary spaces within these delimiters.


Wrong:

Expand Down Expand Up @@ -330,6 +330,36 @@ All Header files should contain header guards
#endif // MY_HEADER_H
```

### Includes

Every header must be **self-contained**: it must compile on its own, without depending on other headers being included before it. This prevents hidden fragile dependencies. If "A" needs "B" and "C", it should include "B" & "C". It shouldn't only include "B", just because "B" happened to include "C" right now. "A" should also not include anything it doesn't directly use.

Includes are written as a single block with no blank lines inside it, ordered as follows:

1. The file's own header first in the .cpp (if the .cpp needs to include its own header).
2. All other project headers, in alphabetical order.
3. All library headers, in alphabetical order.

ItemEnchantmentMgr.cpp example:

```cpp
#include "ItemEnchantmentMgr.h" // The file's own header, first
#include "DBCStores.h" // then project headers, alphabetically
#include "DatabaseEnv.h"
Comment on lines +347 to +348

@Kitzunu Kitzunu Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be the other way around

@NoxMax NoxMax Jul 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

With ASCII sorting? It is essentially sorting "D B C Stores" then "Database Env". D will come before Database.

@Kitzunu Kitzunu Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We have, to my knowledge, always enforced alphabetical sorting

@NoxMax NoxMax Jul 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is always alphabetical, but question is whether its case sensitive alphabetical (ASCII) or insensitive. ASCII sort will place DBCStores sort first. Insensitive will treat it both the same as "dbcstores" and "databaseenv", which would place DatabaseEnv first. Other codebase examples similar to what's in ItemEnchantmentMgr.cpp
ConditionMgr.cpp:

#include "ScriptMgr.h"
#include "ScriptedCreature.h"

GroupHandler.cpp:

#include "LFGMgr.h"
#include "Language.h"

Beyond my general observation, I asked Claude to do an actual analysis of sort orders. It did find examples where sorting is case insensitive. GridTerrainLoader.cpp:

#include "Map.h"
#include "MMapMgr.h"

But the vast majority of examples it found (127 vs 5) favoured case sensitive sorting.

#include "Log.h"
#include "ObjectMgr.h"
#include "QueryResult.h"
#include "Timer.h"
#include "Util.h"
#include <cmath> // then library headers, alphabetically
#include <functional>
#include <vector>
Comment on lines +346 to +356

@Kitzunu Kitzunu Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be project -> lib -> standard C headers

@NoxMax NoxMax Jul 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That would be the Google convention, but I tried to formalize the order based on what AC already uses, which is mostly LLVM. Every single case I found in the codebase, libraries were always in the bottom. Though if you and the other staff think the Google style is what AC should adopt and everything should be eventually changed to match, then yeah sure we can adopt that.

Personally I prefer Google, but I deferred to what's already there.

@Kitzunu Kitzunu Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

isnt the google style lib/standard C at the top?

@NoxMax NoxMax Jul 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yeah it is. But without exception all the examples I found in AC place libraries at the bottom. Google also breaks the include block into sections https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes but AC uses a single block.

@Kitzunu Kitzunu Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

perhaps I get it mixed and c standard should be before lib. probably doesnt matter

@NoxMax NoxMax Jul 14, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It's no more than a style choice. AC uses LLVM mostly with libs in the bottom, and its the other way around from Google. Ultimately though, yeah it's mostly about style consistency.

@Kitzunu Kitzunu Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

then I dont care :P

```

Alphabetical ordering is case-sensitive (ASCII order, matching clang-format's default): uppercase letters sort before lowercase. Above, `DBCStores.h` precedes `DatabaseEnv.h` because the uppercase `B` sorts before the lowercase `a`.

Project headers use quotes (`"..."`); library headers (C++ standard library, boost, etc.) use angle brackets (`<...>`). A third-party library bundled into the codebase, such as G3D, is the exception and uses quotes.

### Text Output

All C++ script/system/command text output must use [acore_string](acore_string) whenever possible (e.g., ChatHandler messages, script system messages).
Expand Down