Skip to content

Coding Standards

Alex Miyamoto edited this page Oct 9, 2019 · 5 revisions

The following standards are those that should be enforced in Usagi. There may be older code that does not adhere to these standards, that code should be corrected as you come across it.

Editor settings

Your editor should be set to use tabs, not spaces. Tab size should be four characters. That said alignment of variable names should be done with spaces rather than tabs for consistency regardless of how the code is viewed.

Naming Conventions

A variables scope in usagi is identified by following prefixes
m_ - Class member variable
g_ - Global variable
ms_ - Static member variable
gs_ - Static global variable
Local variables and struct member variables have no preface

Reasoning:
Identifying the difference between member and local variables at a glance makes code more readable to those unfamiliar. It's also reduces the chances of accidentally shadowing a variable (as users of Unreal are likely to have done on many occasions). Local variables have no prefix.

A variation of hungarian notation is used in usagi for the following types
b - bool
u - unsigned integer
i - signed integer
p - pointer
f - float
d - double
r - real (either float or double depending on specified precision)
e - enum
v - vector
m - matrix
q - quaternion rotation

real is currently float32 but has been added in the hopes that the engine can in the future optionally be built with 64bit precision floating point maths. Always cast from real to double/ float as its precision should never be assumed.

You can combine p and the others, however p should always come first, e.g.:

Vector3f* pvFaceDir;  <--- OK
Vector3f* vpFaceDir;  <--- BAD

Reasoning: It should be possible to understand code at a glance even without comments. It is particularly useful to know at a glance whether a vector is being multiplied by another vector, a float or indeed a matrix.

The first letter of a variable name (excluding the scope prefix) should be lower case, this can be the letter identifying the type, or for more complex types it would be the first letter of its name. Each new word should begin with an upper case letter to help with readability.

Examples: Member variable:

float m_fMyFloat;

Local variable:

bool bDone;

Static member variable:

static MyClass ms_myClass;

Function naming: Regardless of scope all function names should start with a capital letter with each additional word also being in upper case. This is true regardless of scope and applies to both member and global functions.

Example:

bool MyFunction();

Global functions should be contained within a namespace e.g.

namespace Math
{
    float Max(float fA, float fB);
}

Formatting

A curly bracket should always have a line to its self so that the opening and closing brackets align up.

e.g.

if (bFellDown)
{
  GetBackUp();
}

Reasoning: This makes it easier to find each braces counterpart

Statements such as for, while and if should always use braces to surround the statements, the following is not acceptable:

if (bFellDown)
  GetBackUp();   <--- BAD!

Reasoning: The amount of code may increase, and unexpected behavior is less likely to be introduced during a merge.

Structs and Classes

Although the only way structs and classes differ is whether they default to public or private at Vitei we treat structs as raw data by convention, they should only have public variables and no member functions. It is not necessary to explicitly use the public keyword.

Classes however should declare their public variables and functions first, followed by protected and then private, e.g.

class MyClass
{
public:
   MyClass();
   ~MyClass();
   void RequestDoSomething();
protected:
   virtual void DoSomething();
private:
   uint32 m_uRequestCount;
} 

Reasoning: Headers are the first thing a user should look at when determining how to use a class, as such they should be ordered by how relevant the information is to them (with private member functions and variables ideally of no interest to someone just wanting to understand how to use a class).

All variables in a class should either be protected or private (and usually private, you should prefer protected getters and setters over protected variables where possible.

Member initialization

Member variables should be initialized in the order they are defined in the header through initializer lists rather than in the constructor. Please note that this is not currently the case and most variables are initialized in constructors.

Struct variables may be make use of class member initializers, e.g.

struct MyStruct
{
  int iValue = 5;
};

But try to avoid these for classes. They are still acceptable when a simple class has multiple constructors (such as a maths class which needs to be lightweight and will likely have numerous constructors). For more complex and less performance critical classes it is preferable to have a private function for initializing the variables.

Reasoning:
Structs should not include functions by convention, so class member definitions in the declaration of the struct allow us to keep true to this practice. However in a class all members should be protected or private and are only in the header for the benefit of the compiler. We can't move the variables into the CPP file without a PIMPL, but we can atleast stop changes to default variable values from causing every file using that includes the header from recompiling, and avoid showing a user any additional information he doesn't need to understand how to use a class.

Variables

Rather than using base types you should use variables defined in common_ps.h on a per platform basis, on windows these are:

typedef unsigned char      uint8;
typedef signed char        sint8;
typedef signed short       sint16;
typedef unsigned short     uint16;
typedef signed int         sint32;
typedef unsigned int       uint32;
typedef long long          sint64;
typedef unsigned long long uint64;
typedef wchar_t            char16; 
typedef float              float32;
typedef double             float64;
typedef size_t             memsize;
typedef HWND               WindHndl;

// Should start using these for areas where we want to be able to specify precision globally
typedef float		   real;	
typedef int		   sint;
typedef unsigned int	   uint;
typedef size_t             memsize;

Going forward real, sint and uint should be the default variable types except where the size of the variable is relevant.

Enums should be strongly typed, and there size specified for cross platform consistency. Please not this is not currently the case, but should be observed for all new additions.

enum class GamepadHand : uint8
{
   Left = 0,
   Right
};

One exception is when using an enum to declare a bitfield as strong typing can simply make the code more verbose.

enum GamepadCaps
{
   CAP_POINTER = (1 << 0),
   CAP_ACCELEROMETER = (1 << 1),
};

Namespaces

All engine code should be wrapped in the usg namespace in order to avoid conflicts with third party code.

Game specific code does not need to use a namespace.

Global functions should be placed in their own namespace (a namespace within usg in the case of engine code).

NEVER use using in a header file, and avoid using it in cpp files unless it is a particularly verbose or deep namespace.

Source files

All files should be started with the following copyright notice (the full licence is only to be included in license.md):

/****************************************************************************
//	Usagi Engine, Copyright © Vitei, Inc. 2013
****************************************************************************/

All cpp files should include the following header first:

#include "Engine/Common/Common.h"

Although many files do it is not necessary to include Common.h from the headers. This file is currently the pre-compiled header for all classes (note: this is subject to change).

When including other headers you should list engine headers first, followed by game, e.g.

#include "Engine/Common/Common.h"
#include "Engine/Scene/Scene.h"
#include "GameView.h"

Do try and leave a new line at the end of files for compatibility.

Forward declaring

Header files should only have the absolute minimum number of headers included to avoid increasing compile times. Any variables or function parameters which are pointers or references should be forward declared rather than including the relevant header files.

You may either forward declare in place, e.g.

void Init(class GFXDevice* pDevice);

or at the top of the header file, straight after all of the header includes

namespace usg
{
   class GFXDevice;
}

Comments

Usagi doesn't have many comments - all of the coding standards are geared towards creating code which is understandable without too much explanation

The copyright notice in header files should usually be followed by a description of the class

/****************************************************************************
//	Usagi Engine, Copyright © Vitei, Inc. 2013
//	Description: Class for rendering debug output to the screen
*****************************************************************************/

Variable and function names should be clear enough that the purpose is understandable without explanation. Do put comments in everywhere where explanation would be beneficial, but there is no convention for documenting every function in Usagi.

Always make comments by prefacing each line with //, do not use /* */ to comment out sections except for the copyright notice.

Code should be deleted, not commented out when no longer required. Source control is the correct method for keeping track of old code.

If you do need to temporarily comment out a block of code for any reason this should be done with

#if 0
... 
#endif

And not

/*... 
...
...*/

Place comments freely inside functions, however if you find a function is no longer self-explanatory also consider breaking it up into sub functions.

Reasoning:
There is a tendency for code and comments to get out of sync, an incorrect comment is worse than no comment at all. This should be picked up in code reviews, but as we are a small team to be creating our own tech we must try and limit our own workload.

STL

Try and avoid using the standard template library, we previously used our own templated structures and are in the process of switching over to the EASTL. You should not reference the eastl namespace directly, the choice of library and specification of allocators isn't something users need to be aware of.

The majority of types have been defined in the usg namespace, e.g.:
vector, array, deque, hash_map, map, set, string, wstring, string8, string16 and string32
Note that most functions currently still take the usagi U8String. If you use a string for internal purposes please specify the appropriate type (string8 for filenames).
You should always use these variants rather than anything in the std namespace.
Where appropriate we will be replacing

C++ 11

There was no C++11 support on some of the platforms we supported, but it is now highly unlikely that you will find a compiler that doesn't support it so we are embracing a number of the standards.

USE FREELY:

Use override, and where appropriate final, when overriding virtual functions
Use nullptr in place of null
Make use of static_assert
Strongly typed enums

USE SPARINGLY:

Lambda functions are allowed but should never be more than a few statements in length.

auto
The auto keyword should only be used sparingly. A good example of when to use it would be when the resulting type is obvious and it will decrease verbosity (such as a templated structures iterator)

auto pWheelShape = rtd.wheelsData.wheel[i].rtd.pShape;   <-- Bad, the type is not obvious (real code in the engine)
auto it = GameComponents<RigidBody>::GetIterator(); <-- Ok, iterators are verbose and as most iterators behave the same way this isn't hiding data from the user

In lambda functions and templated code it can be hard to avoid auto, no special effort should be made to avoid using it, but it should not be liberally used as a convenience.

Range based for loop

Ranged based for loops are acceptable as they reduce verbosity, however try and avoid using the keyword auto.

for (SpotLight* pLight : m_spotLights.GetActiveLights()) <-- Class type of it pLight is obvious 
for (auto pLight : m_spotLights.GetActiveLights())  <-- Class type of it pLight is hidden 

Visual studio / Visual Assist will usually be able to pick up on the type of auto so go to definition will still work, however as a rule you shouldn't be relying on IDE functionality to make something understandable to the reader.