Godot-docs: Create official Godot C# style guide

[mhilbrunner]

[KellyThomas]

One thing that needs to be decided is what version of c# the sample code should be targeting.

Although the documentation currently states MSBuild 15.0+ is a requirement there is still an effort being made to maintain compatibility with earlier versions.

https://github.com/godotengine/godot/issues/17936
https://github.com/godotengine/godot/issues/15742

[paulloz]

This is some coding conventions from what I saw in the current documentation.

• Public fields, properties, methods should be named using UpperCamelCase
• Private fields, properties, methods and local variables should be named using lowerCamelCase
• Opening and closing brackets should be on their own lines
• Brackets can be spared if there's only one line inside the if, while, for etc.
• this. should be avoided if possible (I don't like this one but that's how everything is written right now)
• Local variables should be declared with var and not the actual type (same as above if you ask me)

[KellyThomas]

At the moment the demo projects have signal handlers named OnSignal() and OnClassSignal(). Once we have decided on a name scheme we should apply it consistently in this project and integrate it into the Connect Signal dialogue.

https://github.com/godotengine/godot-demo-projects/blob/master/mono/monkey_pong/Wall.cs#L6

public void OnWallAreaEntered(Area2D area)

https://github.com/godotengine/godot-demo-projects/blob/master/mono/monkey_pong/Paddle.cs#L26

public void OnAreaEntered(Area2D area)

[neikeq]

We should follow this naming convention: https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-type-members

• Private fields, properties, methods and local variables should be named using lowerCamelCase

• Properties and methods should be named using PascalCase regardless of their accessibility level.
• Locals should use camelCase.
• Public and protected static fields should use PascalCase. Private and internal fields are not mentioned in the guidelines, so I would be fine with camelCase or either _camelCase or _PascalCase (both with an underscore prefix). Use properties instead of public or protected instance fields.

• Local variables should be declared with var and not the actual type (same as above if you ask me)

Only where it makes the code more readable. e.g.:

``` C#
// ACCEPTABLE?
var node = GetNode("..."); // Node node = GetNode("...");
// OK
var floor = new StaticBody();
var sprite = GetNode("...") as Sprite;
foreach (var child in children) { ... }

``` C#
// NOT GOOD
var count = 10; // int count = 10;
var result = task.Result; // Whatever result = task.Result;

[neikeq]

I was using VS Code's auto format, not sure what style it uses by default, other than curly braces in their own line, 4 spaces for indentation, and no spaces between identifier and parenthesis or square brackets.

[mysticfall]

I agree with pretty much everything neikeq listed in the above comment, except for the use of var vs actual types.

Personally, I would use var in every case when it's possible because virtually everyone uses an IDE to code C#, but if the intention is to make the code more readable also in the documentation, I don't have anything against the suggestion.

[paulloz]

Agreed with @neikeq (once again, I wrote what the actual state of the documentation was, not what should be done).
Concerning this remark (to which I agree too):

Use properties instead of public or protected fields.

It should be noted that the current codebase does not comply:

• Vector3
• Vector2
• Transform
• ... (I haven't looked everywhere)

[KellyThomas]

I ~strongly~ prefer camelCase over either _camelCase or _PascalCase for fields.

[neikeq]

Although the documentation currently states MSBuild 15.0+ is a requirement there is still an effort being made to maintain compatibility with earlier versions.

Game projects can use C# 7, which requires MSBuild 15.0+. At some point we will have to enforce MSBuild 15.0 for both the API assemblies and GodotSharpTools.

[mysticfall]

By the way, I'm going to try a mass code clean up on the source files under modules/mono/glue/cs_files with my IDE's code inspector.

I'll try to configure it to the convention we agree upon this thread. So, if you have any suggestions or opinions about it, please let me know.

[mysticfall]

As to internal or private fields, it seems the standard conventions don't say anything but it looks like _camelCase or camelCase to be more commonly used than _PascalCase.

In my case, the default code style configuration of an IDE I use follows _camelCase convention, so I've been coding in that style for a while. But I don't have a too strong opinion against camelCase.

[paulloz]

It's weird to me that internal or private fields could use _PascalCase when it is used for public methods like _Ready or _Process...

[neikeq]

So between camelCase and _camelCase, which one is more commonly accepted?

I just noticed I've been writing "Casel" :open_mouth:

[mysticfall]

I'd say it's _camelCase, because Microsoft seems to follow that style themselves (i.e. an example, and another one) and Rider recommends it as default.

[paulloz]

I don't have a strong preference for one or the other.

[KellyThomas]

dotnet have this style guide explaining that they use:

• _camelCase for instance fields
• s_camelCase for static fields
• t_camelCase for thread static fields

So for them it is part of a larger name scheme.

---

On the other hand MS used to recommend stylecop which had rule SA1306.

[skyace65]

On the subject of using var I prefer using int, float, bool and string and so on over var.

[paulloz]

For documentation writing: when connecting to signals defined in the editor (e.g. here) we should keep the signal handler named in snake_case.

[aaronfranke]

@mysticfall has created a C# code cleanup PR here: https://github.com/godotengine/godot/pull/18053

I agree with most of his changes, but I am not a fan of changing everything to var, at all. It makes the type ambiguous, which is confusing and leads to problems. @mysticfall has encountered one of the issues with using var in his PR, as when he removed default assignments, he had to change the types back to explicit. It can also cause other problems like confusion.

The main use case of var is when it makes code more readable due to avoiding repetition.

var f = new Dictionary<int, List<VeryLongClassName>>();

is much better than

Dictionary<int, List<VeryLongClassName>> f = new Dictionary<int, List<VeryLongClassName>>();

But, replacing int with var makes the code less readable.

I vote for using type names instead of var, unless the type name is longer than ~10 characters.

[mysticfall]

@aaronfranke Thanks for taking time to review my code and share your opinion.

The reason why I prefer var is as follows:

• It's only for local variables, so their declarations are closely located to their use sites.
• In most cases, it's not difficult to infer the actual type of such variables - var a = 10 isn't that much difficult to read than int a = 10, for example.
• In some cases, it can take a bit more efforts to infer the actual type, like for such statements as var a = GetValue(). However, it's often possible to do so from the context (i.e. var x = vector.x), or by using an IDE.

If we decide to use vars conditionally, then I'd suggest to use it for only such cases where we declare a variable and instantiate the target type at the same time, like var x = new Type().

I don't see much benefit of using Vector3 pos = new Vector3() instead of writing var pos = new Vector3(), for example, so I wouldn't apply any hard and fast rule depending on the length of type names.

[aaronfranke]

While I would still personally prefer Vector3 pos, I would be fine with var pos for instances.

But when it comes to using var in place of int and real_t etc, all three of your points are basically "it's not that bad so why not". That's not really a good enough reason. I want you to explain why var is a better choice than int, not just that it's as good, acceptable, or doesn't cause problems (it can).

I'm not trying to pull a Torvalds on you @mysticfall, just trying to discuss things.

[Chaosus]

And public constants should be uppercase ? I hope so..

[mysticfall]

@aaronfranke

I want you to explain why var is a better choice than int, not just that it's as good, acceptable, or doesn't cause problems.

I'd say because it's better for consistency. It's the same reason why we better not mix different line break rules, even though it wouldn't cause any real problems.

If we have a consistent rule, it's easier to configure an IDE or other static code analysers to check if new code conforms to the accepted standard of the project.

I'm not trying to pull a Torvalds on you @mysticfall, just trying to discuss things.

I fully understand it. So, please don't worry about it :)

[aaronfranke]

That's a good point. However, it wouldn't be hard to make a consistent rule for style-checking programs, we'd just have it search for when the type name text is present twice in the same line!

And maybe, as I've suggested, only check if the type name is longer than X amount of characters.

Personally I would rather use var nowhere than everywhere it can be used, I'm sure you disagree :)

EDIT: "use var in every case when it's possible because virtually everyone uses an IDE to code C#"

Actually, when making this PR I often used Notepadqq (basically Notepad++ for Linux)

[aaronfranke]

For casing, here is my proposal: PascalCase methods, camelCase variables, and UPPER constants.

It makes them easy to differentiate, and this is the styling that Unity uses (and while this isn't the only reason to do it this way, appealing to Unity users should definitely be one of the goals of Godot C# to promote Godot). @paulloz @neikeq @KellyThomas

@Chaosus 's PR changes constants to UPPERCASE: https://github.com/godotengine/godot/pull/18038

[mysticfall]

I don't think it'd be a good idea to follow Unity in this matter, as it's notoriously negligent in following the standard coding conventions (i.e. lower case properties).

I think it'd be more reasonable to follow the style guide published by MS themselves which suggests using PascalCase for constants:

• https://blogs.msdn.microsoft.com/mschray/2013/05/06/practical-naming-standards-for-c/
• https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions

[paulloz]

@aaronfranke What are you calling variables? You pinged me in godotengine/godot#17212 saying the fields I was talking about should not be capitalized. As stated above «public and protected fields should be avoided, use properties instead» so anyway something should be changed about these fields as public API should not expose mixed case convention...

[aaronfranke]

A variable like real_t x in Vector3.

[paulloz]

A field then.
It's been addressed above:

Public and protected static fields should use PascalCase. Private and internal fields are not mentioned in the guidelines, so I would be fine with camelCase or either _camelCase or _PascalCase (both with an underscore prefix). Use properties instead of public or protected instance fields.

End user should not care about whether the exposed stuff they use are fields or properties, and should not bother thinking «well, this exposed value is a field so I access it with a lowercase name but this one is a property so I access it with an uppercase name»...

I think it's a bigger issue than discussing usage of var which is an internal coding style thing that can be addressed any time. The exposed API naming convention needs to be freezed before the first stable release as later modifications would be code breaking.

[mysticfall]

I think it could be a good time to gather all the issues in this thread, on which we still need to make an agreement:

1. Wheter to use _camelCase or camelCase for private fields.
2. As to use of vars : a) Never use them b) Always use them c) Only when duplicating type info d) Depending on the length of type names.
3. Whether to use PascalCase or CAPS_WITH_UNDERSCORES for constants.

For me, it's _camelCase, b) or c), and PascalCase. I'll update my PR as soon as we reach an agreement about these issues.

EDIT: I don't have too strong an opinion on vars and it seems that no one else is going for 2.b, so I'll switch my choice to 2.c to make it easier to reach an agreement.

[paulens12]

1. I propose mPascalCase as an alternative. The m stands for member, and it serves the same purpose as _camelCase. But m is in a way more comfortable position than underscore, at least on US QWERTY layout.

2. Am I the only one to pick d? I think b makes the code unreadable, but it's totally fine to replace some 20 chars long type names with var, because the majority of type names would still be visible, and IntelliSense shows the rest anyway. I think var should not be abused (e.g. using it for int), but where it really helps save time, it's not pure evil.

3. That depends on how you name public properties. You can't use PascalCase for both, that would be confusing.

[exts]

Not a fan of m if we're going to even bother with a prefix we should go with what microsoft has been using as an unofficial standard for years which is the underscore for private class members.

[mysticfall]

Regarding @paulens12's suggestions,

As to 1, Personally, I don't like using an arbitrary prefix that isn't used often by other projects. It reminds me too much of the Hungarian notation, which is generally avoided by C# programmers.

Besides, properties and methods are technically also members of a type, so it could be confusing to use the m prefix only for member fields.

As to 2, The reason why I chose c rather than d is that I don't see much benefit in using Vector3 pos = new Vector3() instead of var pos = new Vector3().

In that case, Vector3 is just 7 letters long, but it's obvious what the type of pos would be even when we replace the first type name with var.

On the other hand, something like SomeLongTypeName value = GetValue() wouldn't be as obvious if we replace SomeLongTypeName with var. So, I don't think we should decide when to use var soley on the length of type names.

And regarding 3, I think we should go with the coding guide provided by MS in this case, which is PascalCase for both constants and public properties.

[paulens12]

@exts the reason why I'm proposing m is that underscore is really uncomfortable to type and at least me personally, I'd like to avoid having to type it at all. I get that m is not widely used, but at the same time I don't really get why anyone would choose underscore apart from "everyone else uses it". Why does everyone else use it? Well, I don't know. I think out of the three options (underscore, m, or nothing at all, using this. where appropriate instead), underscore is the worst.

[exts]

@paulens12 I get why you want it, but 'm' was never a standard in c# so no need to introduce an arbitrary prefix like that imho. We need to keep things as C# standard as possible where people unfamiliar with godot can come in and contribute. We don't need to focus on standard practices that has been around purely in the gamedev space for years purely because it's a common thing in the gamedev space (mostly seen in C++ projects)

To answer your "why do people use it" because for years microsoft used it, the people that created the language.

[paulens12]

good points. As for 2., I personally would choose something between c and d, but that was not an option. C means using var ONLY when duplicating type info, which means long names would still have to be typed out. On the other hand, d means that short names could be duplicated. I think the best way would be in the middle: use var when the name is either too long or it's duplicated.

As for point 1, I really just wanted to express my disgrace with underscores here. I don't have a very strong preference of what prefix to use or whether to use them at all, I just dislike underscores, if that makes any sense. Maybe after all, this. is the best choice. Personally, I feel it's still faster to type than stretching my fingers out to reach shift and minus.

[mysticfall]

@paulens12 I'd be happy to go for something between c and d, or to be more specific, suggesting both as recommendations rather than hard-and-fast rules.

Probably it's ok to allow some flexibilities here, since it's not a convention about public APIs. (@paulloz also mentioned something similar on the Discord channel.)

[mysticfall]

@mhilbrunner just coerced persuaded me into writing a draft for the style guide and I'll begin in a couple of days.

So, it'd be nice if you could share your thoughts on the issues raised above before then, even though we'd still continue discussing the matter after I'm done with the draft if it's needed.

[paulloz]

1. Don't care about _ or no _ in front of private fields, that's trivial.
2. var should be used only when it does not impact readability (e.g. there should not be a rule telling people to write var x = GetNumber(); and not int x = GetNumber();).
3. Don't care either.

---

My two cents about how a coding style should be thought / written.

Having a coding style in a project involving a lot of people (whether it's FOSS or not) is more about consistency and readability than about having appealing looking code or lowering keystroke number.

There's no definitive answer to «should we prefix or suffix names with underscores» or «should we prefer lower or upper case». A coding style will never be consensual, there'll always be developers who do not agree with some rules.
It does not matter because conventions aren't here to be good or bad but to ensure that one can read the code in the same way anywhere across the codebase.

IMHO (and this is especially true in FOSS) if a rule does not serve this purpose it'll just feel arbitrary and will prevent people from getting involved and/or from following other (and more important) coding style rules.

[aaronfranke]

For the record, my preference order for var usage choices is d, c, a, b. @neikeq will probably decide.

but it's obvious what the type of pos would be even when we replace the first type name with var.

If we add Point3 for int positions like I suggested, then pos could be ambiguous.

And I do agree it's useful to write the type name if it's not already stated on the same line.

[neikeq]

1. As to use of vars : a) Never use them b) Always use them c) Only when duplicating type info d) Depending on the length of type names.

I think it's not that simple to make a set of rules.

• _a)_ is discarded because there are many places where using var improves readability.
• _b)_ is discarded because there are many places where using var hinders readability.
• _c)_ is fine, but there are exceptions. For example, I think foreach (KeyValuePair<string, Node> kvp in dict) {} could be replaced with foreach (var kvp in dict) {} as long as dict is declared near the loop.
• _d)_ misses the point. Although var is most useful to avoid typing long names, it doesn't necessarily improve readability.

I would simply leave it as "when it improves readability" and add a few code examples from this discussion.

... (e.g. there should not be a rule telling people to write var x = GetNumber(); and not int x = GetNumber();).

In this case, using var does not improve readability, it does the opposite.

One thing I would recommend is to always use built-in type keywords instead of var. I don't see how var would improve readability versus them. Even if you write 1.25f, 1.25d, true/false or a string literal, the keywords are just short to type...

EDIT:
I think I could make an exception about string literals, e.g.: var name = "Manolo", but definitely not chars: char c = 'a'.
Typing object instead of var is also important, IMO, because it makes it clear you are dealing with a downcasted reference.

[mysticfall]

I think we are close to an agreement on the usage of vars. Thanks everyone for the input!

Now we only need to determine on the other two issues, so please keep them coming :)

[neikeq]

I guess we will go with _camelCase for private fields, since it's what most prefer.
Constants should use PascalCase, not CAPS_WITH_UNDERSCORES.

[aaronfranke]

I do have a question about constants, how come Microsoft suggests PascalCase for constants but yet System.Math has an all-caps constant PI? Should we keep Mathf.PI capitalized for consistency with System.Math? What about other constants like proposed TAU? Should we just use PascalCase for constants with names longer than a few characters (like Mathf.Epsilon vs Mathf.EPSILON)?

[mysticfall]

@aaronfranke
That's an interesting case, and I can only assume they were rather inconsistent in naming it. My guess is that they erraneously applied the exception about two-letter acronyms which shouldn't be the case with PI:

A special case is made for two-letter acronyms in which both letters are capitalized.

A funny thing is, they even show PI as an example of non-standard naming practice in their guide:

• https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/capitalization-conventions

So, I guess it was a mistake from their part, but I agree we need to decide if we should replicate such an error in our API or not.

[neikeq]

Indeed. Let's use Pi and Tau.

[mysticfall]

I just updated #18051 to reflect how we decided on the proper usage of implicit typing.

By the way, am I correct in assuming that we shouldn't use any C# 7.0 features in the engine source?

From godotengine/godot#17936, it seems that the requirements for Mono build is MSBuild 14, not 15, so I was wondering if I should mention that the source must conform to the C# 6 specification in the style guide.

[skyace65]

@mysticfall
I don't think MSBuild 14 is the minimum. The Introducing C# in Godot blog post on the Godot website mentions MSBuild15 as the minimum. Compatibility with MSBuild14 might have been accidental or plans could have changed. I'd recommend asking one of the devs who might know just to be sure.

[mysticfall]

@skyace65 It seems that @KellyThomas already mentioned it in his first comment in this thread:

• https://github.com/godotengine/godot-docs/issues/1296#issuecomment-378566156

Personally, I think it'd be better if we target C# 7 with MSBuild 15, if there's no practical reason that prevents it.

@KellyThomas , @neikeq Could you share your thoughts on this issue?

[mysticfall]

I've encountered a case where we need an explicit cast even if a code inspector I use (Rider/ReSharper) thinks it as redundant.

Can I use a vendor specific directive to suppress such false positives, to prevent mistakes? :
c# // ReSharper disable once RedundantCast var max = (float)Mathf.Max(r, Mathf.Max(g, b));

[KellyThomas]

Some users are finding Mono 5.4 to be incompatible with the MSBuild 15 that ships with Build Tools for Visual Studio 2017. Workarounds are to use MSBuild 14 or install MSBuild 15 via Visual Studio Community (or paid).

As long as the Godot stable builds require Mono 5.4 and there is no real need to use c#7 features I would suggest avoiding them in engine code.

[aaronfranke]

Unfortunately, Ubuntu 18.04 will be shipping with Mono 4.6. Normally I'd suggest using whatever the mainstream versions are, but I doubt we'd drop to Mono 4.6.

We may wish to add a link to the Compiling with Mono page about how to install newer Mono.

[mysticfall]

Ok, then I'll use C# 6 / MSBuild 14 until all related issues get resolved. Thanks for the info!

[aaronfranke]

So... tabs or spaces? Tabs are easier to add/remove when working with code. The existing C++ code seems to use mostly tabs, and more editors use tabs by default. But spaces have a more consistent size. GitHub treats tabs as 8 wide, while most other editors treat tabs as 4 wide. Code written using tabs would look different in GitHub vs in text editors. Honestly I don't care which, but we should pick one.

[paulloz]

A comment above is mentioning this:

other than curly braces in their own line, 4 spaces for indentation, and no spaces between identifier and parenthesis or square brackets.

[mysticfall]

I've been rather slow to write the style guide draft, but I already wrote a section which recommends using spaces with soft tabs following this guide (the first item under _'Layout Conventions'_):

• https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/inside-a-program/coding-conventions

I'll fix the formatting of existing source files while I'm doing godotengine/godot#18051.

[mysticfall]

Just opened a PR with an initial draft of the style guide. Please feel free to suggest changes, and sorry for the delay!

[aaronfranke]

For integers stored in a floating format, should we write 0 and 1 or 0f and 1f?

[mhilbrunner]

Closing this for now, as we now got a first version of a C# style guide thanks to @mysticfall and the other contributors here. Thanks for the discussion and helping out, everyone! :)

For specific changes and additions, we should create new issues and PRs to discuss.

[aaronfranke]

For the record, here's a link to it: https://godot.readthedocs.io/en/latest/getting_started/scripting/c_sharp/c_sharp_style_guide.html