Coding style design - Godlike/Unicorn GitHub Wiki
The design topic is quite broad, so we'll outline the topics we'll be talking about:
- Directory structure
- File structure
- Submodule structure
- Class design
- Architecture approaches
Outer structure of repository is as follows:
.
├── cmake # global cmake modules including but not limited to
│ # UnicornEngine's cmake configuration
│
├── demos # unicorn engine demos
├── hooks # repository hooks
├── tests # tests
└── UnicornEngine # engine directory
├── cmake # cmake modules describing `external` contents
│ # and `modules` in this directory
│
├── data # built-in shaders and other assets
├── docs # doxygen settings
├── external # third-party or external projects that are statically
│ # linked with UnicornEngine
│
└─ [module structure] # typical module structure that we'll cover further
Each module (including UnicornEngine itself) has the following structure:
[module root]
├── CMakeLists.txt # cmake project configuration for this module
├── cmake # [optional] cmake modules describing `modules` in this directory
├── include # directory to be used in cmake's `include_directories()`
│ └── unicorn
│ └─ [path to module root]
│ └─ [module contents]
├── source
│ └─ [module contents]
└─ [submodules]
Ideally modules shall not contain any submodules or external projects with obvious exception for UnicornEngine that contains those.
All information regarding the module shall be contained in .cmake
file inside the cmake
directory of parent directory. This .cmake
shall declare the following variables:
-
${UNICORN_ENGINE_[MODULE]_DIR}
with the path tomodule root
for cmake'sadd_subdirectory()
-
${UNICORN_ENGINE_[MODULE]_INCLUDE_DIR}
with the path toinclude
directory for cmake'sinclude_directories()
-
${UNICORN_ENGINE_[MODULE]_LIB}
with the module's library name(s) for cmake'sadd_library()
andtarget_link_libraries()
- All necessary information and prerequisites for module's
CMakeLists.txt
External module configuration **shall be contained in [Module]Config.cmake
file. Module name in this case shall be in UpperCamelCase
.
Internal module configuration **shall be contained in [module].cmake
file. Module name in this case shall be in lower_case_with_underscores
.
Example of creating a module named `utility` with a `Logger` class
Directory structure would look like this:
.
├── cmake
│ └─M UnicornEngineConfig.cmake
├── demos
├── hooks
├── tests
└── UnicornEngine
├─M CMakeLists.txt
├── cmake
│ └─+ utility.cmake
├── data
├── docs
├── external
└─+ utility
├─+ CMakeLists.txt
├─+ include
│ └─+ unicorn # for `#include <unicorn/...>` includes
│ └─+ utility # module name
│ └─+ Logger.hpp # headers
└─+ source
└─+ Logger.cpp # sources without module hierarchy
Now let's cover all modified/added files one by one:
M cmake/UnicornEngineConfig.cmake
@@ -14,4 +14,5 @@ set(UNICORN_ENGINE_INCLUDE_DIR
set(UNICORN_ENGINE_INCLUDE_DIRS
"${UNICORN_ENGINE_INCLUDE_DIR}"
+ "${UNICORN_ENGINE_ROOT}/utility/include"
CACHE LIST "List of UnicornEngine include directories")
M UnicornEngine/CMakeLists.txt
@@ -46,3 +46,6 @@ include(loguru)
## STB
include(stb)
+
+## utility
+include(utility)
@@ -76,5 +79,6 @@ if (BUILD_SHARED_LIBS)
add_definitions(-D${UNICORN_ENGINE_LIB}_EXPORTS)
endif()
+add_subdirectory(utility)
set(UNICORN_ENGINE_HEADERS
@@ -113,6 +117,8 @@ set(UNICORN_ENGINE_SOURCES
add_library(${UNICORN_ENGINE_LIB}
${UNICORN_ENGINE_HEADERS}
${UNICORN_ENGINE_SOURCES}
+
+ $<TARGET_OBJECTS:${UNICORN_ENGINE_UTILITY_LIB}>
)
if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
A UnicornEngine/cmake/utility.cmake
# Copyright (C) 2017 by Godlike
# This code is licensed under the MIT license (MIT)
# (http://opensource.org/licenses/MIT)
set(UNICORN_ENGINE_UTILITY_DIR
"${UNICORN_ENGINE_ROOT}/utility"
)
set(UNICORN_ENGINE_UTILITY_LIB
utility
)
A UnicornEngine/utility/CMakeLists.txt
# Copyright (C) 2017 by Godlike
# This code is licensed under the MIT license (MIT)
# (http://opensource.org/licenses/MIT)
cmake_minimum_required(VERSION 3.0)
cmake_policy(VERSION 3.0)
set(UTILITY_HEADERS
include/unicorn/utility/Logger.hpp
)
set(UTILITY_SOURCES
source/Logger.cpp
)
add_library(${UNICORN_ENGINE_UTILITY_LIB} OBJECT
${UTILITY_HEADERS}
${UTILITY_SOURCES}
)
if (UNIX)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fPIC")
endif()
There are also Logger.hpp
and Logger.cpp
files that we will not list here since they're not adding anything to the example.
Here is the outline of this design approach (points marked with † are subject to change):
- The idea is to separate all external information (that can be used from different CMake files throughout the project i.e. tests, libraries, etc) from internal mechanisms performed by module's
CMakeLists.txt
. - Only domains are described as modules. Not every directory is a CMake project.
- If you're not sure where your files shall be located, please ask for help in our Discord chat
-
cmake/UnicornEngineConfig.cmake
-
† File contains hardcoded values that do not rely on
module.cmake
. This is done so that external projects (i.e. games) can use the same CMake variable forinclude_directories()
to resolve all#include <unicorn/...>
directives. Note: #65 is addressing this point.
-
† File contains hardcoded values that do not rely on
-
UnicornEngine/CMakeLists.txt
- The
include([module])
andadd_subdirectory(${UNICORN_ENGINE_[MODULE]_DIR})
are split in order to allow for some global flags to be entered in-between.
- The
-
UnicornEngine/cmake/[module].cmake
- Declares needed variables for proper CMake processing (both for
UnicornEngine/CMakeLists.txt
andUnicornEngine/[module]/CMakeLists.txt
)
- Declares needed variables for proper CMake processing (both for
-
UnicornEngine/[module]/include
-
Contains the whole tree of includes starting with
unicorn
at top level. This is done to allow#include <unicorn/...>
directives without the use of final compiled package of includes (after cmake'sinstall()
.
-
Contains the whole tree of includes starting with
-
UnicornEngine/[module]/source
-
Does not contain the whole tree, only the infrastructure of the module. The directory should still follow the same structure as the
include/unicorn/[module path]
directory.
-
Does not contain the whole tree, only the infrastructure of the module. The directory should still follow the same structure as the
On a different note we have to address what should be used as a git submodule
and not placed inside the repository. Here is a quick list of questions that you can use for a reference:
- Is the module developed solely for our project? If not - it should always be used as
git submodule
. - Will commits for the module unnecessarily clutter
master
commit history and pull requests? - Will the use of the module benefit of
revert
feature? If commit history is rich, it would be easier to rollback to some previous state by switchinggit submodule hash
instead of reverting commits. - Shall the project use only stable versions of the module?
- Are the issues for the module independent from the project?
- Does it make sense to provide third-party solutions for the same domain of problems as an alternative to this module? i.e. support of different physics engines
In this section we will discuss different file structures:
Each file shall have corresponding copyright block in the beginning. It will not be mentioned in the following sections.
All variables in CMake module files (.cmake
) shall have descriptive docstring.
External module configuration shall be structured as follows:
- Module configuration variables that will be processed by module's CMakeLists.txt [if applicable]
-
${[MODULE]_DIR}
and${[MODULE]_INCLUDE_DIR}
variables -
${[MODULE]_LIB}
variable [if applicable]
Internal module configuration shall be structured as follows:
-
${[MODULE]_DIR}
and${[MODULE]_INCLUDE_DIR}
variables -
${[MODULE]_LIB}
variable [if applicable] - Validations and other variables for the module
- May include
add_definitions()
if necessary
- May include
Try to keep the following structure for CMakeLists.txt files:
-
cmake_minimum_required()
andcmake_policy()
- Try to keep the minimum required version. Check with documentation when in doubt
include_directories()
add_definitions()
-
set([MODULE]_HEADERS)
containing all headers in this module- Use empty lines to separate directories
- Consider splitting to several variables if the list gets too long
-
set([MODULE]_SOURCES)
containins all sources in this module- Use empty lines to separate directories
- Consider splitting to several variables if the list gets too long
-
add_library(${[MODULE}_LIB OBJECT)
with all${[MODULE]_HEADERS}
and${[MODULE]_SOURCES}
This list shall be used as a guide and not the rule. If it makes sense to reorder some stuff for better readability - go for it.
-
The
#define
guard- All header files should have
#define
guards to prevent multiple inclusion - The name of the guard should be formatted according to file's include path:
#ifndef UNICORN_SYSTEM_LOGGER_HPP #define UNICORN_SYSTEM_LOGGER_HPP // ... #endif // UNICORN_SYSTEM_LOGGER_HPP
- All header files should have
-
Include directives in the following order:
- Other headers in this module
- Headers from other
unicorn
modules - Headers from third-party modules but only if they should be included from header files
- System includes
- Use empty lines to separate between groups
- Use empty lines to separate directories within their group
- All include directives shall be sorted alphabetically within their group
-
Preprocessor macros
- If some
#include
depends on some preprocessor definition, it may be placed next to the corresponding#include
definition
- If some
-
Forward declarations
- Use forward declarations where it makes sense
- Avoid using forward declarations when you're using the variable (even when you're just proxying it)
- Remember about potential problems with forward declarations such as:
- Hidden dependencies when recompilations didn't happen for the code that was using the variable
- External module updates may brake forward declarations altogether
- If a header declares multiple symbols, forward declaring them would be more verbose than a simple
#include
which may hurt readability - Forward declarations may be proxied from other
#include
directives, so be aware of it
-
Class/struct/template declarations
- Access modifiers shall be explicitly stated and declared in the following order:
public
protected
private
- Empty access modifiers shall be omitted
- Each access modifier shall be declared only once
- Contents of each access modifier group shall be in the following order:
using
typedef
- nested
class
/struct
/template
-
static const
variables -
static
methods -
static
variables - class methods (with rule of five)
- all needed versions of constructor
- copy constructor
- copy assignment operator
- move constructor
- move assignment operator
- destructor
- all other methods
- class variables
- Access modifiers shall be explicitly stated and declared in the following order:
-
#include
for template implementation [if applicable]
Important notes:
- Use general language features instead of
#pragma
. - Avoid using
#pragma
altogether when you can help it. Pragmas are implementation dependent. Use them only when you know what you're doing and you aim only at specific compilers (i.e. eliminating false positive warnings).