Jump to content

Coding Style (OpenApoc): Difference between revisions

From UFOpaedia
SupSuper (talk | contribs)
No edit summary
Deldonut1 (talk | contribs)
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 c++ code that forms the core of OpenApoc.
{{DISPLAYTITLE:Coding Style (OpenApoc)}}
This document specifies the guidelines for writing and formatting the C++ code that forms the core of '''[[OpenApoc]]'''.


Globally, we use 'standard' c++11. This requires reasonably modern compilers (gcc 4.7+, MSVC 2013+ , clang 3.6+ have been tested). You should avoid using compiler-specific stuff where possible. Exceptions to this exist, but should be wrapped in a #ifdef:
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>.
<pre>
 
#ifdef _MSC_VER
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.
MSVCIsCrazySometimes
#else
// Everything else we support (gcc + clang) are pretty much extension-compatible
GCCIsntMuchBetter
#endif
</pre>


C++11 features are heavily encouraged - patterns from 'older' c++ versions that have been superceded should be avoided.
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.


The formatting sections of this document are enforced by the [http://llvm.org/releases/4.0.0/tools/clang/docs/ClangFormat.html clang-format tool]. Currently the '4.0' version of <code>clang-format</code> is to be used. The configuration file <code>.clang-format</code> in the root of the OpenApoc source repository should match the formatting guidelines specified below.
It is highly recommended to run <code>clang-format</code> on all modified files before committing:


With this, it is highly recommended to run <code>clang-format</code> on all modified files before check-in. This can be run on source files with the following command:
<pre>clang-format -i path/to/file.cpp path/to/file.h</pre>


<pre>
When run from the root of the OpenApoc source repository, it automatically uses the supplied <code>.clang-format</code> configuration file.
clang-format path/to/file.cpp path/to/file.h
</pre>


When ran from the root of the OpenApoc source repository it should automatically use the supplied <code>.clang-format</code> configuration file. The tool also supports modifying the supplied source files to match the configured format when passed the <code>-i</code> flag:
When using the CMake build system, there are dedicated targets:


<pre>
<pre>
clang-format -i path/to/file.cpp path/to/file.h
# Format all source files (from build directory)
</pre>
cmake --build build -t format-sources
 
When using the <code>CMake</code> build system (as used on Unix-based platforms), there is a <code>format-sources</code> make target that will run <code>clang-format -i</code> on all source files within the OpenApoc repositry tree. This provides a single command that can be run before commiting:


<pre>
# Run static analysis (from build directory)
make format-sources
cmake --build build -t tidy
</pre>
</pre>


In the future, it may be possible to use <code>clang-tidy</code>, also from the clang project, to enforce more of the non-formatting sections of this style guideline. One example of this is an identifier name checking tool.
== Indent ==


* Tabs for indenting, spaces for alignment, indenting by 1 tab for each new scope


== 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 cols (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
* Avoid going over '''100 columns''' (at tab width of 4 spaces)
** Don't break strings up to fit this, it looks ugly and makes things even harder to read.
** 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 reallyLongFunctionNameIMeanThisIsReallyBadlyNamedWhateverIDontCareTheyPayMeAnyway(int parameterOne,
void reallyLongFunctionNameIMeanThisIsReallyBadlyNamed(int parameterOne,
int paramTwo, char theThirdOne)
int paramTwo, char theThirdOne)
{
{
if (parameterOne == yetAnotherReallyLongLineHereComeSomeWordsBlaBlaBlaAreYouStillReadingThisComeOnDont
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/when/else/for, space after :/; in for
 
* 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 'sizeof'), but space after flow control keywords, space after comma for multiple args
 
* 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** align to right (to variable) not type
 
* References and pointers: <code>&</code> and <code>*</code> align right (to the variable), not to the type
 
<pre>
<pre>
float**pointerToFloat;
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 _always_ surrounded by {} braces
* New scope is '''always''' surrounded by <code>{}</code> braces, even for single-statement blocks
* New scope has a '{' on the /next/ line at the indent of the old scope(not the new scope)
* Opening brace <code>{</code> goes on the '''next''' line at the indent of the enclosing scope ('''Allman style''')
* closing scope '}' same indent at opening '{', again on a new line
* Closing brace <code>}</code> at the same indent as the opening <code>{</code>
* New scope caused by:
* 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>
** new conditional block (if/else/when/for)
 
<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>
** 'switch'
** 'case' also indents a new scope. {} are optional, based on if new stack variables are needed to handle the case.
*** Note switch should always have a default case unless over an enum class (where they may not if (and only if) every value is handled)
*** All 'case' sections should have a 'break'
<pre>
<pre>
switch (a)
switch (a)
Line 124: Line 129:
case B:
case B:
{
{
auto var = somethingElse();
auto newVariable = getSomething();
doSomethingElse(var);
useIt(newVariable);
break;
break;
}
}
default:
default:
doDefaultCase();
break;
break;
}
}
</pre>
</pre>
** Class/enum/struct declarations
 
*** note: public/private/protected are an exception to this, being aligned to 'class' keyword, not 'within' it's scope
* '''Exception''': Trivial functions (a single statement fitting within 100 columns) may use the definition and body on one line:
 
<pre>
<pre>
class MyClass
int getX() const { return x; }
{
private:
int privateVariable;
public:
void publicFunction();
};
</pre>
* Exception to this is 'trivial' functions that have the definition & contents all on line line
** 'Trivial' is defined by a single statement that fits within the 100-column limit
<pre>
int Class::function() {return 0;}
</pre>
* New scope is not caused by:
** namespace (which should also have a comment stating the namespace name at the closing bracket)
<pre>
namespace OpenApoc
{
Class MyClass
{
private:
int x;
};
}; // namespace OpenApoc
</pre>
** labels
* Labels and #preprocessor directives /always/ on column 0 (start of line) no matter the scope
<pre>
#if defined(LINUX)
x = linuxFunction();
#else
x = otherFunction();
#endif
if (x)
{
goto error;
}
error:
return 0;
</pre>
</pre>


== Case ==
This is enforced by <code>clang-format</code> (<code>AllowShortFunctionsOnASingleLine: All</code>).
* Namespaces should be CamelCase
 
<pre>
== Naming ==
namespace OpenApoc
 
</pre>
{| class="wikitable"
* Classes and enums should be CamelCase
! Style !! Used For
<pre>
|-
class MyClass
| <code>CamelCase</code> || Classes, enums, <code>enum class</code> members, namespaces, template parameters
</pre>
|-
* 'enum class' members should be CamelCase
| <code>camelBack</code> || Methods, member variables, function parameters, local variables
<pre>
|-
enum class MyEnum
| <code>SHOUTY_CAPS</code> || Constants, macros
{
|-
ValueOne,
| <code>lower_case</code> || Labels
ValueTwo,
|}
};
 
</pre>
Short variable names (<code>i</code>, <code>x</code>, <code>y</code>) are fine when the meaning is obvious from context.
* class methods and member variables (public/private/protected) should be camelBack
<pre>
class MyClass
{
public:
void someFunction();
int someVariable;
};
</pre>
* Function parameters (public/private/protected) should be camelBack
<pre>
int function(int parameterOne, char secondOne)
</pre>
* Variables should be camelBack
** Don't be afraid to use 'short' variable names if it's obvious
<pre>
void function()
{
int localVariable = 0;
int x = 1;
int y = 5;
for (int i = 0; i < 5; i++)
</pre>
* class/global constants should be SHOUTY_CAPS, along with _all_ macros
<pre>
#define OPENAPOC_VERSION "1.0"
</pre>
* Labels should be lower_case:
<pre>
exit_loop:
goto exit_loop;
</pre>
* All members should be initialised in all constructors if they don't have a default constructor. You can use member initialisation in the class definition if this is clearer
<pre>
class MyClass
{
int variableMember = 0;
};
</pre>


== Types ==
== Types ==
* Avoid typedef - use 'struct' keyword where necessary in c-like code
<pre>
struct MyStruct
{
int x;
};


void myStructUser(struct MyStruct s)
* Use <code>auto</code> liberally, especially when the type is obvious from the right-hand side. Use <code>auto &</code> to avoid copies.
</pre>
* Use <code>enum class</code> over plain <code>enum</code>
* up<> sp<> wp<> aliases are defined for std::unique_ptr<>, std::shared_ptr<>, std::weak_ptr<> in "library/sp.h". Use them instead of the verbose versions.
* Use <code>struct</code> only for data-only types; structs must '''never''' use access modifiers
* Use anonymous namespaces for 'file-local' stuff (instead of static, as you can wrap classes in it too)
* No C-style casts &mdash; use <code>static_cast&lt;&gt;</code>, <code>dynamic_cast&lt;&gt;</code>, <code>reinterpret_cast&lt;&gt;</code>
* Prefer <code>{}</code> brace initialization
* Prefer <code>std::numeric_limits&lt;&gt;</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&lt;T&gt;</code> || <code>std::shared_ptr&lt;T&gt;</code>
|-
| <code>up&lt;T&gt;</code> || <code>std::unique_ptr&lt;T&gt;</code>
|-
| <code>wp&lt;T&gt;</code> || <code>std::weak_ptr&lt;T&gt;</code>
|-
| <code>mksp&lt;T&gt;(args...)</code> || <code>std::make_shared&lt;T&gt;(args...)</code>
|-
| <code>mkup&lt;T&gt;(args...)</code> || <code>std::make_unique&lt;T&gt;(args...)</code>
|}
 
* '''No naked <code>new</code>''' &mdash; always wrap in smart pointers immediately
* Prefer <code>up&lt;&gt;</code> (exclusive ownership) over <code>sp&lt;&gt;</code> unless shared ownership is genuinely needed
* Use <code>std::move()</code> to transfer <code>up&lt;&gt;</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>&amp;reference</code> member. Otherwise use <code>up&lt;&gt;</code> or <code>sp&lt;&gt;</code> depending on whether ownership is exclusive or shared.
 
== Templates ==
 
* Template type parameters should use <code>CamelCase</code>
* Place <code>template&lt;...&gt;</code> on the line above the function/class declaration
 
<pre>
<pre>
namespace
template <typename ValueType>
ValueType doSomething(ValueType input)
{
{
void localFunction()
return input;
}; // anonymous namespace
}
</pre>
</pre>
* We provide a "UString" class. This should be used for _all_ strings, as it provides platform-local non-ascii charset handling
** All "char*"/std::string params are assumed to be in utf8 format.


== Class Declarations ==


== Templates ==
* <code>public:</code>/<code>private:</code>/<code>protected:</code> written at class indent level (not indented further)
* If templates help, go ahead, don't avoid them
* Always explicitly write <code>private:</code>, even though it is the default
* prefer 'typename' to 'class'
* <code>virtual</code> only on the base class; <code>override</code> on derived classes &mdash; never both together
* template types should be CamelCase
* All classes with virtual methods must have a virtual destructor
<pre>
* Use <code>= default</code> instead of empty <code>{}</code> constructor/destructor bodies
template <typename LocalType> function(LocalType param)
* Use member initializer lists; initializer order must match declaration order
</pre>
* 'short' typenames are OK if it's obvious what's going on
<pre>
template <typename T> Class<T>::function()
</pre>


== Class declarations ==
* member functions camelCase()
* 'public:' 'private:' 'protected:' are indented to the 'class' keyword, everything within them indented to class scope.
** Always use 'private', even if that's the default
<pre>
<pre>
class MyClass
class MyClass : public BaseClass
{
{
private:
private:
int localVariable;
int memberVariable = 0;
UString name;
public:
public:
void publicFunction();
MyClass() = default;
~MyClass() override = default;
 
void doSomething() override;
int getValue() const;
};
};
</pre>
</pre>
* 'virtual' keyword only used for base class, 'override' used for derived
 
** All classes with a virtual (or overrided) function _must_ specify a virtual destructor
== Functions ==
* Inheritence should be on the same line as the 'class' keyword (until you get to the column limit and break)
 
* Functions should be named <code>camelBack</code>
* Use '''early return''' &mdash; 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>
class BaseClass
namespace
{
{
public:
void helperFunction()
virtual ~BaseClass();
virtual void someFunction();
};
 
class SubClass : public BaseClass
{
{
public:
// file-local helper
void someFunction() override;
}
};
} // anonymous namespace
</pre>
</pre>
** Never use both 'virtual' and 'override
 
* Don't define an empty {} body in the header for constructors/destructors etc. - use '= default' instead
* All project code lives in <code>namespace OpenApoc {}</code>. Namespace content is '''not''' indented. Closing brace gets a comment:
 
<pre>
<pre>
class MyBaseClass
namespace OpenApoc
{
{
public:
 
virtual ~MyBaseClass() = default;
};
</pre>
* define pure virtual "virtual void func() = 0;" for 'interface' classes
<pre>
class MyInterface
{
public:
virtual void functionBaseClassesMustOverride() = 0;
};
</pre>
** No need for 'pure' interface classes, they can have code that all subclasses will use!
* For trivial initial values prefer initialisers in the class declaration (It's easier to see what's set and cleans up constructor definitions)
<pre>
class MyClass
class MyClass
{
{
public:
// ...
Type initialisedMember = 0;
};
};
} // namespace OpenApoc
</pre>
</pre>
* In constructors prefer initialisation of members with  initialiser list over assignment
 
** Good:
* Use <code>static_assert()</code> to verify assumptions about alignment, packing (e.g., when reading structs from original game files), or template restrictions
<pre>  
* Avoid Yoda conditionals &mdash; write <code>var == 1</code>, not <code>1 == var</code>
MyClass::MyClass(Type value) : member(value)
 
{
== Comments ==
doWhatever();
 
}
* Prefer <code>//</code> for single-line comments
</pre>
* Multi-line <code>/* */</code> comments: align the <code>*</code> on each intermediate line
** Bad:
* 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 &mdash; 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>
MyClass::MyClass(Type value)
#include "framework/logger.h"
{
 
member = value;
LogInfo("Loaded mod \"{0}\"", modName);
doWhatever();
LogWarning("Value {0} exceeds limit {1}", value, limit);
}
LogError("Failed to load file \"{0}\"", path);
</pre>
</pre>
* Initialisers should be in order of declaration in the class
 
** For example with the class:
* <code>LogInfo</code> &mdash; general information
* <code>LogWarning</code> &mdash; recoverable errors
* <code>LogError</code> &mdash; 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>
class MyClass
#include "library/strings_format.h"
{
public:
Type memberA;
Type memberB;


MyClass(Type valueA, Type valueB);
UString result = OpenApoc::format("Player has {0} credits and {1} agents", credits, agentCount);
};
</pre>
</pre>
** Good:
 
Translation:
 
<pre>
<pre>
MyClass::MyClass(Type valueA, Type valueB) : memberA(valueA), memberB(valueB) {}
UString translated = tr("English text to translate");
</pre>
</pre>
** Bad:
 
== 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>
MyClass::MyClass(Type valueA, Type valueB) : memberB(valueB), memberA(valueA) {}
#pragma once
</pre>
 
* Use 'struct' for 'data-only' types
#include "library/sp.h"
** Structs should _never_ have public/private/protected declarations, if there's anything non-public you shouldn't use a struct.
#include "library/strings.h"
** Likely only going to be used within data reading/writing to files
 
*** Because of this you're probably going to need to use 'sized' typed (see <cstdint> header)
#include <vector>
<pre>
 
struct DataFileSection
namespace OpenApoc
{
{
uint32_t x;
uint32_t y;
uint32_t z;
};
</pre>
** If using a struct to read in data, use a static_assert to ensure correct sizing:
<pre>
static_assert(sizeof(DataFileSection) == 12, "DataFileSection not expected 12 bytes");
</pre>
* If ownership of a member is tied to the class, don't use a pointer and new/delete in constructor/destructor. Just use the type and initialise it correctly in the init list before the constructor.
** If the above is not possible (e.g. complex init requirements, 'may be invalid and null' use a up<>
* If we /know/ a member reference owned by another object will be live for the duration of the class, use a &reference member
* Otherwise use a sp<>/up<> depending if it should take a reference and if having a 'null' object makes sense.


== Functions ==
class ForwardDeclaredType;
* Const functions where possible (IE not modifying any members)
 
<pre>
class MyClass
class MyClass
{
{
private:
int member = 0;
public:
public:
int dataAccessor() const;
void publicFunction();
};
};
</pre>
* Const params where logically not to be modified
<pre>
void function(const Type& readOnlyParam)
</pre>
* Const returns where the caller should never modify
<pre>
const Type& functionWhereYouCantModifyMyReturnThanks()
</pre>


== General code ==
} // namespace OpenApoc
* Where possible use auto
<pre>
auto varableName = function();
</pre>
** Note where auto& may be better to avoid a copy
* sp<> up<> wp<> smart pointers
** make_shared() instead of new
<pre>
auto var = std::make_shared<Type>(args);
</pre>
</pre>
** make_unique() would be nice, but a c++14 feature (may restrict compiler compatibility?), so you have to wrap
*** Use std::move to transfer up<> ownership
<pre>
up<Type> var{new Type{args}};
functionThatTakesOwnershipOfParam(std::move(var));
</pre>
* Never use a 'naked' new - they should always be packaged immediately in a smart pointer
* Use emplace() in STL containers where possible over insert()
** Unless you explicitly want to copy an object in
* Use foreach loops where possible ( "for (auto &element : container)")
<pre>
for (auto &element : container)
{
whatever(element);
}
</pre>
** Exception may be a 'safe' iterator when possibly removing elements during loop, then use iterator and keep copy locally
* Where possible use 'enum class'
* Naming variables - don't be afraid of using 'short' names ('i') if it's use is obvious
* While 'descriptive' names are nice, shorter code is also nice. Don't repeat context
** 'x' is fine is we already know we're doing something in coordinate space, no need to name it theXCoordinateOfTheCityMapInTiles
* Reading code is important - try to make it flow
** avoid 'yoda conditionals' (1 == var) don't help, modern compilers catch a =/== typo easily
** if post increment (x++) flows better use that, don't try to 'optimise' away the copy - the compiler will do that for you
* The compiler is more clever than you could ever possibly hope to be. Write things to be clear and obvious. Only /after/ it's proven to be a problem to you even look at optimisation (then _always have numbers_)
* don't use 'c' casts ("(int)x") - that does different things on if the object type has a defined conversion or not - use static_cast<>/reinterpret_case<> instead
* prefer {} constructor calls where possible
<pre>
MyClass classInstance{argumentOne, argTwo};
</pre>
** Requires you to avoid implicit conversions - this is good!
* STL initialiser lists are good
<pre>
std::vector<int> someInts = {1, 2, 3};
</pre>
* static_assert() any assumptions WRT alignment/packing (when reading structs from files, for example) - or any template restrictions (e.g. if this is only valid on unsigned types, check it!)
* <limits> is preferred to 'c' MAX_INT/whatever
<pre>
auto maxInt = std::numeric_limits<int>::max();
</pre>
* RAII where possible
* Early-return is cool, go ahead
<pre>
if (dontHaveToDoAnything)
{
return;
}
doLotsOfBoringStuff();
</pre>
* goto: is useful in some specific cases (e.g. breaking out of nested loops) - but only use it where another keyword won't do what you want
** Note limitations WRT goto: over stack initialisers - IE you can't do it :)
<pre>


for (auto &x : containerX)
== Key Review Expectations ==
{
for (auto &y : x.containerY)
{
if (weShouldStopAt(y))
{
goto end;
}
}
}
end:
return;
</pre>


== Logging ==
These patterns are consistently enforced during code review:
* LogInfo/LogWarning/LogError take 'printf-style' format string. Make sure everything is the correct type, %d for ints, %u for unsigned, %s for "utf8" const char* "strings" or UString objects directly
* LogInfo is cheap - use it everywhere interesting
* LogWarning should be something that has gone wrong, but recoverable.
* LogError is for fatal errors.


Comments:
# '''const correctness''' &mdash; if something can be <code>const</code>, it must be <code>const</code>
* either // or /***/ is fine - prefer // for single line
# '''Readable conditionals''' &mdash; avoid embedded comments in complex conditionals; prefer early-exit checks
* If doing multi line /\*-style comments have an aligned '\*' with at the beginning of each subseqent line:
# '''One logical change per PR''' &mdash; keep changes focused for clean history and bisectability
<pre>
# '''Use <code>auto</code>''' &mdash; when the type is already visible on the RHS
/* first line
# '''Prefer exclusive ownership''' &mdash; use <code>up&lt;&gt;</code> over <code>sp&lt;&gt;</code> when shared ownership is not required
** second line
** last line**/
</pre>
* Don't comment for the sake of it
** Try to make the code clearer first if a comment is 'required' to make something obvious
** Function/variable names are useful here - if reading it aloud describes what your comment was going to say that's perfect
** //TODO: //FIXME: when leaving known breakage
 
== Includes ==
* "local.h" files first
* then <system.h> includes
* Within each of the 2 blocks try to keep them alphabetically sorted (some exceptions may happen, if there's a system dependency not managed by the system header itself)
* local headers always relative to the root of OpenApoc - even if in the same directory
** e.g. "framework/event.h" not "../event.h" or "event.h"


== Headers ==
== See Also ==
* prefer "#pragma once" to "#ifndef __HEADER_NAME" include guards
* Headers should avoid #include "dependency.h" where possible
** do forward declaration of classes instead where possible
<pre>
class ForwardDeclaredType;


void someFunction(ForwardDeclaredType &param);
* [[OpenApoc]] &mdash; Main OpenApoc page
</pre>
* [[Compiling (OpenApoc)]] &mdash; Build instructions for developers
** 'not possible' includes templates, non-class types, superclasses, try building it without and see what fails
* [https://github.com/OpenApoc/OpenApoc/blob/master/CODE_STYLE.md CODE_STYLE.md] &mdash; Full coding style specification in the repository
* [https://github.com/ayrtondenner/OpenApoc-wiki/blob/master/development/coding-style.md OpenApoc Wiki: Coding Style] &mdash; Extended coding style documentation
* [https://github.com/OpenApoc/OpenApoc OpenApoc on GitHub] &mdash; Source code repository
* [https://discord.gg/f8Rayre OpenApoc Discord] &mdash; 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 :/; in for
	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();
	}
  • switch always has a default case unless switching over an enum class where every value is handled
  • All case sections should have a break
  • case braces {} 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 auto liberally, especially when the type is obvious from the right-hand side. Use auto & to avoid copies.
  • Use enum class over plain enum
  • Use struct only 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-style MAX_INT / INT_MAX macros

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) over sp<> unless shared ownership is genuinely needed
  • Use std::move() to transfer up<> 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 &reference member. Otherwise use up<> or sp<> 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
  • virtual only on the base class; override on derived classes — never both together
  • All classes with virtual methods must have a virtual destructor
  • Use = default instead 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
  • const aggressively: 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() over insert() in STL containers

General Code

  • Use anonymous namespaces instead of static for 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, not 1 == 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 information
  • LogWarning — recoverable errors
  • LogError — 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 #include in 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.
#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:

  1. const correctness — if something can be const, it must be const
  2. Readable conditionals — avoid embedded comments in complex conditionals; prefer early-exit checks
  3. One logical change per PR — keep changes focused for clean history and bisectability
  4. Use auto — when the type is already visible on the RHS
  5. Prefer exclusive ownership — use up<> over sp<> when shared ownership is not required

See Also