Upcoming C++ code style changes for 4.0+

[akien-mga]

At the Godot Sprint in Poznań, we discussed a few items about our code style guidelines and how to make things more consistent.

Below is a list of items that we agreed upon. We can use comments on this issue to further discuss; the list below should be updated to reflect consensus on any changes that should be made. Input is also welcome on what tooling we can use to apply style changes and check PRs for style adequacy.

Note that to avoid increasing the maintenance burden in the vulkan branch, code style changes should only be applied once 3.2 is released and the vulkan branch was merged in master.

Code formatting

No empty line after opening brace

Fixed with #38738. Enforced by clang-format.

This is a habit from @reduz that many of us inherited, but there's really no good reason to justify it. If we really need vertical spacing, we can use Allman style, but since most of us really don't like it, let's stick to the expected style.

Bad:

if (some_cond) {

	ERR_FAIL_COND(foo);
	some_statement();
}

Good:

if (some_cond) {
	ERR_FAIL_COND(foo);
	some_statement();
}

BTW, should also apply to brace-less situations (e.g. after case:).

Tools: Thankfully we can just rely on clang-format for this, as it has a KeepEmptyLinesAtTheStartOfBlocks: false option that we can use.

Enforce one line between function implementations

Fixed with #38738. Albeit manually, so contributors will have to pay attention to it to keep the style consistent.

Basically fixing #29936.

Bad:

void MeshTexture::set_mesh(const Ref<Mesh> &p_mesh) {
	mesh = p_mesh;
}
Ref<Mesh> MeshTexture::get_mesh() const {
	return mesh;
}

Good:

void MeshTexture::set_mesh(const Ref<Mesh> &p_mesh) {
	mesh = p_mesh;
}

Ref<Mesh> MeshTexture::get_mesh() const {
	return mesh;
}

Tools:* I couldn't find any. Help welcome to find something, enforcing this manually would be tedious.

Enforce using braces on single-statement if / else blocks

Fixed with #38738. Albeit with clang-tidy which doesn't run automatically, so "bad" style could still be introduced by PRs. Reviewers should be on the lookout.

This has two parts:

• Prevent putting the if's statement on a single line, e.g. if (cond) do_some_stuff();. We can already enforce that via clang-format, see second commit in (unmerged) clang-format config changes to discuss #29848.
• Always use braces for if and else blocks, even if they only contain a single statement.

Bad:

if (cond) do_some_stuff();
else
	return;

Good:

if (cond) {
	do_some_stuff();
} else {
	return;
}

Tools:* For the first point we can just flip a switch in clang-format (AllowShortIfStatementsOnASingleLine: false). For the second and most important one, I don't think clang-format can handle this for now. IIRC it might be doable with clang-tidy?

Disallow short case labels on a single line

Fixed by #38621. Enforced by clang-format.

This one is a relatively simple change, just toggling an existing clang-format option. See first commit in #29848.
These situations don't arise often, but enforcing a consistent style is good.

Bad:

	case Variant::NIL: return "null";
	case Variant::BOOL: return p_var.operator bool() ? "true" : "false";
	case Variant::INT: return itos(p_var);
	case Variant::REAL: return rtos(p_var);
	case Variant::ARRAY: {
		String s = "[";
		s += end_statement;
		...
		return s;
	}

Good:

	case Variant::NIL:
		return "null";
	case Variant::BOOL:
		return p_var.operator bool() ? "true" : "false";
	case Variant::INT:
		return itos(p_var);
	case Variant::REAL:
		return rtos(p_var);
	case Variant::ARRAY: {
		String s = "[";
		s += end_statement;
		...
		return s;
	}

Tools:* We can also rely on clang-format's AllowShortCaseLabelsOnASingleLine: false option for this.

Switch/case use of brackets

As described in #16521, we have inconsistent use of brackets in switch/case blocks.
Note sure what style we should aim for, especially since there are some semantic differences.

Other things

Feel free to refer to our .clang-format file to see what options we currently don't set (i.e. we use their default value) and https://clang.llvm.org/docs/ClangFormatStyleOptions.html for description on options.

Please comment if you'd like to see other options enabled, or if you know of other tools that can help us enforce a more consistent style guide (both as a manual step to fix the codebase - e.g. clang-tidy - or as a check on PRs/pre-commit hook).

Coding style

Beyond formatting, there are also some conventions in how we code in Godot's C++ codebase. This section doesn't aim at covering actual features (like C++14 usage, this will be for another issue), but more C++ style choices which don't related to pure formatting.

Enforce p_ and r_ prefixes for method parameters

We have an implicit convention in the codebase which is more or less followed by core contributors but often missed by new contributors. We should make it explicit in the documentation, and find out how to enforce it.

This convention is that method parameters should start with a p_ prefix (like "parameter"), unless they are modified by the function itself, in which case they start with a r_ prefix (like "return" - BTW if anyone knows the proper technical jargon to refer to this kind of parameters, that would be useful for the docs). This convention makes it easy to differentiate function parameters from local variables and members, and to spot parameters which might change state and be further used in their calling method.

Bad:

static void _fill_array(Node *p_node, Array &array, int p_level) {  // should be r_array

void Camera2D::set_limit_smoothing_enabled(bool enable) {  // should be p_enable

Good:

void Node::get_argument_options(const StringName &p_function, int p_idx, List<String> *r_options) const {

Tools: This one is likely going to be tricky, I don't think we can having tooling to enforce this. Well written guidelines and good PR reviews should do the trick most of the time. If anyone has ideas on how to partially automate testing those inconsistencies, that would be welcome though.
It's also not always clear whether a parameter should be r_ or simply p_, we might need precise guidelines on this (help welcome from anyone with a good understanding of all this).

Naming of boolean setters/getters

We have a lot of inconsistency in the current codebase on how we name boolean setters and getters, and especially those we expose to the scripting API.

See #33026 to discuss what the convention should be.

[akien-mga]

Discussing from #16521 should likely be merged in this issue.

[Zylann]

About space at beginning of block: I agree it looks odd in the example given, but I actually do this as well in particular when the code inside is larger, I think it improves readability a bit by making the code inside better stand out, rather than being "glued" to the top.

a r_ prefix (like "return" - BTW if anyone knows the proper technical jargon to refer to this kind of parameters, that would be useful for the docs).

I usually refer to them as out parameters, and I used the out_ prefix instead in my own code to mimick some other languages (C#, GLSL and some macros I sometimes saw in system lib headers). r_ is fine too though.

I agree about the other changes.

[aaronfranke]

BTW if anyone knows the proper technical jargon to refer to this kind of parameters,

C# calls the r_ ones "out" parameters as opposed to normal "in" ones.

static void _fill_array(Node *p_node, Array &array, int p_level) { // should be r_array

Personally I'd prefer static Array _fill_array(Node *p_node, int p_level) { // single outs should be return types

If an instance is needed to be passed in, then the existing way makes sense.

[YeldhamDev]

Many thanks for elevating this further.

I would like to once again to bring up comment formatting, not just the orthography, but also the comment styling:

// Most comments are done like this (lots of times without proper capitalization or period).

/*
But there are some rare cases that are done like this, even for single line comments.
*/

[Faless]

spaces

About space at beginning of block: I agree it looks odd in the example given, but I actually do this as well in particular when the code inside is larger, I think it improves readability a bit by making the code inside better stand out, rather than being "glued" to the top.

@Zylann I agree, there are cases, especially long blocks, and when ifs condition is broken over multiple lines, that having and empty line is very useful.

In this sense though, I always found it weird, that the empty line would be after the if, and not before the if. More specifically I suggest we allow code to have an empty line before the end of the block, not at the beginning.

if (some_cond) {
	ERR_FAIL_COND(foo);
	some_statement();

} else if (some_other_cond_that ||
		breaks_over ||
		multiple_lines) {
	ERR_FAIL_COND(foo);
	some_statement2();

) else {
	some_statement3();
}

Just a small consideration.

r_

About the out parameters, yes, C# calls them that way but I would leave r_ just to avoid a lot of useless changes (r_ I guess stands for ret, returned which was common C language).
I would definitely not use out_ which is way to long, but in case go for o_ consistent 2 letters for both input and output.
Again though, I would enforce r_ just for the sake of less pain in my future rebases.

[Krzycho666]

That's great to see that code style is being discussed!
I'm interested to know why don't we actually go for Allman/ANSI style when it comes to curly brackets Intending. It seems to be standard nowadays.

I'm working with Unreal Engine for quite few years now, editing and extending it's codebase on daily basis, and I find it way more easy to work with because of being Allman based.

[Faless]

I'm interested to know why don't we actually go for Allman/ANSI style when it comes to curly brackets Intending. It seems to be standard nowadays.

Well, I've always preferred the Linux kernel style, simply because it doesn't waste a line.
There's also a pretty extensive codebase using that style too.
For people who uses bigger fonts like me, it's much better IMHO.

EDIT: (I usually work with 25-30 lines per screen on my laptop, 35-40 on my desktop)

[Krzycho666]

I see that this might become the most controversial part of the discussion. ;)

Well, I've always preferred the Linux kernel style, simply because it doesn't waste a line.

I'm not quite sure about this argument, mostly because of two reasons:

1. There was empty lines after curly bracket all over the Godot codebase until now.
2. Some people (including you) have proposed empty lines in other situations, which would not be needed if ANSI indenting was used.

What I see on the other hand, is that it does save some space but it seems to be marginal (or non according to current Godot style). K&R standard was a thing in the 80s and 90s for C - and it made sense for a few reasons back then. For me it seems to be a habbit that doesn't quite feet to the needs of this project. There was shorter code blocks, shorter variable names, shorter lines, different language and the content overall is quite different.

// I'm displaying 62 lines in my editor

[Faless]

There was empty lines after curly bracket all over the Godot codebase until now.

Not always, only when deemed helpful.

Some people (including you) have proposed empty lines in other situations, which would not be needed if ANSI indenting was used.

Again, only when needed, not when it's useless.

[Faless]

E.g., all the:

if (something.is_null()) {
	return something_else;
}

which is the most common type of if, do not usually have empty lines in godot code.

[Calinou]

I think using Allman style for functions is fine, but I'd avoid it for control flow and loops (as is done in most PHP code styles). Otherwise, small functions with many conditions/loops can become really large as a result of all the line wrapping.

[aaronfranke]

@Faless Well, I've always preferred the Linux kernel style

The Linux kernel coding style guide uses the Allman style for functions:

However, there is one special case, namely functions: they have the opening brace at the beginning of the next line,

The justification for this inconsistency is incredibly thin, citing Kernighan and Ritchie as gods:

all right-thinking people know that (a) K&R are right and (b) K&R are right.

I don't know if it's such a good idea to use a style which justifies itself as "because we're right".

@Faless EDIT: (I usually work with 25-30 lines per screen on my laptop, 35-40 on my desktop)

Just pitching in, I usually have my editors taking up a whole screen, often on my 2nd monitor, where I typically display about 50 lines at once. My opinion may be biased as someone with plenty of screen space, but also I see @Krzycho666 has over 60 lines in their editor.

@Krzycho666 I see that this might become the most controversial part of the discussion. ;)

In any case, getting rid of empty lines in functions won't increase the diffs if we decide to change the brace style to Allman later, so this discussion doesn't block the changes in the OP.

[Faless]

The Linux kernel coding style guide uses the Allman style for functions:

But not for ifs. I don't mind functions, although I still see no need for a newline there, it's definitely less wasteful than in ifs.

I don't know if it's such a good idea to use a style which justifies itself as "because we're right".

In the old times, people in CS used to be fun and liked self-mockery.
it seems that is sadly no longer true in this world where everything is taken so seriously :( .

I don't know if it's such a good idea to use a style which justifies itself as "because we're right".

I gave you a motivation, I never said we should follow it because it's right.

Just pitching in, I usually have my editors taking up a whole screen, often on my 2nd monitor, where I typically display about 50 lines at once. My opinion may be biased as someone with plenty of screen space, but also I see @Krzycho666 has over 60 lines in their editor

Yep, people with plenty of space seems to have no problem with wasting lines, and I understand that, it's mainly a problem for people with small screens or who uses big fonts because their eyesight sucks (like me 👀 ).

[lawnjelly]

I'm actually happy to use whatever the democracy wants .. my personal preference is actually for Allman indentation for everything. I find it easier to instantly visually see where areas of code begin and end. With the Egyptian braces I find myself seeing end braces and having to waste time trying to work out where the brace starts from lol. Perhaps I just need to retrain myself 😄 .

Given that the godot style can be done with a clang format, you could conceivably just have something in your IDE that converts the file to your preference on loading, and converts it back when saving...

[groud]

I agree with @Faless here, the Allman style wastes a lot of space and is thus really inconvenient for small screens.
Also, IMHO, it does make things a lot harder to read since it scatters away logical blocks (in the functional logic, not the algorithmic one). So basically related lines might end up further away, which makes it difficult to understand what a block of code does.

[twaritwaikar]

I think my preference for Allman comes from my habit that whenever I see an opening or an ending brace, my instinctive behavior is to look for a correspondingly opposite sided brace around that bit of code at the same indentation, instead of a C++ keyword at the same indentation level, which is mostly in any non-Allman style. I am not saying my preference matters much but I propose that this can be efficiently decided by doing a poll of how many people who actively contribute to Godot prefer a particular style. It may not be of any use to try to convert a particular party of people who prefer a particular style, because in the end it is a preference and preferences rarely have logically reasons behind it, instead they come from habits and the kind of code people see everyday.

…

On Tue, Oct 29, 2019, 14:07 Gilles Roudière ***@***.***> wrote: I agree with @Faless <https://github.com/Faless> here, the Allman style wastes a lot of space and is thus really inconvenient for small screens. Also, IMHO, it does make things a lot harder to read since it scatters away logical blocks (in the functional logic, not the algorithmic one). So basically related lines might end up further away, which makes it difficult to understand what a block of code does. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#33027?email_source=notifications&email_token=AHSUAFE4CGMCMMKDGRUX4ZLQQ7Y4DA5CNFSM4JEREH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECPVGIA#issuecomment-547312416>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHSUAFDB2T6KEX3ZU3OLEJTQQ7Y4DANCNFSM4JEREH4Q> .

[Zylann]

I prefer the current brace style, for same reasons. While I think some of the style changes are good small improvements, I don't see the point in suddenly changing such a large portion of code which otherwise was already fine.

[aaronfranke]

I'm actually happy to use whatever the democracy wants

but I propose that this can be efficiently decided by doing a poll of how many people who actively contribute to Godot prefer a particular style.

In reality, open-source projects operate with more of a meritocracy and consensus than a democracy. Meritocracy is an idea that means those who contribute more (and therefore would be affected more by a change) get a greater say, which makes sense. Also, for consensus, it's better to have things that are inconvenient for some rather than blockers for others, and screen space can indeed be a legitimate issue, aside from the issues of changing up the entire code base. Seeing as how the lead devs prefer the current style, and many people here do, I'm 99.9% sure it will stay this way. And while it's not my favorite, this is probably for the best.

[Xrayez]

About space at beginning of block: I agree it looks odd in the example given, but I actually do this as well in particular when the code inside is larger, I think it improves readability a bit by making the code inside better stand out, rather than being "glued" to the top.

@Zylann I agree, there are cases, especially long blocks, and when ifs condition is broken over multiple lines, that having and empty line is very useful.

I increased line spacing slightly in my editor, and I've started to remove empty lines in some places (coincidence?!). For really short statements which don't exceed function signature I find that I don't add empty line either.

[neikeq]

About the always braces for if/else blocks, is this only if there's an else block or are the following also fobidden?:

if (cond)
    return;

if (cond)
    foo();

[Calinou]

@neikeq I think these shouldn't be allowed either, due to the likelihood of making mistakes and spending hours finding them 🙂

[hbina]

Did the discussion every brought up enums?

godot/core/math/math_defs.h

Lines 63 to 102 in ea8d9c5

	enum ClockDirection {
	CLOCKWISE,
	COUNTERCLOCKWISE
	};
	enum Orientation {
	HORIZONTAL,
	VERTICAL
	};
	enum HAlign {
	HALIGN_LEFT,
	HALIGN_CENTER,
	HALIGN_RIGHT
	};
	enum VAlign {
	VALIGN_TOP,
	VALIGN_CENTER,
	VALIGN_BOTTOM
	};
	enum Margin {
	MARGIN_LEFT,
	MARGIN_TOP,
	MARGIN_RIGHT,
	MARGIN_BOTTOM
	};
	enum Corner {
	CORNER_TOP_LEFT,
	CORNER_TOP_RIGHT,
	CORNER_BOTTOM_RIGHT,
	CORNER_BOTTOM_LEFT
	};

[Xrayez]

What about i++ vs ++i in for loops? The current codebase is quite inconsistent in this regard. I prefer ++i myself, but it some cases i++ can be used in array access starting from zero like arr[i++] if using while loops.

[Anutrix]

I would suggest not using i++ or ++i at all in places where both affects are different e.g, in array index. It always causes confusion and leads to typo bugs. Just using it in alone(like for incrementing or decrementing a value only) should be fine.

[Xrayez]

Yeah I agree, my question is rather: whether to use the ++i or i++ in for loops style-wise (in those cases where it does not make a difference). Even core structures like Vector do not follow consistent style regarding this.

[Xrayez]

What do you think about empty lines after curly blocks?

Variant ret;

if (!cond) {
	return ret;
}

for (int y = 0; y < height; ++y) {
	for (int x = 0; x < width; ++x) {

	}
}

return ret;

vs.

Variant ret;

if (!cond) {
	return ret;
}
for (int y = 0; y < height; ++y) {
	for (int x = 0; x < width; ++x) {
		// Do stuff.
	}
}
return ret;

I think the second one is much more compact and doesn't break the code in any way. I see these kind of empty lines sporadically throughout Godot and the codebase is quite inconsistent regarding this style, so I'd prefer to avoid those extra lines when they do not actually worsen the readability of the code. The rationale behind this is that a single closing bracket } already adds up to the space wasted for scoping purposes. It it almost the same as if you would write GDScript code, but without }:

var ret;

if (!cond):
	return ret

for y in height:
	for x in width:
		# Do stuff.

return ret;

Notice that there are still empty lines in GDScript code, but it takes almost the same number of lines written as in C++ version.

[Zylann]

I glue lines like this in short functions or related blocks within larger functions, but I dont think it makes sense to always strip empty lines after curly closes (or even bother about them). Maybe just allow them?

[Xrayez]

Loops that go through 2D data structures should likely use x and y variable names over i and j, see issues like #30424.

[Calinou]

In the long run, we should unify the use of #ifdef over #if defined() where possible in C++ and GLSL code. There are two ways to go about this:

• Always use #ifdef to replace #if defined() when possible.
  • More concise, but takes more time to refactor when a complex condition is needed (or to temporarily transform it into #if 0 or #if 1). There's also no widely available #elifdef yet (it was only recently added in GCC).
• Never use #ifdef to replace #if defined().
  • More verbose, but easier to refactor (and possibly less error-prone?).

I don't have a strong opinion on which option to go for, but I would prefer always using #if defined() for clarity.

[Geometror]

With this comment I would like to reignite this discussion since the inconsistencies in code style and structure make the code sometimes very hard to read or extend, especially affected are files which change quite frequently (e.g. editor_node.h/.cpp). I know, the are some controversial aspects but I am confident that we could come to a consensus or compromise for the most essential ones.
Often style decisions are made on RocketChat, but then they aren't documented but rather kept by word of mouth. As the Godot codebase grows and grows in size, finally creating an official code style guide could be of great advantage (especially for new contributors).
It could prevent repeating discussions, making reviews easier and most importantly, increasing the maintainability of the whole codebase and development efficiency. Due to the nature of Godot as a fast-evolving project with hundreds of contributors a lot of code needs to be read, so improving and maintaining readability is very important.

I compiled some points below which I found while taking a look at various parts of the codebase.

Points to discuss, most essential/general ones first:

1. Long descriptive names over short/compact ones using abbreviations? [the codebase contains quite a lot of singe-letter or ambiguous variable names]
   • naming patterns for specific data structures?
2. Clarify naming style for constants (const/static const/constexpr/...) [there are many places in the codebase where constants have lower case names]
3. Prefer one class per file? In which cases make exceptions? [some source files became very large with a few thousand lines of code, making navigation quite hard]
4. General class structure/order in headers: having clear sections, not mixing member and method declarations, where to place enum and struct declarations, having the same function order in header and source files, etc.
5. Add empty constructors and destructors?
6. Use RES or Ref<Resource>?
7. Prefer descriptive constants over magic numbers [this is a quite common problem]
8. Prefer explicit includes (only include what you use) [see https://github.com/godotengine/godot/pull/57760#issuecomment-1032174395 for a summary of the benefits]
9. Length/format of separator comments in source files between implementations of different classes (////////////////////////////////////////////////////////////) [these are of different length even within some files]
10. bind_methods structure: First all method binds (including setters and getters) and then the property binds or make blocks of getter+setter+property bind for each property?
11. ternary operator: Always put condition in braces, regardless of necessity? [Currently done in TextServer/Label/...]

More or less clear, but not documented:

• Use forward declarations wherever possible
• Initialize pointer member variables with nullptr
• Use the SNAME macro in frequently executed code

As proposed in godotengine/godot-docs#4685, the results of these discussions should be documented in the already present "Code style guidelines" page. In addition, we should refactor the codebase piece by piece (e.g. like in #58395). Of course, this might be a very lengthy process, but I think it's worth it.

[Calinou]

1. Long descriptive names over short/compact ones using abbreviations? [the codebase contains quite a lot of singe-letter or ambiguous variable names]

I agree with avoiding 1-character variables whenever possible. Plain English variable names should be favored, ideally full words unless the contraction is well understood and unambiguous (e.g. coords instead of coordinates).

2. Clarify naming style for constants (const/static const/constexpr/...) [there are many places in the codebase where constants have lower case names]

I'd say const should be lowercase (it's typically a local variable). static const should be uppercase and constexpr should be uppercase too.

Also, using const or constexpr should be done whenever possible to improve readability. Using const for primitive types generally doesn't impact performance, but using constexpr can improve performance since the compiler will compute the value at compile-time. Unfortunately, to my knowledge, this is difficult to enforce with the tooling currently available.

I'd say the only situation where const shouldn't be used (even though it is allowed) is for primitive parameter types in method declarations (e.g. void hello(const int p_example)). The p_ suffix we enforce for function parameters already suffices to avoid most pitfalls in this aspect.

3. Prefer one class per file? In which cases make exceptions? [some source files became very large with a few thousand lines of code, making navigation quite hard]

I agree one class per file should be the way to go, unless the class is very small (<= 100 lines in the header).

4. General class structure/order in headers: having clear sections, not mixing member and method declarations, where to place enum and struct declarations, having the same function order in header and source files, etc.

I'd say enum and struct declarations should be put at the top of the class, since they can be referred to without an instance and are immutable.

I agree with reordering functions to always match between header and source files. This is probably a long and daunting task that can't be easily automated, though.

5. Add empty constructors and destructors?

I'm not sure about this. Are there technical implications if we omit constructors and destructors (or if we add empty constructors and destructors everywhere)?

If there are no downsides to having empty constructors and destructors everywhere, it sounds like a good thing to do to make refactoring easier.

6. Use RES or Ref?

RES is a typedef for Ref<Resource>, so it behaves exactly the same. I think it would be better to remove RES as typing out Ref<Resource> isn't that much more work 🙂

Edit: The same applies to REF (which is a typedef for Ref<RefCounted>.

It's probably a good idea to review typedef usage and remove those that are either rarely used, or don't save enough typing to be worth the added choice paralysis.

See also #50990.

7. Prefer descriptive constants over magic numbers [this is a quite common problem]

Definitely. It's better to use constexpr over #define for those, by the way.

8. Prefer explicit includes (only include what you use) [see https://github.com/godotengine/godot/pull/57760#issuecomment-1032174395 for a summary of the benefits]

I'd go towards following the IWYU approach, even if it makes #include chains at the top of files longer. IDEs might be able to collapse those anyway (just like license headers).

Build times are starting to be more and more of a problem on the master branch, and I'll only get a CPU upgrade in 2023 at the earliest. Therefore, I'll take anything to improve build-time performance for now 🙂

9. Length/format of separator comments in source files between implementations of different classes (////////////////////////////////////////////////////////////) [these are of different length even within some files]

I think separator comments of that kind are counterproductive. In my experience, if your code is well-spaced and well-commented, you don't need any separator comments. Having multiple blank lines is not necessary either, especially since vertical space is precious on 16:9 displays.

10. bind_methods structure: First all method binds (including setters and getters) and then the property binds or make blocks of getter+setter+property bind for each property?

For consistency, I think we should follow what we're doing in the header. This means not grouping the setter/getter with the property if we don't do that in the header.

11. ternary operator: Always put condition in braces, regardless of necessity? [Currently done in TextServer/Label/...]

Always putting the ternary condition operator in braces sounds good to me. Does clang-format have a way to enforce this?

More or less clear, but not documented:

• Use forward declarations wherever possible
• Initialize pointer member variables with nullptr
• Use the SNAME macro in frequently executed code

I agree with all of those.

[Zylann]

Has there been any discussion about a convention for private member variables? In a few cases there is a _ in front, but most of the time there is nothing. It makes it hard to figure out which is local and which is member in long functions, even if parameters are prefixed (files aren't always read using a fancy highlighter). Another reason this is important for them to be different is because contrary to parameters and locals, member variables are subject to race conditions because they keep their state in between calls. So reasoning with them is very different. I have also never seen before another codebase with private members not having a specific style.

[Calinou]

Has there been any discussion about a convention for private member variables?

I'm of the opinion that all member variables should be prefixed with m_, but other core developers tended to disagree. Anyway, it's too late to make this kind of decision for 4.0 now.

[Geometror]

What about trailing underscores? That would keep names of member variables nice and clean while it wouldn't conflict with reserved identifiers.

[akien-mga]

These changes have been made, so closing.