====== Developer guidelines ======


Doomsday Engine is a diverse project that is free and open-source. 
As such, one of many ways to give back to Doomsday is by contributing code.

Be it a bugfix, new feature, or even just cleanup, your work can help make Doomsday the most
technically advanced Doom engine available today.

When working with multiple persons on a project, especially one with a large amount of code,
such as Doomsday or the Linux kernel, it is imperative that there exist guidelines for that code
which is submitted by collaborators such that the project can be approached and understood by all involved
and third parties. 

This document is meant to be a summary of both requirements and best practices for submitting code. It covers,
to the extent deemed necessary various guidelines for submitting code, such as:

  *  Code formatting
  *  Language features
  *  Naming conventions
  *  Version control practices

If you, as a prospective contributor, should have any questions that you feel were unanswered by this document,
you are encouraged to contact a project manager for answers, which may then be added to this document.


===== Code conventions =====


Doomsday is composed of various components, the majority of which are written in both C and C++. 
Doomsday aims to support the C99 and C++11 (C++0x) language sets respectively, with the caveat that the features used must be supported by MSVC 2013. New code //units// (as in files) should be written in C++, as it is currently the intention to migrate towards C++.


===== C++ =====


When writing or modifying C++ for Doomsday, the following guidelines must be adhered to.


==== Indentation ====


Indentation should reflect scope such that code residing in a scope //n// times more restricted should be
indented //n// levels. Spaces must be used instead of tab stops, and one indentation level must be 4 spaces wide.

The following code demonstrates indentation in accordance with the scope:

<code c++>
    class Something : public Thing 
    {
    public:
        // Implementation
            
        Something(string param)
            : Thing(param)
        { 
            // Do things
        }
 
    private:
        // Implementation details            
    };
    
    static void doSomething(bool predicate)
    {
        // Nested type
        // This is ideally at the top of the method. Use your judgement.
        struct NestedStructure
        {
            // Members
        };
        
        { // Nested scope block
            doSomethingOther();
        }
        
        if (predicate)
        {
            doSomething();
        } 
    }
    
    { // Scope block
        int scopedInt = 2;
        doSomethingMore(scopedInt);
    } // New line
</code>

As far as switches go, it must be indented in a readable manner. How this is done is up to the writer.
The following is an example of a well-indented switch.

<code c++>
    switch (predicate)
    {
    case CASE_1:
        doSomething();
        break;
 
    case CASE_SCOPED: { // Scope block
            doSomething();
        }
        break; // Break resides outside the scope block. 
 
    case CASE_FALL_THROUGH_CODE:
        doSomething();
    case CASE_FALL_THROUGH:
    default:
        break;
    }
</code>


==== Brace style ====


Doomsday, for the most part, follows [[http://en.wikipedia.org/wiki/Indent_style#Allman_style|Allman style]] brace
formatting.

Braces should not reside at a level of indentation less than that of the declaration to which they belong.
They must always begin on the line //proceeding// the statement to which they correspond.

Once more, in regards to switches, should you need a scoped case, you may either place the beginning of the
scope block immediately proceeding the case (''case THING: {''), or following the case statement.


==== Flow control ====


You should use a space to separate the predicate following an ''if'', ''while'', ''for'' and the like. For long 
predicates, the predicate may be broken at an appropriate place, and the rest put on the following line. 
Be sure that multiline predicates are aligned sensibly — use your intuition. 


<code c++>
    if (option && option2 && (compound && option)
        || nextLineOption)
    {
        // do something
    }
</code>


=== Return points ===


In regards to other flow control keywords, such as ''break'' or ''continue''
you should avoid nesting in order to create more readable code. If you have excessive return points in a void method,
in the interest of readability you should look at reorganizing to use conditionals, as the compiler will optimize it all the same (in most cases).


<code c++>
    void tooManyReturns()
    {
        // some code up here
        
        if (!thing1) return;
        if (!thing2) return;
        if (!thing3 == VALUE) return;
        
        // more code here
    }
    
    // This can be rewritten as
    
    void conditionalExecution()
    {
        // some code up here
        
        if (thing1 && thin2 && (thing3 == VALUE))
        { 
            // more code here
        }
    }
</code>


=== Predicates ===


Regarding conditional flow control, such as ''if'' and ''while'' avoid using a predicate like this:


<code c++>
    int that = 10;
    
    if (that)
    {
        // code 
    }
    
    // Better rewritten as 
    
    if (that != 0)
    {
        // code
    }
    
    // Or, in some cases `that` should always be positive
    
    if (that > 0)
    {
        // code
    }
</code>

This allows a reader to infer that it is neither a boolean value or a pointer, but rather a number or something which
overrides one of the pictured operators.

Regarding assigment or field declaration in predicates, this practice should be avoided wherever possible. 
If the need for this is apparent, then it should be strictly for a field declared in the predicate. For instance,
the following code demonstrates proper assignment in a predicate:

<code c++>
    if (Thing *thingPointer = getThingPointer())
    {
        // do things with thingPointer
    }
</code>

Whereas the following code demonstrates cases where this behavior is either unecessary, or inappropriate.

<code c++>
    // Innapropriate use: 
    
    Thing *thingPointer = getThingPointer();
    // do stuff with thingPointer
    if (thingPointer = getAnotherThingPointer())
    {
        // Don't do this!
        // Erroneous field reuse should be avoided at all costs
    }
    
    // do more with thingPointer 
    // Don't do the above either, 
    // If a field is needed outside the scope of the
    // conditional expressions branch(es), then it should
    // not be modified within the expressions predicate 
    
    // Unecessary use 
    Thing *thingPointer = getThingPointer();
    if (Foo *fooPointer = doBar()) 
    {
        // use thing pointer in here
        // use foo pointer in here 
    }    
    
    // don't use thingPointer past this point
</code>

In the former of the above two situations, placing both declarations in a scope block 
along with the if statement would be preferrable.

In summary, the predicate should never modify an existing field, and should only declare one if it 
is syntactically clear and leads to more idiomatic code. Otherwise, alternative approaches should be
preferred, especially if they do not pollute the parent scope.


=== goto ===


''goto'' should be avoided at all costs, as it is both hard to debug and may cause unexpected
behavior. If you encounter a method using ''goto'', see if it can be refactored out. 
The same applies for other such similar operators.

<note>Old code in the game plugins sometimes uses ''goto''. When working with such code, be wary of accidentally altering the behavior since vanilla gameplay may rely on subtle edge cases and other such details.</note>


==== Preprocessor ====


As far as preprocessor formatting goes, it is preferable that the //pound// (''#'') symbol be placed on column 0,
however, directives may reside at an indentation level with respect to surrounding code.

<code c++>
    #if condition
    #   if condtion2
    #       define MACRO(x) thing1(x);
    #   else
    #       define MACRO(x)
    #   endif
    #endif
</code>

It is advisible that the preprocessor not be used to define constants when they could be defined natively using ''const'' or ''static const''
This is advantageous as native constants respect scope, and may furthermore be debugged more easily.


==== Private implementations ====


We recommend that C++ classes use the PIMPL pattern as it encourages correctly separating the public and internal parts of a class, and makes it possible to retain ABI compatibility even when the implementation of the class changes. You can find macros in **de/libcore.h** that make it more convenient to work with private implementations.

Example of a class declaration in a **.h** file:


<code c++>
class MyClass
{
public:
    MyClass();
 
private:
    DENG2_PRIVATE(d)
};
</code>

The implementation in a **.cpp** file:

<code c++>
DENG2_PIMPL(MyClass)
{
    // private members
    int a = 0;
 
    Impl(Public *i) : Base(i)
    {
        // `Public` is a typedef for `MyClass`, `Base::self` is a reference to `*i`
    }
 
    ~Impl()
    {
        // destruct
    }
};
 
MyClass::MyClass() : d(new Impl(this))
{}
 
// ~MyClass not necessary because `d` will be automatically deleted.
</code>


==== Member order ====


In class definitions, headers, and source units it is advisable that methods be ordered primarily with respect to visibility -- e.g. static methods at the top of the file, and secondly with respect to concern.
Type declarations and constants isolated to a single source unit may be placed either at the beginning, or within the narrowest scope possible.


==== Naming conventions ====


Classes, Enumerations, Structures, and Unions declared in C++ units should be named using ''UpperCamelCase'' with an appropriate name. Local variables, methods, 
and method parameters should be named using ''lowerCamelCase''. It is preferrable that method names contain a descriptive verb (e.g. ''joinServer'' or ''fetchDescriptor''). We generally follow a Qt-like naming convention, where getters do no have a verb in the method name.
  *  ''int someValue()''
  *  ''void setSomeValue(int)''

An underscore prefix is used for private members:
  *  ''int _someValue;'' is a private member in a class
  *  ''int someValue;'' is a public member (including members in a PIMPL struct)

Constants, regardless of scope, and macros, should be named using ''UPPER_SNAKE_CASE''. There should be no need for use of ''lower_snake_case'' or ''Mixed_snake_case'' in C++.


==== Const ====


''const'' must be used consistently in your classes: getters and all other members that do not modify the (effective) state of the class should always be const. Prefer to use const references to pass arguments to methods.

The ''const'' keyword should be placed //after// type identifiers:


 String const &str
 static int const CONSTANT = 1;



===== C =====


C code should be formatted similarly to C++, with some exceptions.


==== Naming conventions ====


Enumerations, Structures, and Unions, being types, should be named using ''lower_snake_case'' with a suffixed ''_t''. All other symbol names should be formatted in accordance with the aforementioned C++ guidelines.


===== Doxygen =====


Comments in Doxygen format describing the purpose, usage, parameters, return values, errors, and other information about functions is encouraged everywhere. Place Doxygen documentation about methods/functions in the header files. The [[:api_documentation]] is generated automatically by the autobuilder once per day.

For example:

<code c++>
/**
 * Does magical things to foobars.
 *
 * @param fooBar  Foobar to magic.
 *
 * @return  @c true, if the foobar is magicked.
 *          Otherwise @c false.
 */
bool Foobar_DoMagic(foobar_t *fooBar);
</code>


===== Version control =====


Doomsday uses Git and [[https://github.com/|GitHub]]. In order to submit code, you must have ordered it in logic units,
known as commits, and pushed it to a fork of the official Doomsday Engine repository. Once you are ready to have your
work reintegrated, open a pull request with a description of your work.

This section contains guidelines to which you should adhere in order to insure that your work can be integrated
without issue.


==== Commit messages ====


When writing a commit message, you should keep in mind that the intended audience for the message is not only other developers but also the end users who read build reports to see what has changed in each build. Therefore, the message should contain enough detail to be understood on its own. If nothing else, the commit message should explain motivations behind the changes (if they are not obvious, like a bugfix, cleanup, or referencing a tracker issue).

Your commit message should contain //tags// that describe the type of commit (e.g. ''Fixed'') and those components which have been affected by the commit. You can see many examples of the tags by examining the Git history. The commit tags serve a couple of purposes:
  *  At-a-glance summary of what the commit is about.
  *  Group commits in the build reports generated by the [[devel:autobuilder]]. Untagged commits will appear under a “Miscellaneous” heading in the build reports.
  *  Index the commits in the [[:codex]].

While the tags are not checked for validity, you should generally use one of the commonly used tags as listed in [[http://source.dengine.net/codex/index_by_size.html|Codex: Tags by size]]. The most common tags are: ''Fixed'', ''Refactor'', ''Added'', and ''Cleanup''.

Doomsday Engine uses an external [[http://tracker.dengine.net/projects/deng|issue tracker]] to manage issues. The tracker is integrated with the git repository, and will parse commit messages for information about commits. Should you need to reference an issue ID in your commit message, you may do so by placing the text ''IssueID #0''
in your commit message, where ''0'' is your issue ID. When your commit is merged, the issue tracker will attach your
commit to the relevant issue.

An example commit might look like this:

<code>Fixed|libcommon|Doom: Foobar is no longer crashing the game

Turns out the foobar was accessing a null pointer.

IssueID #666
</code>

==== Commit etiquette ====


Avoid keeping work-in-progress commits in your commit history. If you need to save your work before checking out a commit or
branch, you may use ''git stash'' to store your progress rather than a commit, or keep your work-in-progress commits in a private work branch and rebase/squash them later.

Commits should generally represent the completion of a goal. As such, it is //preferable// that they compile,
but not critical, though it should be noted that whether or not a desired commit may be built successfully can and 
will effect the change of getting your pull request merged.


==== Feature branches ====


When working on multiple tasks at once, you can work on feature branches should you wish to avoid cross-contaminating
code. A feature branch is a branch from the working master that contains a set of changes specific to one task or 
feature. An added advantage of feature branches is the ability to open a pull request specifically with that 
feature branch's changes.


==== Integrate often ====


In order to insure the smoothest merge, among other things, try to pull from your base often (e.g. ''origin master'').
If you must make a merge commit (without conflicts), not that it will be ignored by the tracker. If you have to resolve conflicts,
it is your decision whether they are trivial enough (i.e. some newlines in a comment) to be lumped in to a commit to the likes of ''Merge origin/master into ...''
or if a more descriptive commit is needed. 

Failure to integrate regularly means that, should you choose to integrate changes from upstream in to your working 
base, you can run in to unpleasant merge conflicts; alternatively, should a project manager wish to merge your work,
they might run in to similar merge conflicts.


===== Documentation =====


Always write appropriate API documentation for your class APIs using Doxygen.

While/after implementing a feature, document it here in the wiki:

  *  User/gameplay features should be categorized into .
  *  Modding/map author features should be categorized into .
  *  Technical/internal/API features should be categorized into .

Certain documentation is processed with [[devel:Amethyst]]. This includes the [[:readme]] included in distribution packages, Unix manual pages, and runtime console commands and variables.