08 Code Formatting Guidelines

Generally speaking, the PCSX2 project does not make strict rules or guidelines on how code should be formatted. Either same-line or next-line brace formatting is allowed, and most any variable naming convention is also allowed. There are, however, a few basic code formatting strategies that are unquestionably 'good' as far as readability is concerned, and any contributing coder will do both yourself and the rest of the PCSX2 team a big favor by trying to stick to these guides:

Topic 1: Do not use variable type prefixes!

Variable type prefixes are commonly known as Hungarian Notation, and are the sort of variable names popularized by the Win32API. They look something like this is in ideal situations:

DWORD dwSomeDoubleWord;

... and look something more like this in practical situations involving real code:

wxChar* lpwstrOmgThisSucks;

Besides being obscenely ugly, this naming convention is totally impractical for modern programming via an IDE with autocomplete features. Coders using such tools want to do autocomplete by the subject of the variable's purpose, and are then automatically provisioned variable type information via tooltip, making the prefix neigh worthless. If I'm looking for the MasterVolume of a voice, I might think VoiceVolumeMaster, MasterVolumeVoice, or VolumeVoiceMaster and in fact I'll probably have no idea of it's a dword, word, int, or float value type. Any code using this naming convention will be changed to something more sensible.

Just don't do it. Ever.

Topic 2: Using PCSX2 base types instead of C/C++ atomic types.

Most of the time you want to use either PCSX2 cross-platform types in the place of C++ atomic types. Most of the common types are:

u8, s8, u16, s16, u32, s32, u64, s64, u128, s128, uptr, sptr

These types ensure consistent operand sizes on all compilers and platforms, and should be used almost always when dealing with integer values. There are exceptions where the plain int type may be more ideal, however: typically C++ compilers define an int as a value at least 32 bits in length (upper range is compiler/platform specific), and temporary arithmetic operations often conform to this spec and thus can use int (a compiler may be allowed then to use whichever data type the target CPU is most efficient at handling). Other C++ integer types such as short and long should not be used for anything except explicit matching against externally defined 3rd party library functions. They are too unpredictable and have no upside over strict operand sizing.

Note that using the C++ atomic types float and double are acceptable, and that there are no defined alternatives at this time.

Special note: PCSX2-specific types uptr and sptr are meant to be integer-typed containers for pointer addresses, and should be used in situations where pointer arithmetic is needed. uptr/sptr definitions are not type safe and void* or u8* should be used for parameter passing instead, when possible.

wxWidgets base types are no longer preferred, and should be avoided when possible, though they can be used as needed when dealing with gui code.

Topic 3: Do not use private class members; use protected instead.

There is no justifiable reason for any class in PCSX2 to use private variable members. Private members are only useful in the context of dynamically shared core libraries, and only hinder the object extensibility of non-shared application code. Use protected for all non-public members instead, as it protects members from unwanted public access while still allowing the object to be extended into a derived class without having to waste a lot of silly effort to dance around private mess.

Topic 4: Name Protected Class Members with 'm_'

It is highly recommended to prefix protected class members with m_; such as m_SomeVar or m_somevar. This is a widely accepted convention that many IDEs are adding special built in support and handling for, and for that reason it can help improve class organization considerably for all contributors to the project. However, like most other guidelines it is not a definitive requirement.

Topic 5: Avoid compounding complex operations onto a single line with a function declaration.

Example:

// Not so good...
void DoSomething() { Function1(); Function2(); var += 1; }

// Good...
void DoSomething() {
    Function1(); Function2();	var += 1;
}

The reason for this guideline is that it can assist debugging tremendously. Most C++ debuggers cannot breakpoint function calls to the bad example, disabling a programmer's ability to use the debugger to quickly track the calling patterns for a function or conditional, and thus removing one of the most ideal tools available to a programmer for understanding code execution patterns of code written by another programmer. For these reasons, code written with such compounding may likely be unrolled onto multiple lines by another programmer at any time, if that programmer is tasked with troubleshooting bugs in that code.

If the operation of a function is simpler (one or two operations max), and if there are many such functions as part of a class or interface, then compounding on a single line for sake of compact readability may be justified; and in such cases, the programmer should also use the inline_always attribute on the function to denote that it has a trivial and repetitive operation that can be ignored during debugging.

Topic Ex: Specifying Function Inlining

IMPORTANT This section is preliminary documentation for a planned design, and is not currently implemented in the PCSX2 source code.

While modern compilers like to tout that the compiler should be left to decide function inlining, the truth is that for PCSX2's purposes, compiler inlining heuristics fail miserably, never inlining nearly as much as they should. It's often necessary for us, as programmers, to tell the compiler what to inline. PCSX2 offers four special classes of forced function inlining, which are provided as replacements for __forceinline, inline, and __attribute__(always_inline). When used properly, these inlining attributes help improve the performance and quality of development and debug builds.

inline_always this attribute force inlines a function even in debug builds. It is the functional equivalent of C#'s "skipover" attribute, and should generally be applied only to very simple accessor operations. It acts as both a speedup of debug build performance and as a convenience tool, allowing debuggers to trace through code without having to manually trace into every basic object constructor or property accessor.

inline_devel this attribute enables inlining in both development and release builds, and should be used on almost any function for which the programmer knows that inlining is in the best interest of the code performance. Keep in mind, however, that development builds are used as debugging and stack tracing tools by programmers and testers, and that means some types of functions are not well suited to being inlined. These namely include very long/complicated functions, which should bear the inline_release attribute instead.

inline_release this attribute enables inlining in public release builds only. This attribute should generally be used on functions that are very long and involved, and only called from one to three locations, thus making them good candidiates for inlining (these are cases the compiler's optimizer should be smart enough to inline itself in release builds, but generally fails to inline).

inline_never special attribute which force-disables inlining of a function completely. This attribute is of extremely limited use, usually to combat stupid inlining tricks that some compilers might fall into on special cases of complex function nesting, so you probably won't even need to remember it exists unless you make a habit of writing complex and hard to understand class implementations.

Topic Ex.2: Savestate Versioning

While PCSX2 willfully makes no promise to retain backwards support for savestates, it is none the less a convenience for developer, tester, and user alike when we do. So when possible it is recommended that programmers making changes to the savestate code (normally denoted as a Freeze() function) be sure to increment the savestate version in Savestate.h and, if you know how, implement backward support for the old version (if not, then simply increment the savestate version and let someone else fix the backwards compatibility in a later revision). This allows PCSX2 to at the very least fail gracefully when encountering an unknown savestate, and ideally it allows for another coder familiar with the system to properly implement backwards compatibility with the older savestate revision.