CODINGSTYLE.md
Why a set coding style is important
This project is an open source project. To get open source working as intended, many people should be able to comprehend the code. This implies that there is a well documented and uniform flow of code. That does not necessarily mean that a contributor should not write optimised yet cryptic code - one should always care about performance. However, other developers need to understand the code which means complicated parts of code must have good documentation.
Why we require that EVERYTHING is documented
What is simple to some might appear very complicated to others. Documentation helps these others. Anyone should be able to improve the code. But the main reason is, that when patch contributors want their patches added to the common codebase, the code needs to be reviewed by one or more developers. Documentation makes reviewing much easier.
void ThisIsAFunction()
{
}
int number = 10;
Vehicle *u_first_wagon = v->next;
int value = v->value;
uint32 _global_variable = 3750;
static const char * const _global_array[] = {
"first string",
"second string",
"another string",
"last string followed by comma",
};
protected:
char protected_array[10];
for (int i = 0;;);
bool is_maglev_train = true;
if (!is_maglev_train) DoSomething();
/* Unoptimized code:
* foo is not touched inside the loop!
*/
for (uint8 i = 0; i < 100000; i++) {
/* Do something */
if (value_to_check == (foo * 4) % 5 + 6) DoSomething();
/* Do something else */
}
/* Better:
* The used value of foo is calculated outside the loop.
*/
const uint32 bar = (foo * 4) % 5 + 6;
for (uint8 i = 0; i < 100000; i++) {
/* Do something */
if (value_to_check == bar) DoSomething();
/* Do something else */
}
enum DiagDirection {
DIAGDIR_BEGIN = 0,
DIAGDIR_NE = 0,
DIAGDIR_SE = 1,
DIAGDIR_SW = 2,
DIAGDIR_NW = 3,
DIAGDIR_END,
INVALID_DIAGDIR = 0xFF,
BROKEN_DIAGDIR = 0xFE,
};
enum {
DEPOT_SERVICE = (1 << 0),
DEPOT_MASS_SEND = (1 << 1),
DEPOT_DONT_CANCEL = (1 << 2),
DEPOT_LOCATE_HANGAR = (1 << 3),
};
DiagDirection enterdir = DIAGDIR_NE;
static const int MAXIMUM_STATIONS = 42;
/** Enum referring to the widgets of the build rail depot window */
enum BuildRailDepotWidgets {
BRDW_CLOSEBOX = 0,
BRDW_CAPTION,
BRDW_BACKGROUND,
BRDW_DEPOT_NE,
BRDW_DEPOT_SE,
BRDW_DEPOT_SW,
BRDW_DEPOT_NW,
};
/* ... */
/** Widget definition of the build rail depot window */
static const Widget _build_depot_widgets[] = {
{ WWT_CLOSEBOX, RESIZE_NONE, 7, 0, 10, 0, 13, STR_00C5, STR_..}, // BRDW_CLOSEBOX
{ WWT_CAPTION, RESIZE_NONE, 7, 11, 139, 0, 13, STR_..., STR_...}, // BRDW_CAPTION
{ WWT_PANEL, RESIZE_NONE, 7, 0, 139, 14, 121, 0x0, STR_NULL}, // BRDW_BACKGROUND
{ WWT_PANEL, RESIZE_NONE, 14, 71, 136, 17, 66, 0x0, STR_..}, // BRDW_DEPOT_NE
{ WWT_PANEL, RESIZE_NONE, 14, 71, 136, 69, 118, 0x0, STR_..}, // BRDW_DEPOT_SE
{ WWT_PANEL, RESIZE_NONE, 14, 3, 68, 69, 118, 0x0, STR_..}, // BRDW_DEPOT_SW
{ WWT_PANEL, RESIZE_NONE, 14, 3, 68, 17, 66, 0x0, STR_..}, // BRDW_DEPOT_NW
{ WIDGETS_END},
};
enum SomeEnumeration {
SE_BEGIN = 0, ///< Begin of the enumeration, used for iterations
SE_FOO = 0, ///< Used for xy
SE_BAR, ///< Another value of the enumeration
SE_SUB, ///< Special case for z
SE_END, ///< End of the enumeration, used for iterations
};
[[fallthrough]] attribute.for (;;) {if (a == b) {
Foo();
} else {
Bar();
}
if (very_large_checks &&
spread_over_two_lines) {
Foo();
}
if (a == b) Foo();
switch (a) {
case 0: DoSomethingShort(); break;
case 1:
DoSomething();
[[fallthrough]];
case 2:
DoMore();
b = a;
break;
case 3: {
int r = 2;
DoEvenMore(a);
[[fallthrough]];
}
case 4: {
int q = 345;
DoIt();
break;
}
default:
NOT_REACHED();
}
for (int i = 0; i < 10; i++) {
Foo();
Bar();
}
class ThisIsAClass {
public:
typedef Titem_ *ItemPtr;
private:
static const int MAX_SIZE = 500;
int size;
ItemPtr *items;
public:
explicit ThisIsAClass();
~ThisIsAClass();
int GetSize() { return this->size; }
virtual void Method();
};
/*virtual*/ void Class::Method()
{
this->size *= 2;
}
Templates are a very powerful C++ tool, but they can easily confuse beginners. Thus:
template <typename T, typename Tsomething, int N, uint8_t Tnumber_of_something>
int Func();
When adding code or comments to an existing formatted section, follow the existing style if possible without editing the preexisting lines.
If your addition cannot be aligned with existing code, do not align the comments with anything and use only a single space between the code and the comment.
Good:
enum Vehicle {
BUS, ///< Take the bus.
+ CAR, ///< Drive your car.
BIKE, ///< Ride your bike
+ TRAIN, ///< Catch the train.
}
"Car" is shorter than Bike which allows you to easily align the new comment. "Train" is longer. It is NOT desirable to change the vertical comment alignment of this enum.
Bad:
enum Vehicle {
- BUS, ///< Take the bus.
- BIKE, ///< Ride your bike
+ BUS, ///< Take the bus.
+ CAR, ///< Drive your car.
+ BIKE, ///< Ride your bike
+ TRAIN, ///< Catch the train.
}
OpenTTD used to vertically-align inline Doxygen comments as shown above. OpenTTD has since stopped strictly following this rule to keep diffs smaller and reduce pollution to the git blame history for non-functional changes.
We use Doxygen to automatically generate documentation from the source code. It scans the source files for recognizable comments.
/**
///<
Use /** for multi-line comment blocks. Use ///< for single line comments for variables. Use //!< for single-line comments in the NoAI .hpp files. For comments blocks inside a function always use /* / or //. Use // only if the comment is on the same line as an instruction, otherwise use / */.
/**
* @file
* This is the brief description.
* This is the detailed description here and on the following lines.
*/
Warning
If a file lacks a file comment block then NO entities in that file will be documented by Doxygen!
/**
* A brief explanation of what the function does and/or what purpose it serves.
* Then follows a more detailed explanation of the function that can span multiple lines.
*
* @param foo Explanation of the foo parameter
* @param bar Explanation of the bar parameter
* @return The sum of foo and bar (@return can be omitted if the return type is void)
*
* @see SomeOtherFunc()
* @see SOME_ENUM
*
* @bug Some bug description
* @bug Another bug description which continues in the next line
* and ends with the following blank line
*
* @todo Some to-do entry
*/
static int FooBar(int foo, int bar)
{
return foo + bar;
}
/**
* A short description of the struct.
* More detailed description of the its usage.
*
* @see [link to anything of interest]
*/
struct foo {
}
This table shows the commands you should use with OpenTTD. The full list is here.
| Command | Action | Example |
|---|---|---|
| @attention | Starts a paragraph where a message that needs attention may be entered. The paragraph will be indented. | @attention Whales crossing! |
| @brief | Starts a paragraph that serves as a brief description. For classes and files the brief description will be used in lists and at the start of the documentation page. For class and file members, the brief description will be placed at the declaration of the member and prepended to the detailed description. A brief description may span several lines (although it is advised to keep it brief!). | @brief This is the brief description. |
| @bug | Starts a paragraph where one or more bugs may be reported. The paragraph will be indented. Multiple adjacent @bug commands will be joined into a single paragraph. Each bug description will start on a new line. Alternatively, one @bug command may mention several bugs. | @bug Memory leak in here? |
| @note | Starts a paragraph where a note can be entered. The paragraph will be indented. | @note Might be slow |
| @todo | Starts a paragraph where a TODO item is described. The description will also add an item to a separate TODO list. The two instances of the description will be cross-referenced. Each item in the TODO list will be preceded by a header that indicates the origin of the item. | @todo Better error checking |
| @warning | Starts a paragraph where one or more warning messages may be entered. The paragraph will be indented. | @warning Not thread safe! |
| <small>Function related commands</small> | ||
| @return | Starts a return value description for a function. | @return a character pointer |
| @param | Starts a parameter description for a function parameter with name <parameter-name>. Followed by a description of the parameter. The existence of the parameter is checked and a warning is given if the documentation of this (or any other) parameter is missing or not present in the function declaration or definition. |
The @param command has an optional attribute specifying the direction of the attribute. Possible values are "in" and "out". | @param n The number of bytes to copy @param[out] dest The memory area to copy to. @param[in] src The memory area to copy from. | | @see | Starts a paragraph where one or more cross-references to classes, functions, methods, variables, files or URL may be specified. Two names joined by either :: or # are understood as referring to a class and one of its members. One of several overloaded methods or constructors may be selected by including a parenthesized list of argument types after the method name. Here you can find detailed information about this feature. | @see OtherFunc() | | @b | Displays the following word using a bold font. Equivalent to <b>word</b>. To put multiple words in bold use <b>multiple words</b>.| ...@b this and @b that... | | @c / @p | Displays the parameter <word> using a typewriter font. You can use this command to refer to member function parameters in the running text. To have multiple words in typewriter font use <tt>multiple words</tt>. | ... the @p x and @p y coordinates are used to... | | @arg / @li | This command has one argument that continues until the first blank line or until another @arg / @li is encountered. The command can be used to generate a simple, not nested list of arguments. Each argument should start with an @arg / @li command. | @arg @c AlignLeft left alignment. @arg @c AlignCenter center alignment. @arg @c AlignRight right alignment | | @n | Forces a new line. Equivalent to and inspired by the printf function. |@n |
Doxygen knows two different kinds of comments:
You can omit either one or put them into different places but there's only one brief and one detailed description allowed for the same entity.
Doxygen knows three modes for documenting an entity:
The latter is a little harder to maintain since the prototype of the entity it describes then is stored in several places (e.g. the .h file and the file with the descriptions). Also while it makes the code easier to read it also makes it easier to omit the important step of updating the description of an entity if it was changed - and we all know why that shouldn't happen ;)
Because of those reasons, we will only use the first two documentation schemes.
Doxygen supports both Qt and JavaDoc comment styles:
It also supports more comment styles but those two are the ones which are standardized. For OTTD we'll be using the JavaDoc style. One of the reasons is that it has a feature that the Qt style doesn't offer: JavaDoc style comment blocks will automatically start a brief description which ends at the first dot followed by a space or new line. Everything after that will also be part of the detailed description.
The general structure of a JavaDoc style comment is
/**
* This is the brief description. And this sentence contains some further explanations that will appear in the detailed description only.
*/
and the resulting descriptions of that block would be:
The distinction between the brief and detailed descriptions is made by the dot followed by a space/newline, so if you want to use that inside the brief description you need to escape the space/newline:
/**
* This is a brief description (e.g.\ using only a few words). Details go here.
*/
If you're doing a one-line comment, use:
int i; ///< This is the description.
Also in the comment block you can include so-called structural commands which tell Doxygen what follows. In general, their area of effect begins after the command word and ends when a blank line or some other command is encountered. Also, multiple occurrences of the same structural command within a comment block or the referring entity will be joined in the documentation output usually.
To achieve a coherent whole and to make changelog writing easier, here are some guidelines for commit messages. There is a check-script on the git server (also available for clients, see below) to make sure we all follow those rules.
The first line of a message must match:
<keyword>( #<issue>|<commit>(, (#<issue>|<commit>))*)?: ([<component>])? <details>
Keywords can either be player-facing, NewGRF / Script author-facing, or developer-facing.
For player-facing changes, we have these keywords:
docs/ folder etc.For NewGRF / Script author-facing changes, we use the same keywords as player-facing changes, followed by [NewGRF] / [Script] component.
This also means the commit is aimed (and worded) towards the NewGRF / Script authors, rather than players.
For developer-facing changes, we have these keywords:
If you commit a Fix for an issue, add the corresponding issue number in the form of #NNNNN.
In the case of Fixes, if you know the hash of the commit in which the bug was introduced (eg regression), please mention that hash (the first 7 characters) as well just after the keyword (or, if present, after the issue number).
Finding the trouble-causing commit is highly encouraged as it makes backporting / branching / releases that much easier.
Do not mention two keywords; if two apply, pick one that best represents the commit (for example, "Fix #123" is mostly always better than "Revert", even if both are true).
The <details> part starts with a capital and does not end with a dot.
Try to be descriptive to what the player will notice, not to what is actually being changed in the code.
See changelog.md for inspiration.
To further structure the changelog, you can add components. Example are:
Further explanations, more details, etc. don't go into the first line. Use a new line for those.
Complete examples:
Fix: [YAPF] Infinite loop in pathfinderFix #5926: [YAPF] Infinite loop in pathfinderCodefix 80dffae: Warning about unsigned unary minusFix #6673, 99bb3a9: Store the map variety setting in the savegameCodefix #5922: ClientSizeChanged is only called via WndProcGdi which already has the mutexCodechange #1264, #2037, #2038, #2110: Rewrite the autoreplace kernelTo find out if/where you have trailing whitespace, you can use the following (unix/bash) command:
grep -n -R --include "*.[ch]" '[ ]$' * | grep --invert-match ".diff" | grep --invert-match ".patch"
Automatically removing whitespace is also possible with the following shell script (Note that it only checks .c, .cpp, .h, .hpp and .mm files):
#!/bin/sh
IFS='
'
for i in Makefile `find . -name \*.c -o -name \*.cpp -o -name \*.h -o -name \*.hpp -o -name \*.mm`
do
(
echo '%s/[ ]\{1,\}$/'
echo w
echo q
) | ed $i 2> /dev/null > /dev/null
done
The client-side hooks perform various checks when you commit changes locally.
Get the hooks:
git clone https://github.com/OpenTTD/OpenTTD-git-hooks.git openttd_hooks
Install the hooks, assuming "openttd" is your work tree folder:
cd openttd/.git/hooks
ln -s -t . ../../../openttd_hooks/hooks/*