Coding Conventions

Table of Contents

1. Pascal Coding Conventions

In general, we follow the standard Lazarus and Delphi coding conventions, used throughout most modern Object Pascal code.

These are documented nicely on:

Some particular conventions:

1.1. Indent by 2 spaces

procedure Foo;
var
  X: Integer;
begin
  X := 1;
  if Y > 4 then
  begin
    while Z = 'hello' do
    begin
      Bar;
      Xyz;
    end;
    WriteLn(X);
  end;
end;

1.2. Do not use tabs or trailing whitespace

Do not leave "trailing whitespace" at the end of lines.

Reason: In the long run, it causes unnecessary diffs when someone removes this whitespace, which many text editors (and some humans :) ) do automatically.

Do not use tabs. Use only spaces for all indentation.

Reason: Tabs are nice when they are used 100% consistently for indentation, but in the long run it’s hard to enforce using them 100% consistently esp. when many people are working on the same codebase. And when tabs are used inconsistently (sometimes mixed with spaces for indentation), problems arise — because people have different tab width in their editors, so for some people indentation will look broken. It’s much easier to enforce the rule always use spaces.

1.3. Use PascalCase for everything

PascalCase means that the first character of each word is capitalized. Use it for all identifiers.

  • Including constants. So write MyConstant instead of e.g. MY_CONSTANT.

  • Including local variables. Even 1-letter variable names (so write I instead of i).

  • Including type names. Even the type names that are Pascal keywords (so write String / Boolean instead of string / boolean). Note: this rule was changed during CGE 6.5 development. So you will find a lot of code using lowercase string now in engine sources, but new code should use String.

Warning
The keywords nil, true, false are still lowercase.

1.4. true, false, nil are lowercase (not True, False, Nil)

Reason: We just historically wrote them always as lowercase and now it would be too big effort (for too little gain) to change. Some Pascal codebases write them also lowercase (e.g. LCL, although it is not 100% consistent) so we’re not alone in this decision.

1.5. Put begin on a new line, indentation of begin and end should match

Do not mimic C "K & R" style (https://en.wikipedia.org/wiki/Indent_style#K.26R) in Pascal:

// DON'T WRITE THIS:
for I := 1 to 10 do begin
  Writeln(I);
end;

Instead, the "begin" should usually be indented the same as "end".

// THIS IS OK:
for I := 1 to 10 do
begin
  Writeln(I);
end;

To look simpler, it’s OK to omit begin/end when they would surround only 1 statement:

// THIS IS EVEN BETTER:
for I := 1 to 10 do
  Writeln(I);

1.6. Put else on a new line, unless it’s right after end

The "else" keyword is written on a new line, unless it’s right after "end". So:

// THIS IS OK:
if Foo then
  Bar
else
  Xyz;

// THIS IS ALSO OK:
if Foo then
begin
  Bar
end else
begin
  Xyz;
end;

// THIS IS ALSO OK:
if Foo then
begin
  Bar
end else
  Xyz;

// THIS IS ACCEPTABLE, BUT BETTER AVOID IT:
if Foo then
  Bar
else
begin
  Xyz;
end;

// THIS IS NOT OK:
if Foo then
begin
  Bar
end
else
begin
  Xyz;
end;

// THIS IS NOT OK, BUT IS USED IN A LOT OF CODE:
// (Michalis was using this convention for a long time,
// until it was pointed to him that it doesn't look optimal,
// and Michalis agreed :)
// Do not use this in new code, but don't be surprised if it still occurs somewhere.
// Michalis will gradually get rid of it in CGE sources.)
if Foo then
  Bar else
  Xyz;

1.7. Put if on a new line, even if it’s right after else

// THIS IS OK:
if Foo then
  Bar
else
if SomethingElse then
  Xyz;

// THIS IS ALSO OK:
if Foo then
begin
  Bar
end else
if SomethingElse then
begin
  Xyz;
end;

// THIS IS ALSO OK:
if Foo then
begin
  Bar
end else
if SomethingElse then
  Xyz;

1.8. Do not use with

Never use with keyword. Using with makes the code very difficult to read, as some of the symbols inside the with A do begin …​ end clause are bound to A, and some are not, and it’s completely invisible to the human reader which symbols are which. And it’s impossible to determine it, without intimately knowing the complete API of class/record A.

Note
Speaking in more general (and opinionated) terms: Using with produces, in our opinion, code that is "easier to write but harder to read" and this is a bad tradeoff. The readability of the code is much more important (because the code must be reviewed and maintained to be reliable).

E.g. what does this code do?

with A do
begin
  SourceX := X;
  SourceY := Y;
end;

Does it modify A contents, or does it modify outside variables, merely reading the A contents? You really don’t know, until I show you the documentation (or declaration in Pascal) of the class of A, and all it’s ancestors.

Compare this with these obvious code snippets:

SourceX := A.X;
SourceY := A.Y;

or

A.SourceX := X;
A.SourceY := Y;

The with also makes the code fragile to any changes of A API.

  • Every time you add/delete a field/property/method in A, then the code inside with A do begin …​ end potentially changed it’s meaning. It may compile, but suddenly will do something completely different, because the symbols inside the with clause will be bound to a different namespace.

  • This is in contrast to all the other code (outside with) using A. If you remove some field from A, then you have a nice guarantee that all the code accessing A.NonExistingField will no longer compile. This is great when you need to break A API, this way you can remove / change something, and let the compiler tell you what needs to be adjusted.

    Using with A …​ takes away some of this advantage. Even if you removed NonExistingField from A, the code with A do NonExistingField := …​ may still compile, if the outer namespace happens to have the NonExistingField symbol.

1.9. Order of the uses clause: standard, CGE, application

The uses clause of our units and examples should follow the order

  • standard units (RTL, LCL, VCL…​)

  • then our own (CastleXxx) units

  • then eventual game-specific units (GameXxx)

Each part should start from a newline.

// THIS IS OK:
uses SysUtils, Classes,
  CastleUtils, CastleViewport,
  GameViewPlay;
Note
We don’t have a strict rule for the order of units within each group, e.g. we don’t really care in which order you specify CGE units — CastleUtils, CastleViewport or CastleViewport, CastleUtils are both OK. The order often loosely follows the dependency order (if unit A is used by B, then we write A, B, like CastleUtils, CastleViewport or SysUtils, Classes, Controls) but we really don’t make this a strict rule (it would be too tiresome to maintain, for no gain). Many uses clauses have in practice an order dictated by history and that’s OK. We only require strict separation between these 3 groups (standard, engine, application).

1.10. Use strict private where possible

Use strict private whenever you can, that is: use it for private stuff that is not accessed by other classes/routines in the same unit.

Use just private for stuff that is accessed by other classes/routines in the same unit. Basically, use private where you would use friends in C++.

This improves code readability in case of large units, that feature more than just a single class. And we have many such large units.

1.11. Do not use strict protected

Using strict protected is not advised in CGE.

Reason: The distinction between strict protected and protected is not very useful for readability (regardless if something is strict protected or just protected, you must think "something outside of this class, even outside of this unit, may access it"). Moreover, it is forced downward, on all descendants of this class (that must then differentiate between overriding in strict protected vs protected, which is uncomfortable because the decision whether to use strict protected or protected should be an internal (implementation) decision within the ancestor, not affecting the descendants).

So, use just one protected section, do not bother splitting it into strict protected and protected.

1.12. Write abbreviations inside Pascal identifiers as regular words, without capitalizing all letters (e.g. GetJson, SetUrl, MakeHttpQuery)

Use Json, Url, Http etc. in Pascal identifiers. Not JSON, URL, HTTP.

Reason: It looks more readable in long identifiers, e.g. GetHttpResponse is more readable than GetHTTPResponse.

Note that this rule applies only to the Pascal identifiers. The Pascal identifiers are CamelCase, and so they warrant writing abbreviations inside like words, Json, Url, Http. Normal English text (e.g. in comments and error messages) should follow English conventions of writing the whole abbreviation uppercase.

  • E.g. documentation for GetHttpResponse could say { Returns a response received over HTTP. }.

  • E.g. GetHttpResponse could raise exception like this: raise Exception.Create('Error creating HTTP response').

1.12.1. New content (identifiers, comments) should talk about URL, not URI

Existing content (identifiers, documentation) in some places uses the term URI, in some URL.

To make things simple, all future code should just talk about URLs (not URIs), and our documentation about URLs just clearly says "most URLs actually accept a URI (a bit broader term than URL); in particular you can use data URI scheme".

Background:

The difference between URL and URI is explained in terminology section of the networking manual (URI is a more general concept than URL) but admittedly the difference is small and we didn’t pay strict attention to this difference when naming some things, like TCastleSceneCore.Url. In effect, in some places we used the term URL where we should have used URI or vice-versa.

X3D standard also mixes them a bit, in effect TInlineNode.SetUrl actually accepts various URIs (including data URI scheme).

The glTF standard was more careful to talk about URIs not URLs. FPC URIParser unit is also named more correctly.

However, users are simply more familiar with the term URL (and not URI), so we decided to just use URL everywhere.

1.13. Indenting inside classes

type
  TMyClass = class
  private
    MyField: Integer;
    procedure Foo;
  public
    MyPublicField: Integer;
    procedure Bar;
  end;

If you use the nested types / constants, indent the fields inside the var block as well. See the example below, notice that MyField is now indented more than in the example above. Admittedly it’s not a perfect convention — MyField indentation is now inconsistent with MyPublicField. But on the other hand, MyField indentation is consistent with MyNestedConst and TMyNestedClass and how you usually indent var block.

type
  TMyClass = class
  private
    type
      TMyNestedClass = class
      end;
    const
      MyNestedConst = 123;
    var
      MyField: Integer;
    procedure Foo;
  public
    MyPublicField: Integer;
    procedure Bar;
  end;

1.14. File extensions

  • *.pas files are units,

  • *.inc are files to be included in other Pascal source files using $I (short for $Include).

  • *.dpr and *.lpr are Pascal program files. We advise using .dpr extension, unless it’s an application that will really only ever work with Lazarus (and not Delphi) e.g. because it relies on LCL.

    Reason: Lazarus accepts either .dpr or .lpr as extensions for the main program file. But Delphi tolerates only .dpr extension. And almost all CGE programs compile (or will compile) with both Delphi and Lazarus. So, like it or not, we have adjusted to Delphi, and just use .dpr.

1.15. Write reentrant routines

All the engine functions are "reentrant", which means that they are safe to be called recursively, even through your own callbacks. E.g. the TFileProc callback passed to FindFiles can call FindFiles again inside it’s own implementation, causing a recursion.

1.16. Some naming conventions

  • If some procedure modifies it’s 1st parameter then I usually end it’s name with "Var" ("to variable").

    Often you will be able to see the same operation coming in two flavors:

    function DoSomething(const X: SOME-TYPE, ...): SOME-TYPE;
    procedure DoSomethingVar(var X: SOME-TYPE,...);

    The 1st (functional-like) version is more flexible, but the 2nd version may be faster (especially if SOME-TYPE is large, or requires time-consuming initialization).

    See e.g. CastleVectors and CastleImages units.

    This rule doesn’t apply when SOME-TYPE is some class instance. It’s normal that a procedure may modify the given class instance contents, no need to signify this with a "Var" suffix.

  • The term "stride" refers to a distance in bytes between memory chunks, following OpenGL conventions.

    If somewhere I use parameters like V: ^SOME-TYPE and Stride: Integer then it means that these parameters define a table of SOME-TYPE values. Address of 1st item is V, address of i-th is (V + i * Stride).

    Stride may be negative. Stride may also be 0, then it means that Stride = SizeOf(SOME-TYPE).

  • Do not use Minimal or Maximal. Use Min and Max.

1.17. Compilation symbols

We use standard FPC and Delphi compilation symbols like:

  • MSWINDOWS (do not use WINDOWS which isn’t defined by all compilers),

  • LINUX,

  • UNIX (remember this is broader than Linux, it includes also FreeBSD, macOS, Android, iOS…​ pretty much everything except Windows),

  • CPUI386,

  • CPUX86_64,

  • FPC

to differentiate between compiler versions, and some more.

We detect Delphi by just lack of FPC, e.g. property Foo: Integer …​; {$ifdef FPC}deprecated 'Delphi does not support deprecated clause for properties';{$endif}.

See castleconf.inc (included in every CGE unit).

We also use DEBUG symbol. The build tool, when compiling in debug mode (--mode=debug), defines the DEBUG symbol, and adds some runtime checks, like range checking and overflow checking. You can use {$ifdef DEBUG} in your own code to add additional things. There’s also the RELEASE symbol, but usually we don’t check for it’s existence — if DEBUG then we’re in debug mode, else we’re in release mode.

1.18. Exceptions' messages

This is good:

raise Exception.Create('Invalid parameter');

raise Exception.CreateFmt('Foo must be > 0 but is %d', [
  Foo
]);

raise Exception.CreateFmt('%s:%s: Cannot draw an uninitialized component', [
  Foo.Name,
  Foo.ClassName
]);

raise Exception.CreateFmt('Invalid 3D model file %s', [
  URIDisplay(Url)
]);

raise Exception.CreateFmt('Too many seconds: %f', [
  TimeInSeconds
]);

Guidelines:

  • Do not start the message with 'Error: ' or anything else that just means "we have an error". This is redundant, since all exceptions signal some error.

  • Don’t end the Message with '!' character. Do not cause panic :) The exception message must look normal when presented to the end-user. If something should not occur (and signals a bug) then use EInternalError exception class to mark this.

  • If the exception is related to some object instance, include useful information to identify this instance. If you have a TComponent descendant, then both Name and ClassName contain useful information to show. (see above example)

  • If the exception is related to some file / URL, include this URL using URIDisplay(Url).

  • Usually, Message should be a single sentence, and not end with the '.' character. But we do not follow this rule 100%, it’s OK to break it for good reasons — sometimes a multi-line sentence message is useful. The last sentence in this case should not the dot.

  • Message should not contain any line-breaks. Reason: this doesn’t look good when displayed in some situations. Especially when one Message is embedded as part of the Message of other exception.

    We do not follow this rule 100%, it’s OK to break it with good reasons. We know that some information really looks much cleaner when split into multiple lines (e.g. TMatrix4.ToString output is multi-line already).

  • Message should not contain any general program information like ApplicationName, ExeName, ApplicationProperties.Version etc. (The exception to this rule is when such information is really related to the error that happened, may help to explain this error etc.) The code that finally catches and outputs the exception should make such information available.

1.19. Prefer to make callbacks of object

ObjectPascal is a hybrid OOP language and it has global function pointers and method pointers. They are incompatible, since the method pointer is actually two pointers (the class instance, and the code address). When designing a function that takes a callback, you’re faced with a problem: define "a pointer to a method" or "a pointer to a global function/procedure"?

In the past, I often chose to use "a pointer to a global function/procedure". With a generic "Data: Pointer" parameter, to allow passing user data. This is easier to use when you don’t have a class instance (and you don’t want to create a dummy class just for this), and it’s always allows to add overridden version with "of object" callback (passing object instance as the Data);

Nowadays, I usually define "of object" callbacks, assuming that all non-trivial code is usually in some class, and the "of object" is more natural to be used in OOP.

1.20. Order of methods

Place the implementation of constructors (Create*) first, then destructor (Destroy), and then the rest of methods. I do not have a precise rule about the ordering of the rest of methods — I usually like to group related methods together.

1.21. Remember that StrToFloat and friends are locale-dependent. Almost always use StrToFloatDot instead.

Standard StrToFloat in FPC and Delphi converts floats to/from Strings using locale-dependent DecimalSeparator value. On some systems (e.g. on Polish Windows) it is equal to comma (,), not a dot (.). This is usually not what you want: when you read/write files, or command-line arguments, you usually want to have "dot" as the only decimal separator, so that your application works regardless of user’s system locale.

So instead use StrToFloatDot. As a bonus, it is also marginally faster.

Same advise applies for related functions:

  • Use StrToFloatDefDot instead of StrToFloatDef

  • Use TryStrToFloatDot instead of TryStrToFloat

  • Use FormatDot instead of Format

  • Use FloatToStrDot instead of FloatToStr

1.22. Do not use LongInt / LongWord. Use Int32 / UInt32, Int64 / UInt64, Integer / Cardinal.

Embarcadero decided to make things weird: https://docwiki.embarcadero.com/RADStudio/Sydney/en/Simple_Types_(Delphi). The LongInt / LongWord are

  • 32-bit on 32-bit platforms, and on 64-bit Windows.

  • They are 64-bit on 64-bit OSes that are not Windows (like Linux, Android, iOS).

This is

  • Completely weird. Why did you make it so inconsistent across platforms? You should have either made it architecture-dependent (64-bit on all 64-bit platforms), or architecture-independent (32-bit on all platforms regardless of their CPU architecture). Making an exception specifically for 64-bit Windows is a really bad idea, it invites bugs when writing code that is supposed to be cross-platform.

  • Contrary to older Pascal documentation statements, that suggested that LongInt / LongWord have 32-bit always. It was the Integer / Cardinal that were supposed to be (maybe) system-dependent! (though they remain in practice 32-bit always, in both FPC and Delphi.)

  • Incompatible with FPC.

So just don’t use these types in CGE code.

  • Use Int32 / UInt32 when you want to have integers of guaranteed 32-bit size. The names are consistent with Int64 / UInt64.

  • Use Int64 / UInt64 when you want to have integers of guaranteed 64-bit size. The QWord (FPC name for UInt64) is also good.

  • Use Integer / Cardinal when you don’t care much about the bit size. In practice they are always 32-bit on all platforms (with both FPC / Lazarus), although long time ago they were supposed to be platform-dependent.

  • Use TListSize for counts and capacities of lists. (It is signed, to not cause overflows with frequent constructions like for I := 0 to List.Count - 1 do...)

  • Use PtrInt / PtrUInt when you want to have integers of guaranteed pointer-size.

1.23. Do not use Extended. Use Single / Double.

Traditionally, Extended used to be a 10-byte floating-point type, available in old Pascal compilers like Turbo Pascal. But modern systems don’t support such 10-byte floating-point type natively, so both FPC and Delphi actually make it an alias to some other type (usually Double, standard 8-byte floating-point type) on most (but not all) platforms.

The exact rules that define "what exactly is Extended" depend on the compiler. Basically, both FPC and Delphi try to alias it to the best (most precise) floating-point type that can still be efficiently implemented on given OS/CPU combination.

The rules are:

  • FPC: docs say that Extended is alias for Double for most of non-i386 architectures.

    One known exception to the above is Linux on x86-64, that still allows to use traditional 10-byte Extended, despite not being i386.

  • Delphi: See https://docwiki.embarcadero.com/RADStudio/Sydney/en/Simple_Types_(Delphi) . Similar to FPC, Extended is just Double (8 bytes) on most platforms.

    Delphi makes exception for Win32, where Extended is traditional 10-byte type.

    Moreover, Delphi defines Extended to be a new 16-byte floating-point type on some platforms:

    • 64-bit Intel Linux

    • 32-bit Intel macOS

    • 32-bit Intel iOS Simulator

The end result makes Extended not very useful for general cross-platform (and cross-compiler) code, due to this uncertainty. You cannot rely that Extended has 10-byte precision, and if you use Extended a subtle bugs may creep in due to not testing on all possible Extended situations. And GPUs don’t support anything above Double anyway.

So we recommend you just use Single for most of code, and eventually fallback on Double if you must have great precision (which usually means you proved that some calculation goes badly with Single and switching it to Double is a reasonable fix).

For CGE developers: While the above recommendation holds (do not use Extended), if you really have to use it, then the castleconf.inc defines:

  • EXTENDED_EQUALS_DOUBLE symbol when Extended is 8-byte Double.

  • EXTENDED_OVERLOADS symbol when you should define Extended overloads. This means that EXTENDED_EQUALS_DOUBLE is not defined, or that compiler wants such overloads anyway (Delphi allows and actually wants such overloads, because even when Extended is 8-byte Double, it is still a distinct type from Double).

  • EXTENDED_EQUALS_LONG_DOUBLE symbol when Extended is 16-byte "long double".

1.24. Most code should use just String, and be prepared that it is 8-bit on FPC and 16-bit on Delphi. Only if writing to stream, explicitly use 8-bit AnsiString (in usual case, when you write UTF-8).

On FPC, we follow the same approach to String as in Lazarus: String is an alias to AnsiString, and it should always contain UTF-8 data. We use necessary compiler switches to make String = AnsiString, and the CastleUtils has necessary initialization to make sure that strings can just carry UTF-8 on all platforms.

See FPC docs:

On Delphi, we follow the standard approach of modern Delphi: String is an alias to UnicodeString, and it contains UTF-16 encoded data.

See Delphi docs:

Correspondingly, Char is 8-bit with FPC, and 16-bit with Delphi. And PChar points to 8-bit characters on FPC, 16-bit on Delphi.

With both compilers, you can explicitly use AnsiString to request 8-bit string. And AnsiChar for 8-bit character, PAnsiChar to have a pointer to them.

What to do?

  • In most CGE code, just use String and Char and most of the time it just does what you want. You can often ignore the fact that FPC will do this using 8-bit chars and Delphi will do this using 16-bit chars.

  • Exceptions:

    • When we read/write to streams, like using various CastleClassUtils routines, we use 8-bit strings. Since UTF-8 is the file format that most software expects, it is a superset of ASCII (that is: simplest text files) etc. So CastleClassUtils routines dealing with streams + strings just declare AnsiString as input/output type.

      There are exceptions marked with DefaultString in the name, right now only MemoryStreamLoadFromDefaultString. This routine writes 8-bit on FPC, and 16-bit on Delphi.

    • When interacting with external libraries, you will most often use PAnsiChar (not PChar) as most of them expect 8-bit UTF-8 (or just ASCII) text.

Our unit CastleUnicode contains various helpful things to deal with strings in a way that it Unicode-safe and works with both FPC and Delphi painlessly. In particular, use:

TCastleStringIterator

Iterate over strings, to get all Unicode character codes in sequence.

StringCopy

Get a substring of a string, with positions and count and whole logic accounting for multi-byte characters.

StringLength

Count of Unicode characters in a string, accounting for multi-byte characters.

1.25. Use BeforeDestruction for finalization tasks that keep the object state consistent

While most of the finalization code should usually go to the destructor (Destroy), it is OK (and sometimes even desirable) to override BeforeDestruction and put there code that finalizes things, but keeps the object state consistent. The idea is that finalization can be split into 2 phases:

  1. BeforeDestruction : Do only things that keep the object state consistent. This means that 100% of class implementation is prepared to handle the state in the middle (and after) BeforeDestruction happens. Examples of things to do here:

    • Detach things, e.g. calling BeforeParentDetach and clearing the behaviors from parent TCastleTransform.

    • Calling notifications.

  2. Destroy : Do things that put the object in half-uninitialized state. Not all of class implementation has to be prepared to handle this state.

    • For example, here you can free (always using FreeAndNil) a child object owned by this instance.

    • This is also when TComponent free notification mechanism (provoked by FreeNotification, TFreeNotificationObserver) happens. This happens in TComponent.Destroy (at least in case of FPC).

100% of class implementation must be ready to work during BeforeDestruction. So

  • Do not just blindly move parts of finalization from Destroy to BeforeDestruction. It would be a mistake to fix things this way, as at some point we would then need BeforeBeforeDestruction and so on.

  • Things that can happen from BeforeDestruction can safely still call methods/properties of the class that is being destroyed. This is important, because detaching things often causes some callbacks, some virtual method calls — and in general you cannot control what the code there will do.

As a bonus, implementing BeforeDestruction is sometimes easier than implementing Destroy: Because BeforeDestruction is only called if the class constructor reached the end, i.e. fully initialized everything (see Delphi docs of TObject.BeforeDestruction). This is in contrast to implementation of Destroy, that has to be very careful, because destructor can be called even on half-initialized object state (because destructor is called even when the constructor fails with an exception).

Once the actual Destroy starts, the object may be in half initialized state and in general it is not guaranteed that operating on such object works. The object may have some critical fields cleared (uninitialized), and in general methods/properties may assume they are initialized. E.g.

  • Create does Something := TMySomething.Create

  • Destroy does FreeAndNil(Something)

  • Nothing else can free/change Something instance

  • → This means that most of the implementation may assume that Something is not nil. There is no need to write if Something <> nil then…​ before accessing Something methods/properties.

  • Only a subset of class implementation, that is known it can be reached from Destroy calls (directly or indirectly) has to be prepared to handle Something = nil case.

We often make implementation of some class "robust" to work even when this class is in the middle of Destroy. But there is no guarantee about it and we do it on a case-by-case basis (when it is needed by something). In general, class implementation may assume that it is never called in "half initialized" state, and that user code checks if not (csDestroying in SomeObject.ComponentState) then…​ if it could happen when SomeObject is in the middle of Destroy operation.

1.26. Blank lines

Note
This is a guideline, not a strict rule. There are many cases where we added (or didn’t add) blank lines on a case-by-case basis, because "here it looked better". Still, do follow this guideline if there are no good reasons to deviate from it.

In the interface, put a blank line before a comment in

  • protected,

  • (not advised) strict protected,

  • public,

  • published

sections. Unless that comment is preceded by the line with section keyword (like public).

Putting a blank line before a comment in a private or strict private section is allowed but not required. Often you want to make these private sections more terse (also there is no need to strictly document everything private) so it’s OK to not use blank lines.

Do not add blank lines before the line with section keyword (like public). Do not add blank lines before every undocumented (private) method / property. E.g. this is good:

type
  TMyClass = class
  private
    Foo: Integer;
    Bar: String;
    procedure DoSomething;
    { This is something else. }
    procedure DoSomethingElse;
    procedure SetSuperProperty(const Value: Integer);
  public
    { Incredibly useful operation. }
    procedure DoSomethingIncrediblyUseful;

    { Another incredibly useful operation. }
    procedure AnotherIncrediblyUseful;

    { Super important. }
    property SuperProperty: Integer read FSuperProperty write SetSuperProperty;
  published
    { Published super important property. }
    property PublishedSuperProperty: Integer ...;

    { Another published super important property. }
    property AnotherPublishedSuperProperty: Integer ...;
  end;

This is not good:

// DON'T WRITE LIKE THIS
type
  TMyClass = class
  private

    Foo: Integer;

    Bar: String;

    procedure DoSomething;

    { This is something else. }
    procedure DoSomethingElse;

    procedure SetSuperProperty(const Value: Integer);
  protected

    procedure Something;

    procedure Something;

  public

    { Incredibly useful operation. }
    procedure DoSomethingIncrediblyUseful;
    { Another incredibly useful operation. }
    procedure AnotherIncrediblyUseful;
    { Super important. }
    property SuperProperty: Integer ...;

  { Another published super important property. }
  published property PublishedSuperProperty: Integer ...;
  end;

1.27. Start a property setter with if FXxx <> Value then

A typical setter implementation should check whether the new value is different than the previous, and early abort (not do anything) when they are equal.

Reason: This is an easy and useful optimization. The cost of checking for equality is usually trivial and it can save non-trivial operation cost. Do this even if you suspect (or know) that layer below is also doing such checks. Don’t trust the authors of "layer below" (esp. if it’s an external library).

Implementing setters like this from the start is much easier than latter adding such conditions. If your setter doesn’t make the check if Xxx <> Value then, then later developers will be afraid of adding it, fearing that it breaks something (by not re-initializing something in some cases). So we generally require to implement setters like this from the start.

Our convention is to do it by if FXxx <> Value then followed by the FXxx := Value; assignment (with the FXxx always on the left side, so it’s easy to see that comparison and assignment work on the same identifiers). Like this:

procedure TMyClass.SetFoo(const Value: Integer);
begin
  if FFoo <> Value then
  begin
    FFoo := Value;
    // do some side-effect
    SynchronizedObject.Foo := Value;
    Invalidate;
  end;
end;
Note
Compare vectors like if not TVector3.PerfectlyEquals(FFoo, Value) then…​.
Note
The condition to perform setter should check for exact equality (so check floats using <> not SameValue, check vectors using PerfectlyEquals not Equals). Reason: Right after performing setter, we want the property value to be exactly equal to what was set. This is more natural for user (following general rule "properties should behave like fields when reasonable"). This also avoids hard-to-find bugs like "increasing the property 1000 times by epsilon/2 has no effect".

Lazarus CodeTools generate a setter with early exit using Exit instead, we discourage this setter form (not because it’s inherently worse, but just the above is also good and it is prevalent in CGE sources, so better follow existing convention):

// DON'T WRITE LIKE THIS
procedure TMyClass.SetFoo(const Value: Integer);
begin
  if FFoo = Value then
    Exit;

  FFoo := Value;
  // do some side-effect
  SynchronizedObject.Foo := Value;
  Invalidate;
end;

1.28. Document at least every protected / public / published identifier, use a language that looks good in English and uses PasDoc tags

All protected, public, published identifiers should be documented (unless you really don’t have anything to add that isn’t already spelled out by the identifier name).

Documenting the private and strict private identifiers is also encouraged if there’s anything that isn’t obvious. We don’t strictly require documenting them — as their meaning / purpose is often obvious enough from the name, and the details can be inferred by analyzing the implementation code. But if you’re unsure, it is better to write docs.

Do not make a documentation that just repeats what the identifier already says. If you define property RotationAxis: TVector3 …​ then documenting it like this isn’t very helpful:

// DON'T WRITE LIKE THIS
{ The axis of rotation. }
property RotationAxis: TVector3 ...;

If this is really everything you have to say, then you can skip the documentation in such case — there’s no point in writing exactly the same words that the identifier is already composed of.

But there’s almost always something more that you can add that is useful.

  • You can usually summarize connections of the identifier to other identifiers or to the core function of the class.

  • If the property doesn’t contain a default clause (e.g. records, like TVector3, cannot have such clause) then say in the documentation what is the default value (that the constructor sets).

  • You can often mention as @seealso closely related things.

  • If there are any limitations in how something works, describe them.

  • If the property value is ignored in some circumstances, it’s useful to mention it.

This is a better example:

{ Object rotates around the given axis with the speed of @link(RotationSpeed).
  This property is ignored if @link(Rotating) is @false.
  Default value is +Y (0,1,0).
  @seealso RotationSpeed
  @seealso Rotating }
property RotationAxis: TVector3 ...;

As for English:

  • Make it correct English, but also make it terse and precise.

  • Do not waste words on useless prefixes like "This property defines rotation axis." or "This method starts the rotation.". Instead be shorter: "Rotation axis." (for a property) or "Start rotating." (for a method).

  • Especially make the 1st sentence really good.

    • We use PasDoc with --auto-abstract, so the 1st sentence is used in various places as a good summary of this identifier.

    • It also means that the 1st sentence should "stand on its own".

    • Be aware that first sentence is detected by looking for the first dot followed by a space character. Avoid using mid-sentence abbreviations that would confuse this mechanism, like i.e., etc., e.g..

      For example this is not good: Rotation axis, i.e. the axis around which it rotates. Here, PasDoc would accidentally think that 1st sentence is just Rotation axis, i.e..

      To avoid this trap, rephrase the sentence, or don’t use abbreviation (Rotation axis, that is: the axis around which it rotates.). Or use explicit PasDoc @abstract tag.

  • We use PasDoc with --auto-link, so identifiers are converted into links automatically, without the need to use PasDoc @link tag.

    Advantage: the comments look more like regular English.

    Disadvantage: if you make a typo in related identifier name, or the related identifier will be renamed — we will not warn about it.

    When the risk of latter is significant, we often still write @link(SomethingRelated) explicitly, instead of just SomethingRelated. So, there’s no strict rule whether to write or not to write @link(…​) explicitly, follow your heart :)

1.29. Do not add useless comment { TClassName }

Lazarus IDE, when editing, adds automatically a bit useless comment with just a class name, { TClassName }, before each class. They are not useful (a person reading API documentation generated by PasDoc, or reading source code, will see the class name anyway nearby). And these comments obscure the good documentation string for PasDoc. (See https://github.com/pasdoc/pasdoc/issues/149 for discussion about it too.)

I recommend to disable this ("Header comment for class") in Lazarus settings, see the screenshot in https://github.com/pasdoc/pasdoc/issues/149#issuecomment-1308837496 .

1.30. Use the same OS and CPU names as FPC (e.g. x86_64, aarch64, i386)

In Castle Game Engine we use the same names for OSes (operating systems) and CPUs (processors) as FPC. FPC in turn follows names used by GNU tools.

Reasons, in short:

  • This makes all the tools use consistent naming.

  • This avoids unnecessary name conversions in the codebase. Conversions that would spread to tools, packing scripts, documentation…​ lots of additional error-prone work, so we don’t want that.

We acknowledge that some of FPC (and GNU) names for OS / CPU are not "optimal". Users sometimes prefer (are accustomed to) a different name. Sometimes FPC naming is justified by current technical reasons, sometimes it is only historical. So in the documentation / UI / website, you are welcome to write longer names (e.g. instead of "Darwin" write "macOS (Darwin)" ).

  • But be sure to also show the "core" name (used by CGE tools, FPC etc.) to make the name recognizable for users.

  • In our experience, inventing and using only our own names for OS / CPU would really only add more confusion.

    We are then doomed to have support questions like "why does lazbuild --os=xxx --cpu=yyy behave differently than castle-engine compile --os=xxx --cpu=yyy". We want to avoid such questions :)

  • Moreover, sometimes the "user-friendly names" would backfire. For example, Windows 64-bit was an unambiguous name for many years (it was obvious that 64-bit implies the x86_64 architecture, dominant architecture on computers where Windows is used). But now Windows on Aarch64 is coming. Thankfully, internally, FPC and GNU always called them with clearly separate names: Win64 on x86_64 and Win64 on Aarch64.

    Warning
    Delphi also stepped into his mistake. They use ambiguous names like Windows 64 or Linux 64. Sooner or later they will have to deal with the fact that OSes supports various 64-bit CPU architectures, not only x86_64. Don’t follow Delphi OS / CPU naming, it just doesn’t scale.

Examples:

  • Darwin is used as OS name for what is more commonly known as macOS. This is correct (name of macOS operating system is Darwin, Wikipedia explicitly says so). Though we acknowledge that many users are not aware of the difference or are just accustomed to call it macOS.

  • Win32 and Win64 are used a OS names. Using just Windows would be more logical as OS name, we fully agree. But for historical reasons, Win32 and Win64 remain to be used.

  • i386 is the common 32-bit CPU architecture on PCs. It is often also called x86, which may even be more correct.

  • x86_64 is the common 64-bit CPU architecture on PCs. It is sometimes (e.g. by Debian) called also amd64.

  • Aarch64 is the CPU name for common mobile (but also coming to desktops - on Raspberry Pi, Pine64 products, Apple Silicon Macs) 64-bit CPU architecture. It is sometimes called arm64, though this is not strictly correct.

  • Avoid naming CPUs as just "32-bit" or "64-bit".

    Unless by 64-bit CPUs you really mean all CPUs with 64-bit pointer size, so both x86_64 and Aarch64 (and maybe others).

    But if you really mean just x86_64, then say x86_64, not 64-bit architecture or such.

    This is esp. important as Aarch64 architecture is going to be more and more popular among the desktop systems. It is already used by Raspberry Pi, Pine64 products, Apple Silicon Macs, new Windows versions. So all popular desktop OSes (Linux, macOS, Windows) have now 2 relevant 64-bit architectures: x86_64 and aarch64. Make sure to be precise in your communication about this.

1.31. Dealing with FPC 3.3.1 case analysis and avoid new warnings

FPC 3.3.1 performs "case analysis": whether all possible input values are accounted for in the case clauses. It means that:

  • It will emit a warning if not all possible values are handled. For example for this code:

    type
      TOptions = (Option1, Option2, Option3, Option4);
    begin
      case OptionValue of
        case Option1: ...;
        case Option2: ...;
      end;
    end;

    Though Pascal doesn’t require to handle all possible values, and it’s defined what happens when you don’t handle some values (it’s just ignored). But still we want to avoid FPC warnings, and actually use this analysis for our benefit.

    For older FPC versions (and Delphi), we used raise EInternalError.Create to catch the cases when case didn’t handle all inputs but should. But this only makes an error at run-time. So we recognize that new FPC 3.3.1 mechanism is beneficial, making these warnings at compile-time.

  • It will emit a warning if else code is unreachable (because all possible values are handled). For example for this code:

    type
      TOptions = (Option1, Option2);
    begin
      case OptionValue of
        case Option1: ...;
        case Option2: ...;
        else Something; // warning: unreachable code
      end;
    end;

There are advantages of this analysis, though it’s also another complication when writing code — esp. because we have to account also for compilers that don’t do this analysis (like older FPC and Delphi). Our castleconf.inc defines COMPILER_CASE_ANALYSIS for new FPC.

So how to write your case?

1.31.1. If your case should exhaust all the options

type
  TOptions = (Option1, Option2);
begin
  case OptionValue of
    case Option1: ...;
    case Option2: ...;
    {$ifndef COMPILER_CASE_ANALYSIS}
    else raise EInternalError.Create('OptionValue: unhandled value');
    {$endif}
  end;
end;

This way

  • Compiler with COMPILER_CASE_ANALYSIS will not make a warning at compilation "unreachable code". And it will make a warning at compilation if you forgot to account for all OptionsValue values.

  • Compilers without COMPILER_CASE_ANALYSIS will make a run-time error if you forgot to account for all OptionsValue values.

1.31.2. If your case knowingly does not exhausts all options

type
  TOptions = (Option1, Option2, Option3, Option4);
begin
  case OptionValue of
    case Option1: ...;
    case Option2: ...;
    // For compilers without COMPILER_CASE_ANALYSIS, you could also not write anything.
    // But to account compilers with COMPILER_CASE_ANALYSIS, you must write "else ;"
    else ;
  end;
end;

1.31.3. If you know you exhausts all options at run-time, but compiler doesn’t know it at compile-time

type
  TOptions = (Option1, Option2, Option3, Option4);
begin
  if OptionValue in [Option1, Option2] then
    case OptionValue of
      case Option1: ...;
      case Option2: ...;
      else raise EInternalError.Create('OptionValue: unhandled value');
    end;
end;

This way

  • Compiler with COMPILER_CASE_ANALYSIS will not make a warning because you did not exhaust all OptionsValue values.

  • Compilers without COMPILER_CASE_ANALYSIS - also no warnings.

1.31.4. If you don’t care whether you exhaust all options or not (you want to be prepared for future extensions of enum type), but you don’t want any warnings

For this case, there’s no solution that works for compiler with COMPILER_CASE_ANALYSIS that avoids warnings regardless of whether TOptions has or doesn’t have additional values beyond Option1 and Option2.

type
  TOptions = (Option1, Option2 {, ... you don't want to worry if this type will be extended in future, you know you only need to handle Option1 and Option2 });
begin
  case OptionValue of
    case Option1: ...;
    case Option2: ...;
    // Write "else ;" if *right now* there are enum values beyond Option1 and Option2.
    // Be prepared to adjust it in future if TOptions changes.
    // else ;
  end;
end;
procedure TCastleView.TDesignOwner.Notification(AComponent: TComponent; Operation: TOperation);
begin
  case Operation of
    opInsert: ParentView.SetNameReference(AComponent, AComponent.Name, true);
    opRemove: ParentView.SetNameReference(AComponent, AComponent.Name, false);
    // don't care about other TOperation values
  end;
  // ...
end;

1.32. Large engine units may be split into include files

For some large units, it is reasonable to split them into multiple include files. The structure to do it looks like this:

unit CastleXxx;

interface

uses ... { all units required by all includes, in the interface };

{$define read_interface}
{$I castlexxx_foo.inc}
{$I castlexxx_bar.inc}
{$undef read_interface}

implementation

uses ... { all units required by all includes, in the implementation };

{$define read_implementation}
{$I castlexxx_foo.inc}
{$I castlexxx_bar.inc}
{$undef read_implementation}

// if necessary, dispatch initialization / finalization by calling procedures in include files
initialization
  CallInitializationOfFoo;
finalization
  CallFinalizationOfFoo;
end.

and then in each include we do this

{%MainUnit castlexxx.pas}

{$ifdef read_interface}
type
  TMyClass = class
    procedure MyMethod;
  end;

procedure MyProc;
{$endif read_interface}

{$ifdef read_implementation}
procedure TMyClass.MyMethod;
begin
end;

procedure MyProc;
begin
  ...
end;
{$endif read_implementation}

This is not really our idea — at least FPC source code also uses a similar trick for some units.

The {%MainUnit castlexxx.pas} is a comment for the compiler, but it is useful for the Lazarus CodeTools (used in both Lazarus IDE and also in our VS Code extension) to properly complete code when inside an include file.

When is such organization useful?

  • Splitting unit into multiple include files shows the logical "blocks" of the unit in a clear way.

    For example, CastleControls unit (see source) is mostly a collection of components. Placing each component in a different include file is easier to work with — you look at a small include file, not at a large unit file.

    Note
    As with everything, there are advantages and disadvantages :) If the include files are actually connected to each other, then during editing you may need to use less "Find / Replace In This File" operations and more "Find / Replace In Multiple Files" operations. A good text editor has all these operations, but surely working with multiple files is harder. So we try to limit this technique (splitting into includes) only when it really makes sense, when most of the time your include file contains all your brain needs to care about.

    A good split is often "one class per one include file". But not always. Sometimes we group some related classes into one include file.

  • To express the same argument in other words (thinking "what do we avoid", instead of "what do we gain"): Splitting into include files avoids having very large Pascal .pas files.

    Sometimes, a unit grows into something big (for example a collection of components, like CastleControls; or even interconnected collection of components, CastleTransform). Having all the code in one large Pascal .pas file is uncomfortable — you have a big text file to edit. Large files are less comfortable to work with, for text editors, for version control diffs, but mostly for our brains — because you just have to think about "this big code that you’re editing".

  • It’s not a technique we generally recommend you to do in your own applications, and we actually try to not do this for CGE units — unless they grow really large.

    A rule of thumb is that once a unit grows above 1000 lines, it’s probably good to think should it be split.

    Maybe it should be split into multiple unis? This is cleaner from the language standpoint, it allows the compiler to strictly protect the separation (and we also do this technique sometimes, e.g. our renderer is split into a number of internal units).

    But sometimes the split into multiple units is not comfortable or not possible. E.g. splitting CastleControls or CastleTransform would mean you’d often have super-long uses clauses, because you often use multiple classes from these units. Moreover, sometimes, classes need access to each other private stuff. Moreover, sometimes the split is impossible because it would force circular dependencies in units interfaces (this most often occurs in tightly-coupled container/children relationships, e.g. TCastleApplication and TCastleWindow really need to be in one unit CastleWindow).

    So, in such cases, splitting into include files is a good compromise. It allows you to keep the code in one unit, but still have it split into logical blocks.

2. Other languages

  • Indent by 4 spaces in Java and Objective-C.

  • Never use tabs. (Unless they are inherent to the language, like Makefile).

  • Follow the standard coding conventions for that language.

2.1. bash scripts: use portable shebang

To make our bash scripts portable, use this shebang (first line of a shell script):

#!/usr/bin/env bash

This shebang is more portable than often-used #!/bin/bash — as it works reliably also on FreeBSD and OpenBSD. In total, we want our bash scripts to work on a variety of systems: Linux, FreeBSD, macOS, Windows (with Cygwin or MSYS2) etc.

So never use #!/bin/bash as shebang.

Also never use #!/bin/sh. On most Linux systems, this is not equivalent to bash, instead it points to a shell that is a bit more lightweight but also quite more limited than bash. In CGE, performance of our shell scripts is really not critical all, so we should just use bash and have more maintainable scripts.

2.2. bash scripts: strict mode

Read and follow the Use Bash Strict Mode (except the shebang shown there as example).

The set and IFS configuration described there make writing and maintaining shell scripts really much easier. Essentially bash behaves a bit more like a programming language.

Summing it up with the previous advise, this is what your scripts should start with:

#!/usr/bin/env bash
set -euo pipefail
IFS=$'\n\t'

For historical reasons, some of CGE scripts may use something a little more relaxed:

#!/usr/bin/env bash
set -eu

Usually this is just a historical thing and will be upgraded in time. We should follow Use Bash Strict Mode fully going forward (except shebang, which we want more portable).

2.3. Use PowerShell (not BAT files) for Windows scripting; or, in many cases, use bash (from Cygwin or MSYS2)

For many non-critical tasks, we use bash scripts for Windows too. Both Cygwin and MSYS2 provide bash and many other Unix tools for Windows.

For some critical tasks though, really used by many people (like building CGE build tool), we don’t want to assume that user has Cygwin / MSYS2. In this case, we use PowerShell on Windows.

Note: GitLab CI also uses PowerShell on Windows machines.

3. Guidelines (how to write good code)

3.1. You can (and should) free the things you allocated; you should never free other things, unless explicitly allowed

Note
See Modern Object Pascal Introduction for Programmers if you’re unsure what some of the terminology used in this section means.

Pascal is a language with manual memory management, i.e. you have to remember to free the things you allocate.

It’s often quite easy to manage this, assuming that other code "plays nice":

  1. If you create something in your constructor, then you are responsible for destroying it in your destructor (or when it needs to be recreated for some reason).

    This works OK and assumes that nothing else will free your instance. Otherwise you will get a dangling pointer, i.e. a non-nil reference that is invalid because something else freed the memory it pointed to.

    Some code may allow others to free the things it created. It can avoid having a dangling pointer e.g. by observing the reference using FreeNotification or TFreeNotificationObserver, see custom components. (Or by not storing the reference at all.) But this permission ("others can free this thing I created") has to be explicitly documented as allowed.

  2. You can also use the "ownership" mechanism:

    • Ownership mechanism on TComponent: owner TComponent will make sure that children TComponents are freed.

      Other things can also free the children, this will be handled OK.

    • Ownership mechanism on TObjectList: list will make sure that children are freed (when they are removed from the list, or when list is destroyed).

      Other things cannot free the children, or you will get dangling pointers. That is, TObjectList cannot detect when an item was freed through some other mechanism, e.g. because you did O := List[123]; FreeAndNil(O);.

    • Ownership mechanism on TComponentList: list will make sure that children are freed (when they are removed from the list, or when list is destroyed).

      Other things can also free the children, this will be handled OK. Though be careful: do not free things when we may be in the middle of iteration over the list containing them.

In short, things are easy when nothing else frees the object you were supposed to free. Without explicit allowance, the other code should not free the things you created.

This guideline is thus: When you create an instance of something, then you decide when and how it should be freed. Nothing else should free it, unless explicitly permitted by the documentation.

This includes the cases when you receive an instance of something by accessing some property, function or class function. Do not free it, unless it is explicitly documented that you should free it. Only free things where you have called constructor.

This rule has some additional consequences:

3.1.1. If you receive an instance from some code you don’t control, do not assume anything about the lifetime of this instance; use TFreeNotificationObserver to watch it

The prominent example of this is when you expose a settable property like property FooInstance: TFoo read FFooInstance write SetFooInstance.

The best code should not assume that FooInstance will exist as long as it is set. Developer should be allowed to free it and the class that refers to it should account for it (automatically dropping all references to it, in particular FooInstance should change to nil). This is a consequence of the above guideline: developer can free the things that the developer created!

The solution is to use components (TComponent descendants) with FreeNotification or TFreeNotificationObserver, see custom components.

This allows the developer to do this:

C := TSomeParent.Create(Application);
Foo := TFoo.Create(Application);

C.FooInstance := Foo;
FreeAndNil(Foo)

Assert(C.FooInstance = nil);

3.1.2. Avoid making functions/methods that return new instance of some object and pass the responsibility to free it on the caller

For example this is allowed (if you document it like below) but discouraged:

{ Creates TFoo.
  The caller is responsible for freeing the resulting TFoo instance. }
function MakeSomeInstance: TFoo;
begin
  Result := TFoo.Create;
end;

Instead of creating such methods, it is better to enable the same functionality by advising to use a constructor of the proper type. In the above example, the recommendation should be "use the TFoo.Create explicitly instead of relying on a wrapper MakeSomeInstance ". This makes things more obvious: when developer does F := TFoo.Create, then developer knows (s)he should take care of freeing F.

Of course, this recommendation is harder to achieve in real-life cases that look more complicated than MakeSomeInstance :) You probably had some reason to introduce such function. Maybe the new TFoo instance must be configured in some special way, maybe it needs some special parameter coming from the internal variable of this unit or private field of containing class. The point is to think about remaking it. Maybe it should rather be a new TFoo constructor that takes special parameter, and it can convey the same functionality?

The idea is that, by default, developer should be able to assume that when (s)he does F := MakeSomeInstance, (s)he doesn’t need to care to free F. Because the developer didn’t call constructor explicitly.

If your function returns something that the caller should free, it must be explicitly documented.

3.2. Things (parameters, properties, function results) are not allowed to be nil unless this is explicitly stated by the documentation

Technically, in Pascal a value of an object instance (like X: TMyObject) can always be nil.

But this guideline limits the cases when you have to worry that something is nil, in order to avoid writing lots of checks X <> nil in the code.

Follow this:

  • All the object instances given as routine parameters must be non-nil. (That is, they cannot be nil.) If a routine allows some particular parameter as nil, it should be explicitly documented as allowed.

    This puts the burden on the caller, while the implementation of a routine can assume they are not nil.

    This means that writing code like this is considered OK (there’s no need to check Person <> nil) and you don’t even need to document explicitly "Person cannot be nil!":

    procedure GreetPerson(const Person: TPerson);
    begin
      Writeln('Hello ' + Person.FirstName + ' ' + Person.LastName);
    end;
  • All the functions that return an object instance always return a non-nil value. If a particular function can return nil, it should be explicitly documented as possible.

    This puts the burden on the function implementation, not on the caller.

    This means that writing code like this is considered OK (there’s no need to check GetCurrentUser(Context) <> nil):

    procedure GreetCurrentUser;
    begin
      Writeln('Hello ' + GetCurrentUser(Context).FirstName + ' ' + GetCurrentUser(Context).LastName);
    end;
  • All the properties (of an object instance type) have non-nil value (after the object is created). If a property may be nil, it should be explicitly documented.

    This means that writing code like this is considered OK (there’s no need to check GetCurrentUser <> nil). And you don’t need to care whether GetCurrentUser is a property or a parameter-less method, since the last 2 rules are consistent:

    procedure GreetCurrentUser;
    begin
      Writeln('Hello ' + SomeObject.GetCurrentUser.FirstName + ' ' + SomeObject.GetCurrentUser.LastName);
    end;

3.3. Declarative API (classes with independent properties) is simple to use

In general prefer declarative API (properties) over imperative (methods, esp. with complicated usage scenarios).

The classes that expose the "solution" as a set of properties are simple to use. Exposing a "solution" as a set of methods (that must be called in some specific order for the desired effect) is usually not as simple. Of course this is just a general guideline, I’m sure you know lots of exceptions to this rule! CGE itself is a big OOP library with lots of classes with lots of properties and lots of methods. If something is naturally an "action" ("draw it now!") then it should be a method (Render). But if something is a "state" ("use this color when drawing") then it is a property (property Color: TCastleColor; property Text: String; works better than procedure Draw(const Color: TCastleColor; const AText: String)).

Properties should work independently. Property value should not be "automatically" set by setting another unrelated property (e.g. setting TCastleUserInterface.HeightFraction does not set also TCastleUserInterface.Height) or by doing something (e.g. adding a control does not set its TCastleUserInterface.Height; it represents "the desired height", and programmer should instead read EffectiveHeight to know the resulting height). Of course there are exceptions to the latter — some methods naturally set some properties, but then it should be clear that given method does this, e.g. TCastleViewport.Setup2D sets Viewport.Camera.ProjectionType.

You want getting/setting properties to work naturally, regardless of the order in which it happens (this is nice for the programmer using the API, and necessary for reliable deserialization).

Properties should generally work like you would expect a variable works. E.g. reading a property right after setting it should result in the same value. Setting property multiple times to the same value should have no effect. See the Pascal guidelines on Properties (modern Pascal introduction) : …​it’s a good convention to design properties to behave more-or-less like fields:…​.

Classes with independent properties are simple to use — both from CGE editor (that exposes any published properties of TComponent descendants), and from code (code that sets a few properties is obvious to follow).

3.4. Optimize smartly: profile, optimize where it matters (and not where it doesn’t), think about smarter algorithms and moving CPU work to GPU to get big benefits

If you want to suggest some optimization (of speed, of memory usage) to the engine, especially if it:

  • makes a significant code complication to the existing code,

  • or it adds a significant amount of new code (which is also a code complication)

→ then always first do some tests/thinking whether it’s really worth it.

There are many situations where optimizing is not a good idea, because it will not change the "bottleneck" code (which means that the speed / memory use of something else is so large (in comparison) that it completely "masks" the thing that you optimize, making it irrelevant). In such cases, optimization is actually harmful, because the code quality goes down — the optimized code is usually longer and/or more convoluted.

Note
In the rare cases when the optimized code is also shorter and cleaner, you have a full green light to do it just because the code quality is better.

Bottom line:

  • We want to have less code.

  • We want to have simpler code.

  • Do not optimize just because you have an idea how to make some line of code faster. This thinking often leads to performing many tiny optimizations (and thus reducing code quality) that have no noticeable effect on the execution speed or memory use of real applications. First test/think whether it’s worthwhile to optimize this piece of code.

As you can see, I put more emphasis on thinking about code quality than optimization. That is because I see some of us often making the mistake of not caring about code quality enough, and instead rushing to make an optimization (that lowers code quality for little-to-no gain to the final applications).

Of course, this does not mean we don’t want to optimize. It just means that we require justification for each optimization, the optimization must have a noticeable effect on some real-world use-case. We want the code to be fast and use little memory — there are various ways to achieve this, often using smart algorithms on CPU, and/or thinking about how the CPU cache is used, and/or delivering data in better chunks to GPU. Low-level optimization of some local routine is often not the most effective approach.

There is also a dreaded "death by 1000 cuts" that we want to avoid, which is sometimes caused by missing a number of small optimizations that would have a noticeable effect overall. E.g. that’s why we use "Single" throughout the engine code, not Double or Extended. (except some special code where we have testcases that "Single" precision is not enough). Using "Double" everywhere would have a noticeable negative effect on the speed (yes, I tested it long time ago). But e.g. obsessively avoiding calling Sqrt in the engine…​ proved to be usually a useless optimization, causing various bugs and not achieving any speed gain.

So, there are cases to be made for some low-level optimizations. But don’t fall into the trap of implementing lots of useless low-level optimizations blindly.

3.5. Fix warnings (let the compiler help you write reliable code)

We want the compilation of the engine, and all examples, to pass without any warnings. On all supported platforms and with all supported compilers.

We consider compiler warnings as generally useful, so we don’t want to mindlessly hide all warnings (or get into a habit of ignoring them). We want to obey the warnings. A warning is a TODO — you need to deal with this, maybe by improving the code to be more reliable.

Sometimes the warning isn’t really valid, but you still should deal with it. It is acceptable, in justified cases, to selectively turn off the warning, like this:

{$warnings off} // knowingly calling deprecated method in another deprecated method
X.Foo;
{$warnings on}

We still consider this a "win": while you will occasionally have to deal with useless warnings from the compiler, but on the other hand you will pay attention to compiler warnings and the compiler will help you detect real problems in code. The warnings reported by the compiler are often about "edge-cases", situations that don’t happen often (or maybe even you think they will never happen), but sometimes the compiler is right: that special situation can happen, and the code should account for it.

3.6. Be consistent in API design

Ideally, we add new features to the engine by just "naturally" extending the existing API. By "naturally" I mean that the new concept (class, method, property, whatever)

  • uses similar terminology as existing API concepts (e.g. word "scene" in CGE context refers to TCastleScene, not something else),

  • uses similar conventions (e.g. "Y goes up, even in 2D"),

  • uses existing CGE constructs (e.g. for vectors, use CGE vectors from CastleVectors, like TVector3).

The engine API should be consistent (with itself).

Example: If you ever need 4x3 matrix, make it consistent with existing TMatrix4. Do not declare it like this:

// DON'T DO THIS, THIS MATRIX TYPE IS INCONSISTENT WITH CGE
TNewMatrix4x3 = class
  MyData: array [1..4*3] of Float;
end;

Can you spot all the inconsistent things in this TNewMatrix4x3 declaration above? That would make using TNewMatrix4x3 weird alongside the existing TMatrix4? This would be much better:

TMatrix4x3 = record
public
  const
    ColsCount = 4;
    RowsCount = 3;
  var
    Data: array [0..ColsCount - 1, 0.. RowsCount - 1] of Single;

  // See TMatrix4 declaration for more things to put here:
  // - overload operators,
  // - define Rows, Columns, Transpose, Zero, Identity
end;

3.7. Backward compatibility is important; having a consistent (easy to learn) API and useful features is even more important

Sometimes adding new things without breaking old things, in such way that the end API is consistent, is not possible. We didn’t predict everything when designing CGE API :) In this case, we have to change our API, to incorporate the new feature, such that the resulting engine API feels consistent. We don’t want features to reinvent some concepts, e.g. because some detail in the existing concept was not perfectly suitable. We want to adjust existing concepts to be better.

In this case, when the API of existing concept must change, try hard to keep the old API still available. Use deprecated Pascal directive, use it with a string parameter that explains shortly the advised alternative, like

procedure Foo; deprecated 'use Bar';

It works nicely, will display a visible warning to users that they use the deprecated feature, but at the same time Foo will continue to function normally.

But there are rare cases when providing backward compatibility is not possible. Or maybe it would be possible, but would be a lot of work on engine development side (to make it / maintain it), or the existence of "compatibility crutch" would be too confusing to new users. In this case, after considering all the options, I say it is acceptable to break backward compatibility. You’re doing it for the greater good. The engine must have room to evolve, to be better and better, we must have a way to change old/bad/un-optimal API design into new API design. This is the only way to keep our API consistent and keep adding new features. IOW:

  • We don’t want an engine that, in 10 years, has a poor API (because new features (e.g. VR) are added to it in a weird way) and is hard to pick up by new people.

  • In exchange, we can accept the fact that, over the years, you will sometimes have to adjust your code to keep it compiling with the latest engine version.

Think about how the engine will look in a few years. Think about the perception of new people that learn the engine. If the engine has now 100 users, think that it will have 10x (1000) users in a year. We don’t want the new 900 people to suffer from bad design decisions we make today, and suffer because we could not fix these decisions out of fear of breaking backward compatibility.

Note

I am not trying to say that this reasoning applies equally to all the software. Though I think that it does apply to most of the software, actually.

You need to plan to be perfect in the future, you cannot be stuck in bad design decisions. And (no matter how smart you are) you have inevitably made some bad design decisions (which was not evident when you started, maybe even it was impossible to predict as you didn’t knew all your features, but may become evident as you move along and explore possibilities of this thing you make).

But I certainly recognize that there are cases when it’s easier or harder to break backward compatibility. E.g.

  • designing a file format, like X3D or glTF,

  • designing API of a library that is dynamically linked, and distributed independently of the using applications, like OpenGL. You need to keep ABI compatibility then, which is even stronger than API. Not just language level breakage is not possible, also e.g. calling convention cannot change.

In such cases, it is insanely difficult to break compatibility, because your users (in the wild) do not have automatic mechanisms that would force them to upgrade. Instead, their files / applications would suddenly stop working. There are solutions to this (explicitly giving file format version in each file, like in glTF and X3D; explicitly requesting API version at initialization with flags for forward/backward compatibility behavior, like in OpenGL) but essentially breaking compatibility is hard — you can do it only once per a long time, and you must have convincing arguments that new version is better (so it’s worth forcing the entire ecosystem, like authoring tools, to upgrade).

However, CGE is a library that is statically linked into your project. In the worst case, if we really break compatibility — your game will not compile after you upgrade CGE. But we don’t make it crashing for users. Developer will see that it will not compile, and will have to fix it.

4. Submitting your code contributions

It’s best to use GitHub’s pull requests.

  1. Fork the https://github.com/castle-engine/castle-engine/ . This is done by clicking on the appropriate button on GitHub.

  2. Clone your fork (i.e. download it to your local computer).

  3. Optional: Create a new branch in your fork, just for this specific feature, e.g. doing git checkout -b my-new-feature. This allows to separate your work on various CGE features.

  4. Work on your feature, committing and pushing as usual, to your branch in your fork.

  5. When ready, submit a pull request using https://github.com/castle-engine/castle-engine/pulls

See GitHub documentation (and other sites) for information about pull requests:

Advantages of pull requests:

  • They allow you to comfortably work on your pull request, committing and pushing and showing your changes to anyone. There is no need to ask for any permission to do this. (But, if you want, you can of course let us know about your work, see https://castle-engine.io/talk.php . We may be able to advise on a best way to add something to CGE.)

  • They allow us to use "code review" features of GitHub. This is a comfortable way to comment on your changes.

  • They allow everyone to submit, review and merge the changes relatively easily. And all operations can be done using the command-line or web interface, so it’s comfortable / easy / flexible for everyone.

If for some reason you really cannot follow this workflow, it is OK to simply send a traditional ".patch" file, done by "git diff" or "svn diff" (you can access https://github.com/castle-engine/castle-engine/ as a GIT or SVN repository.) You can attach it to a new issue.

5. Adding an example to CGE repository

Examples in CGE subdirectory examples should follow the snake_case naming (for their subdirectories and for the project name).

If you create a new example using editor "New Project" template, you should edit it:

  • Edit the README.md to describe the purpose of this example.

  • Edit the qualified_name of the example to be like qualified_name="io.castleengine.my.great.project".

  • Insert the standard engine copyright at the top of Pascal files, like code/gameinitialize.pas and code/gameviewmain.pas.

  • Remove the .gitignore from the example subdirectory.

  • Edit the top-level CGE .gitignore to ignore Unix binary of this executable. All other things should be already ignored by existing masks in that .gitignore.

  • Add a screenshot of the running application.

    • See how pretty it looks in eye_of_beholder or occlusion_culling.

    • For now, we don’t have strict rule about whether to include window border and some decorations. I try to make all screenshots using GNOME 3 built-in screenshot tool, that adds a nice drop-shadow around the window border. This looks pretty on a white background when browsing GitHub. But I know that in other contexts, the shadow and window border will be a hindrance for UI, and we will remove them then. For now, don’t worry about it much, any pretty screenshot is OK.

    • For now, we don’t have strict rule about screenshot aspect ratio. I recommend to use a window with aspect ratio around 16:9.

    • Call this file screenshot.png. The name is important in case we will want to show this screenshot in the future as e.g. thumbnail in CGE editor "Open Examples" dialog.

    • (Optional) If you think that additional screenshots are useful, go ahead and add them, with filenames like screenshot_xxx.png. In particular the editor screenshot should be called screenshot_editor.png. See how it looks in occlusion_culling demo. We may do a "carousel" of thumbnails in some UI, we will use then all screenshot*.png images.

    • Show all screenshots from README.md. Add them like this by Markdown:

      ![Screenshot](screenshot.png)
      
      ![Screenshot from editor](screenshot_editor.png)
      
      ![Screenshot running on Android](screenshot_android.png)

6. Adding new unit or path

List of tasks when adding new unit or path to CGE is admittedly not trivial, as we support multiple compilers and multiple ways to build CGE. We try to centralize it, but it’s not always possible without causing headaches for some other cases. So sometimes you have to do a few things. Here we document all the steps:

  • And both new unit and new path to:

    • proper Lazarus package (one of packages/*.lpk)

    • proper Delphi package (one of packages/delphi/*.{dpk,dproj})

    • run check tools/internal/check_packages to make sure it is OK (Jenkins and GH Actions will check it anyway), new top-level path may require being added to check_packages.dpr

    • fpmake.pp

  • New path only:

    • extend EnginePaths in src/castleinternaltools.pas

    • add to castle-fpc.cfg (used when compiling build tool, used also by LSP server)

    • rebuild the build tool (as it uses CastleInternalTools)

    • reinstall in Delphi castle_engine_design package (as it uses CastleInternalTools)

    • add to doc/pasdoc/mk_docs.sh (if the new path contains non-internal units that should be documented)

    • add to check_units_dependencies (if it makes sense to define new rules)

    • rerun ./tools/internal/examples_regenerate_auto_files and commit updated DPROJ

    • rerun castle-engine generate-program --guid-from-name in some applications in CGE that require updated DPROJ, but are not affected by examples/regenerate_auto_files_in_all_examples.sh:

      • examples/delphi/{fmx,fmx_play_animation,vcl}

      • tests/

      • tests/delphi_tests/

      • tools/build_tool/

  • New unit only:

    • add to CastleCacheUnit (optional, only for units used by most applications, and not already pulled by dependencies)


To improve this documentation just edit this page and create a pull request to cge-www repository.