Skip to content

Code Style

Anders Jenbo edited this page Mar 19, 2019 · 1 revision

C vs C++

Even though the source files are confirmed to end with .cpp, all Diablo code was C code.

Formatting

We have set up a .clang-format definition that should be used for automatically formatting the code to the project code style. The formatting is based on WebKit-code style, with some adjustments to better fit what has been found to be the original code style. Note that we only follow the formatting part of the WebKit style (i.e. everything clang-format automatically cleans up).

Since the source code must compile as C code, local variable declarations are separated from statements. To keep uniform formatting, insert a new line between the local variable declarations and statements of functions. See the example below:

good:

int f() {
	int x;

	x = 5;
	return x;
}

bad:

int f() {
	int x;
	x = 5;
	return x;
}

Data types

In general we've found that the original code mostly used Windows Data Types.

Some globals and parameters are using a hungarian notation specifying the type. The ones we've identified for now are the following:

BYTE bLen;
BOOLEAN bLen; // byte-sized bool - note that BYTE and BOOLEAN are only distinguishable in context
BOOL bLen; // BOOL is 32bit
WORD wLen;
DWORD dwLen;
static BOOL sgbSomeFlag;
static DWORD sgdwSomeDword;

The n name prefix (nLen) seems to be inconsistently used in regards to variable size, usually just meaning some count or flag.

For any other variables and parameters try to stick to these as well, as outlined below.

Booleans

Most of the code uses standard WinAPI BOOL (32-bit) as boolean type. However, some of the code (esp. networking code) uses byte-sized boolean values. For these a BOOLEAN should be used.

To maintain a consistent code style between those types, the WinAPI defines TRUE/FALSE should be used in any case as boolean literals.

Unsigned integers

  • 8-bit: Use the WinAPI typedef BYTE for that.

    Reasoning: We've found that a lot of DirectDraw code is based closely on DX SDK samples, which use BYTE in these cases.

  • 16-bit: Use the WinAPI typedef WORD for that. Theoretically both USHORT and WORD would be possible, but no definitive hints as to the correct one are found yet, so for now we are just choosing one.

  • 32-bit: Use the WinAPI typedef DWORD for that.

    Reasoning: The asserts found in the 1.00 Debug version show casts to DWORD.

  • 64-bit: Use unsigned __int64 for that.

    Reasoning: Although using the WinAPI typedef QWORD would be more in line with the 32-bit-sized type, that typedef did not exist in the Windows SDK supplied by VC++ versions Diablo 1.00 and the earlier patches shipped in (VC++ < 5). It's pretty unlikely that they changed the type retroactively in the later patch releases. (long long was not a thing in these older compilers either.)

Signed integers

TBD. So far we haven't found anything hinting at specific types, so for now the types remain the regular types IDA spat out (char, short, int)

Rules

Pointers

Null-pointers should use NULL instead of 0 to stand out more between numeric values.

Increments and decrements

  • Avoid all increments/decrements used as expressions and try to rewrite the code to include them as statements unless it'd prevent the compiler from generating binary-exactly or contradict with other information we have (like all local variable names).

    Avoid:

    foo[++count] = 0;

    Prefer:

    count++;
    foo[count] = 0;
  • Avoid pre-increments/-decrements whenever possible. The default should be foo++/foo--.

Conditions/checks (esp. zero/falsy checks)

  • Booleans: Use the expression itself: if (!condition)
  • Pointers: Use explicit comparisons with NULL: if (ptr == NULL)
  • Integers: Use explicit comparisons with 0: if (count == 0)

Reasoning: Comparing with the literals of the right type makes the intent of the check clearer in many cases.

Comments

  • Use // TODO: for TODOs on devilution's side.
  • Use // BUGFIX: as markers for actual Diablo bugs.