Coding Style (OpenApoc): Difference between revisions
No edit summary |
Re-add 9 coding conventions from CODE_STYLE.md that were missing: comments style, trivial one-line functions, ownership guidance, static_assert, iterator exception, forward declaration limits, Yoda conditionals, numeric_limits, short variable names |
||
| (3 intermediate revisions by 2 users not shown) | |||
| Line 1: | Line 1: | ||
This document specifies the guidelines for writing and formatting the | {{DISPLAYTITLE:Coding Style (OpenApoc)}} | ||
This document specifies the guidelines for writing and formatting the C++ code that forms the core of '''[[OpenApoc]]'''. | |||
OpenApoc uses '''C++17'''. This requires reasonably modern compilers ('''GCC 8+''', '''MSVC 2019+''', '''Clang 7+''' have been tested). You should avoid using compiler-specific extensions where possible. Exceptions exist, but should be wrapped in an <code>#ifdef</code>. | |||
< | |||
Code formatting is enforced by '''[https://clang.llvm.org/docs/ClangFormat.html clang-format]''' (version '''18'''). The configuration file <code>.clang-format</code> in the root of the OpenApoc source repository defines the project style. | |||
// | |||
</ | |||
Static analysis is enforced by '''[https://clang.llvm.org/extra/clang-tidy/ clang-tidy]''' (version '''18'''). The CI pipeline rejects unformatted code and code that fails tidy checks. | |||
It is highly recommended to run <code>clang-format</code> on all modified files before committing: | |||
<pre>clang-format -i path/to/file.cpp path/to/file.h</pre> | |||
< | When run from the root of the OpenApoc source repository, it automatically uses the supplied <code>.clang-format</code> configuration file. | ||
clang-format | |||
When | When using the CMake build system, there are dedicated targets: | ||
<pre> | <pre> | ||
# Format all source files (from build directory) | |||
cmake --build build -t format-sources | |||
# Run static analysis (from build directory) | |||
cmake --build build -t tidy | |||
</pre> | </pre> | ||
== Indent == | |||
* Tabs for indenting, spaces for alignment, indenting by 1 tab for each new scope | |||
<pre> | <pre> | ||
void function() | void function() | ||
| Line 43: | Line 33: | ||
reallyLongFunctionNameWithLotsOfArguments(argOne, argTwo, | reallyLongFunctionNameWithLotsOfArguments(argOne, argTwo, | ||
argThree); | argThree); | ||
} | } | ||
</pre> | </pre> | ||
* Avoid going over 100 | |||
** If you find yourself going over this it's often a hint to try to pull things out of loops/into functions | * Avoid going over '''100 columns''' (at tab width of 4 spaces) | ||
** Don't break strings up to fit this | ** If you find yourself going over this, it's often a hint to try to pull things out of loops / into functions | ||
** Don't break strings up to fit this; it looks ugly and makes things harder to read | |||
* If you have to break, indent the following line by an extra tab | * If you have to break, indent the following line by an extra tab | ||
** When breaking a single statement, break the line before the next operator. Avoid having an operator as the last thing on a line. | ** When breaking a single statement, break the line '''before''' the next operator. Avoid having an operator as the last thing on a line. | ||
<pre> | <pre> | ||
void | void reallyLongFunctionNameIMeanThisIsReallyBadlyNamed(int parameterOne, | ||
int paramTwo, char theThirdOne) | int paramTwo, char theThirdOne) | ||
{ | { | ||
if (parameterOne == | if (parameterOne == yetAnotherReallyLongCondition | ||
&& youHaveBetterThingsToDo) | && youHaveBetterThingsToDo) | ||
{ | { | ||
| Line 63: | Line 55: | ||
== Whitespace == | == Whitespace == | ||
* Spaces before and after operators | * Spaces before and after operators | ||
<pre> | <pre> | ||
a = b; | a = b; | ||
| Line 69: | Line 63: | ||
a + b; | a + b; | ||
</pre> | </pre> | ||
* Space after if/ | |||
* Space after <code>if</code>/<code>while</code>/<code>else</code>/<code>for</code>, space after <code>:</code>/<code>;</code> in <code>for</code> | |||
<pre> | <pre> | ||
for (auto &a: b) | for (auto &a : b) | ||
</pre> | </pre> | ||
* No spaces after function name (or function-like keywords like | |||
* No spaces after function name (or function-like keywords like <code>sizeof</code>), but space after flow control keywords, space after comma for multiple arguments | |||
<pre> | <pre> | ||
func(a, b); | func(a, b); | ||
if (a == 0) | if (a == 0) | ||
</pre> | </pre> | ||
* References and pointers: & and* | |||
* References and pointers: <code>&</code> and <code>*</code> align right (to the variable), not to the type | |||
<pre> | <pre> | ||
float | float *pointerToFloat; | ||
Type &ref; | |||
</pre> | </pre> | ||
== Scope == | == Scope == | ||
* Indent 1 tab for each new scope | * Indent 1 tab for each new scope | ||
* New scope is | * New scope is '''always''' surrounded by <code>{}</code> braces, even for single-statement blocks | ||
* | * Opening brace <code>{</code> goes on the '''next''' line at the indent of the enclosing scope ('''Allman style''') | ||
* | * Closing brace <code>}</code> at the same indent as the opening <code>{</code> | ||
* New | * New scopes are caused by: | ||
** Functions | ** Functions | ||
** Conditional blocks (<code>if</code>/<code>else</code>/<code>while</code>/<code>for</code>) | |||
** <code>switch</code>/<code>case</code> | |||
<pre> | <pre> | ||
void functionDefinition() | void functionDefinition() | ||
| Line 96: | Line 101: | ||
} | } | ||
</pre> | </pre> | ||
<pre> | <pre> | ||
if (x) | if (x) | ||
| Line 110: | Line 115: | ||
doThatLastThing(); | doThatLastThing(); | ||
} | } | ||
</pre> | |||
* <code>switch</code> always has a <code>default</code> case unless switching over an <code>enum class</code> where every value is handled | |||
* All <code>case</code> sections should have a <code>break</code> | |||
* <code>case</code> braces <code>{}</code> are optional, based on whether new stack variables are needed | |||
<pre> | <pre> | ||
switch (a) | switch (a) | ||
| Line 124: | Line 129: | ||
case B: | case B: | ||
{ | { | ||
auto | auto newVariable = getSomething(); | ||
useIt(newVariable); | |||
break; | break; | ||
} | } | ||
default: | default: | ||
break; | break; | ||
} | } | ||
</pre> | </pre> | ||
* | * '''Exception''': Trivial functions (a single statement fitting within 100 columns) may use the definition and body on one line: | ||
<pre> | <pre> | ||
int getX() const { return x; } | |||
int | |||
</pre> | </pre> | ||
== | This is enforced by <code>clang-format</code> (<code>AllowShortFunctionsOnASingleLine: All</code>). | ||
< | == Naming == | ||
</ | {| class="wikitable" | ||
! Style !! Used For | |||
< | |- | ||
class | | <code>CamelCase</code> || Classes, enums, <code>enum class</code> members, namespaces, template parameters | ||
</ | |- | ||
| <code>camelBack</code> || Methods, member variables, function parameters, local variables | |||
|- | |||
| <code>SHOUTY_CAPS</code> || Constants, macros | |||
|- | |||
| <code>lower_case</code> || Labels | |||
|} | |||
</ | Short variable names (<code>i</code>, <code>x</code>, <code>y</code>) are fine when the meaning is obvious from context. | ||
< | |||
</ | |||
< | |||
</ | |||
< | |||
</ | |||
< | |||
</ | |||
< | |||
</ | |||
== Types == | == Types == | ||
* Use <code>auto</code> liberally, especially when the type is obvious from the right-hand side. Use <code>auto &</code> to avoid copies. | |||
</ | * Use <code>enum class</code> over plain <code>enum</code> | ||
* | * Use <code>struct</code> only for data-only types; structs must '''never''' use access modifiers | ||
* Use | * No C-style casts — use <code>static_cast<></code>, <code>dynamic_cast<></code>, <code>reinterpret_cast<></code> | ||
* Prefer <code>{}</code> brace initialization | |||
* Prefer <code>std::numeric_limits<></code> over C-style <code>MAX_INT</code> / <code>INT_MAX</code> macros | |||
== Smart Pointers == | |||
OpenApoc provides short aliases in <code>library/sp.h</code>: | |||
{| class="wikitable" | |||
! Alias !! Equivalent | |||
|- | |||
| <code>sp<T></code> || <code>std::shared_ptr<T></code> | |||
|- | |||
| <code>up<T></code> || <code>std::unique_ptr<T></code> | |||
|- | |||
| <code>wp<T></code> || <code>std::weak_ptr<T></code> | |||
|- | |||
| <code>mksp<T>(args...)</code> || <code>std::make_shared<T>(args...)</code> | |||
|- | |||
| <code>mkup<T>(args...)</code> || <code>std::make_unique<T>(args...)</code> | |||
|} | |||
* '''No naked <code>new</code>''' — always wrap in smart pointers immediately | |||
* Prefer <code>up<></code> (exclusive ownership) over <code>sp<></code> unless shared ownership is genuinely needed | |||
* Use <code>std::move()</code> to transfer <code>up<></code> ownership | |||
* '''Ownership guideline''': If ownership of a member is tied to the class, use a direct member (no pointer). If a reference owned by another object is guaranteed to outlive the class, use a <code>&reference</code> member. Otherwise use <code>up<></code> or <code>sp<></code> depending on whether ownership is exclusive or shared. | |||
== Templates == | |||
* Template type parameters should use <code>CamelCase</code> | |||
* Place <code>template<...></code> on the line above the function/class declaration | |||
<pre> | <pre> | ||
template <typename ValueType> | |||
ValueType doSomething(ValueType input) | |||
{ | { | ||
return input; | |||
} | } | ||
</pre> | </pre> | ||
== Class Declarations == | |||
* <code>public:</code>/<code>private:</code>/<code>protected:</code> written at class indent level (not indented further) | |||
* | * Always explicitly write <code>private:</code>, even though it is the default | ||
* | * <code>virtual</code> only on the base class; <code>override</code> on derived classes — never both together | ||
* All classes with virtual methods must have a virtual destructor | |||
< | * Use <code>= default</code> instead of empty <code>{}</code> constructor/destructor bodies | ||
* Use member initializer lists; initializer order must match declaration order | |||
</ | |||
* | |||
< | |||
</ | |||
<pre> | <pre> | ||
class MyClass | class MyClass : public BaseClass | ||
{ | { | ||
private: | private: | ||
int | int memberVariable = 0; | ||
UString name; | |||
public: | public: | ||
void | MyClass() = default; | ||
~MyClass() override = default; | |||
void doSomething() override; | |||
int getValue() const; | |||
}; | }; | ||
</pre> | </pre> | ||
* ' | |||
** | == Functions == | ||
* | |||
* Functions should be named <code>camelBack</code> | |||
* Use '''early return''' — prefer separate <code>if (cond) return;</code> checks over deeply nested conditionals | |||
* '''<code>const</code> aggressively''': const member functions, const parameters, const return types, const local variables | |||
* Range-for loops: <code>for (auto &element : container)</code> / <code>for (const auto &element : container)</code> | |||
** '''Exception''': When removing elements during iteration, use an iterator-based loop instead of range-for | |||
* Prefer <code>emplace()</code> over <code>insert()</code> in STL containers | |||
== General Code == | |||
* Use anonymous namespaces instead of <code>static</code> for file-local functions and classes | |||
<pre> | <pre> | ||
namespace | |||
{ | { | ||
void helperFunction() | |||
{ | { | ||
// file-local helper | |||
} | |||
} | } // anonymous namespace | ||
</pre> | </pre> | ||
* | * All project code lives in <code>namespace OpenApoc {}</code>. Namespace content is '''not''' indented. Closing brace gets a comment: | ||
<pre> | <pre> | ||
namespace OpenApoc | |||
{ | { | ||
class MyClass | class MyClass | ||
{ | { | ||
// ... | |||
}; | }; | ||
} // namespace OpenApoc | |||
</pre> | </pre> | ||
* | * Use <code>static_assert()</code> to verify assumptions about alignment, packing (e.g., when reading structs from original game files), or template restrictions | ||
< | * Avoid Yoda conditionals — write <code>var == 1</code>, not <code>1 == var</code> | ||
== Comments == | |||
* Prefer <code>//</code> for single-line comments | |||
</ | * Multi-line <code>/* */</code> comments: align the <code>*</code> on each intermediate line | ||
** | * Use <code>//TODO:</code> and <code>//FIXME:</code> markers to flag work-in-progress or known issues | ||
* Do not comment for the sake of it — if the code can be made clearer, prefer that over adding a comment | |||
== Logging == | |||
Uses '''fmt-style format strings''' with positional <code>{0}</code>, <code>{1}</code> placeholders ('''not''' printf-style): | |||
<pre> | <pre> | ||
#include "framework/logger.h" | |||
{ | |||
LogInfo("Loaded mod \"{0}\"", modName); | |||
LogWarning("Value {0} exceeds limit {1}", value, limit); | |||
LogError("Failed to load file \"{0}\"", path); | |||
</pre> | </pre> | ||
* | |||
** | * <code>LogInfo</code> — general information | ||
* <code>LogWarning</code> — recoverable errors | |||
* <code>LogError</code> — fatal errors | |||
== Strings == | |||
Use '''<code>UString</code>''' (from <code>library/strings.h</code>) for all strings. All <code>char*</code>/<code>std::string</code> values are assumed UTF-8. | |||
String formatting uses the <code>fmt</code> library: | |||
<pre> | <pre> | ||
#include "library/strings_format.h" | |||
UString result = OpenApoc::format("Player has {0} credits and {1} agents", credits, agentCount); | |||
</pre> | </pre> | ||
Translation: | |||
<pre> | <pre> | ||
UString translated = tr("English text to translate"); | |||
</pre> | </pre> | ||
** | |||
== Includes and Headers == | |||
* Use <code>#pragma once</code> (no traditional include guards) | |||
* '''Include order''': Local headers first, then system headers. Each block alphabetically sorted. | |||
* Local headers use paths '''relative to the OpenApoc root''': <code>"framework/logger.h"</code>, not <code>"../logger.h"</code> or <code>"logger.h"</code> | |||
* '''Prefer forward declarations''' over <code>#include</code> in headers where possible | |||
** Forward declarations are '''not possible''' for templates, non-class types (e.g., <code>enum class</code>), and superclasses. When in doubt, try building without the include and see what fails. | |||
<pre> | <pre> | ||
#pragma once | |||
#include "library/sp.h" | |||
#include "library/strings.h" | |||
#include <vector> | |||
namespace OpenApoc | |||
{ | { | ||
class ForwardDeclaredType; | |||
class MyClass | class MyClass | ||
{ | { | ||
private: | |||
int member = 0; | |||
public: | public: | ||
void publicFunction(); | |||
}; | }; | ||
} // namespace OpenApoc | |||
</pre> | </pre> | ||
== Key Review Expectations == | |||
These patterns are consistently enforced during code review: | |||
# '''const correctness''' — if something can be <code>const</code>, it must be <code>const</code> | |||
# '''Readable conditionals''' — avoid embedded comments in complex conditionals; prefer early-exit checks | |||
# '''One logical change per PR''' — keep changes focused for clean history and bisectability | |||
< | # '''Use <code>auto</code>''' — when the type is already visible on the RHS | ||
# '''Prefer exclusive ownership''' — use <code>up<></code> over <code>sp<></code> when shared ownership is not required | |||
</ | |||
== | == See Also == | ||
* [[OpenApoc]] — Main OpenApoc page | |||
* [[Compiling (OpenApoc)]] — Build instructions for developers | |||
** | * [https://github.com/OpenApoc/OpenApoc/blob/master/CODE_STYLE.md CODE_STYLE.md] — Full coding style specification in the repository | ||
* [https://github.com/ayrtondenner/OpenApoc-wiki/blob/master/development/coding-style.md OpenApoc Wiki: Coding Style] — Extended coding style documentation | |||
* [https://github.com/OpenApoc/OpenApoc OpenApoc on GitHub] — Source code repository | |||
* [https://discord.gg/f8Rayre OpenApoc Discord] — Community discussion and support | |||
[[Category:OpenApoc]] | [[Category:OpenApoc]] | ||
Latest revision as of 23:05, 1 March 2026
This document specifies the guidelines for writing and formatting the C++ code that forms the core of OpenApoc.
OpenApoc uses C++17. This requires reasonably modern compilers (GCC 8+, MSVC 2019+, Clang 7+ have been tested). You should avoid using compiler-specific extensions where possible. Exceptions exist, but should be wrapped in an #ifdef.
Code formatting is enforced by clang-format (version 18). The configuration file .clang-format in the root of the OpenApoc source repository defines the project style.
Static analysis is enforced by clang-tidy (version 18). The CI pipeline rejects unformatted code and code that fails tidy checks.
It is highly recommended to run clang-format on all modified files before committing:
clang-format -i path/to/file.cpp path/to/file.h
When run from the root of the OpenApoc source repository, it automatically uses the supplied .clang-format configuration file.
When using the CMake build system, there are dedicated targets:
# Format all source files (from build directory) cmake --build build -t format-sources # Run static analysis (from build directory) cmake --build build -t tidy
Indent
- Tabs for indenting, spaces for alignment, indenting by 1 tab for each new scope
void function()
{
reallyLongFunctionNameWithLotsOfArguments(argOne, argTwo,
argThree);
}
- Avoid going over 100 columns (at tab width of 4 spaces)
- If you find yourself going over this, it's often a hint to try to pull things out of loops / into functions
- Don't break strings up to fit this; it looks ugly and makes things harder to read
- If you have to break, indent the following line by an extra tab
- When breaking a single statement, break the line before the next operator. Avoid having an operator as the last thing on a line.
void reallyLongFunctionNameIMeanThisIsReallyBadlyNamed(int parameterOne,
int paramTwo, char theThirdOne)
{
if (parameterOne == yetAnotherReallyLongCondition
&& youHaveBetterThingsToDo)
{
doWhatever();
}
}
Whitespace
- Spaces before and after operators
a = b; a && b; a + b;
- Space after
if/while/else/for, space after:/;infor
for (auto &a : b)
- No spaces after function name (or function-like keywords like
sizeof), but space after flow control keywords, space after comma for multiple arguments
func(a, b); if (a == 0)
- References and pointers:
&and*align right (to the variable), not to the type
float *pointerToFloat; Type &ref;
Scope
- Indent 1 tab for each new scope
- New scope is always surrounded by
{}braces, even for single-statement blocks - Opening brace
{goes on the next line at the indent of the enclosing scope (Allman style) - Closing brace
}at the same indent as the opening{ - New scopes are caused by:
- Functions
- Conditional blocks (
if/else/while/for) switch/case
void functionDefinition()
{
newScopeHere();
}
if (x)
{
doWhatever();
}
else if (y)
{
doWhateverTheSecond();
}
else
{
doThatLastThing();
}
switchalways has adefaultcase unless switching over anenum classwhere every value is handled- All
casesections should have abreak casebraces{}are optional, based on whether new stack variables are needed
switch (a)
{
case A:
doSomething();
break;
case B:
{
auto newVariable = getSomething();
useIt(newVariable);
break;
}
default:
break;
}
- Exception: Trivial functions (a single statement fitting within 100 columns) may use the definition and body on one line:
int getX() const { return x; }
This is enforced by clang-format (AllowShortFunctionsOnASingleLine: All).
Naming
| Style | Used For |
|---|---|
CamelCase |
Classes, enums, enum class members, namespaces, template parameters
|
camelBack |
Methods, member variables, function parameters, local variables |
SHOUTY_CAPS |
Constants, macros |
lower_case |
Labels |
Short variable names (i, x, y) are fine when the meaning is obvious from context.
Types
- Use
autoliberally, especially when the type is obvious from the right-hand side. Useauto &to avoid copies. - Use
enum classover plainenum - Use
structonly for data-only types; structs must never use access modifiers - No C-style casts — use
static_cast<>,dynamic_cast<>,reinterpret_cast<> - Prefer
{}brace initialization - Prefer
std::numeric_limits<>over C-styleMAX_INT/INT_MAXmacros
Smart Pointers
OpenApoc provides short aliases in library/sp.h:
| Alias | Equivalent |
|---|---|
sp<T> |
std::shared_ptr<T>
|
up<T> |
std::unique_ptr<T>
|
wp<T> |
std::weak_ptr<T>
|
mksp<T>(args...) |
std::make_shared<T>(args...)
|
mkup<T>(args...) |
std::make_unique<T>(args...)
|
- No naked
new— always wrap in smart pointers immediately - Prefer
up<>(exclusive ownership) oversp<>unless shared ownership is genuinely needed - Use
std::move()to transferup<>ownership - Ownership guideline: If ownership of a member is tied to the class, use a direct member (no pointer). If a reference owned by another object is guaranteed to outlive the class, use a
&referencemember. Otherwise useup<>orsp<>depending on whether ownership is exclusive or shared.
Templates
- Template type parameters should use
CamelCase - Place
template<...>on the line above the function/class declaration
template <typename ValueType>
ValueType doSomething(ValueType input)
{
return input;
}
Class Declarations
public:/private:/protected:written at class indent level (not indented further)- Always explicitly write
private:, even though it is the default virtualonly on the base class;overrideon derived classes — never both together- All classes with virtual methods must have a virtual destructor
- Use
= defaultinstead of empty{}constructor/destructor bodies - Use member initializer lists; initializer order must match declaration order
class MyClass : public BaseClass
{
private:
int memberVariable = 0;
UString name;
public:
MyClass() = default;
~MyClass() override = default;
void doSomething() override;
int getValue() const;
};
Functions
- Functions should be named
camelBack - Use early return — prefer separate
if (cond) return;checks over deeply nested conditionals constaggressively: const member functions, const parameters, const return types, const local variables- Range-for loops:
for (auto &element : container)/for (const auto &element : container)- Exception: When removing elements during iteration, use an iterator-based loop instead of range-for
- Prefer
emplace()overinsert()in STL containers
General Code
- Use anonymous namespaces instead of
staticfor file-local functions and classes
namespace
{
void helperFunction()
{
// file-local helper
}
} // anonymous namespace
- All project code lives in
namespace OpenApoc {}. Namespace content is not indented. Closing brace gets a comment:
namespace OpenApoc
{
class MyClass
{
// ...
};
} // namespace OpenApoc
- Use
static_assert()to verify assumptions about alignment, packing (e.g., when reading structs from original game files), or template restrictions - Avoid Yoda conditionals — write
var == 1, not1 == var
Comments
- Prefer
//for single-line comments - Multi-line
/* */comments: align the*on each intermediate line - Use
//TODO:and//FIXME:markers to flag work-in-progress or known issues - Do not comment for the sake of it — if the code can be made clearer, prefer that over adding a comment
Logging
Uses fmt-style format strings with positional {0}, {1} placeholders (not printf-style):
#include "framework/logger.h"
LogInfo("Loaded mod \"{0}\"", modName);
LogWarning("Value {0} exceeds limit {1}", value, limit);
LogError("Failed to load file \"{0}\"", path);
LogInfo— general informationLogWarning— recoverable errorsLogError— fatal errors
Strings
Use UString (from library/strings.h) for all strings. All char*/std::string values are assumed UTF-8.
String formatting uses the fmt library:
#include "library/strings_format.h"
UString result = OpenApoc::format("Player has {0} credits and {1} agents", credits, agentCount);
Translation:
UString translated = tr("English text to translate");
Includes and Headers
- Use
#pragma once(no traditional include guards) - Include order: Local headers first, then system headers. Each block alphabetically sorted.
- Local headers use paths relative to the OpenApoc root:
"framework/logger.h", not"../logger.h"or"logger.h" - Prefer forward declarations over
#includein headers where possible- Forward declarations are not possible for templates, non-class types (e.g.,
enum class), and superclasses. When in doubt, try building without the include and see what fails.
- Forward declarations are not possible for templates, non-class types (e.g.,
#pragma once
#include "library/sp.h"
#include "library/strings.h"
#include <vector>
namespace OpenApoc
{
class ForwardDeclaredType;
class MyClass
{
private:
int member = 0;
public:
void publicFunction();
};
} // namespace OpenApoc
Key Review Expectations
These patterns are consistently enforced during code review:
- const correctness — if something can be
const, it must beconst - Readable conditionals — avoid embedded comments in complex conditionals; prefer early-exit checks
- One logical change per PR — keep changes focused for clean history and bisectability
- Use
auto— when the type is already visible on the RHS - Prefer exclusive ownership — use
up<>oversp<>when shared ownership is not required
See Also
- OpenApoc — Main OpenApoc page
- Compiling (OpenApoc) — Build instructions for developers
- CODE_STYLE.md — Full coding style specification in the repository
- OpenApoc Wiki: Coding Style — Extended coding style documentation
- OpenApoc on GitHub — Source code repository
- OpenApoc Discord — Community discussion and support