Skip to content

Commit

Permalink
HACKING updated
Browse files Browse the repository at this point in the history
  • Loading branch information
captainurist committed Nov 1, 2023
1 parent 7c6ef3f commit 8fce171
Showing 1 changed file with 27 additions and 31 deletions.
58 changes: 27 additions & 31 deletions HACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,31 +7,26 @@ Dependencies
------------

Main dependencies:
* [SDL2](https://www.libsdl.org/download-2.0.php)crossplatform media framework;
* [FFmpeg](https://ffmpeg.zeranoe.com/builds/) — video support;
* [OpenAL](https://www.openal.org/downloads/OpenAL11CoreSDK.zip) — audio support;
* [Zlib](http://gnuwin32.sourceforge.net/packages/zlib.htm) — compression.
* [SDL2](https://github.com/libsdl-org/SDL/tree/SDL2)cross-platform media framework;
* [FFmpeg](https://github.com/FFmpeg/FFmpeg) — video support;
* [OpenAL Soft](https://github.com/kcat/openal-soft) — audio support;
* [Zlib](https://github.com/madler/zlib) — compression.

On Windows, the above dependencies are resolved automatically during the cmake phase.
By default, we are using prebuilt dependencies, and they are resolved automatically during the cmake phase.

On Linux they are usually part of distribution and you only need to install the development versions of SDL2, ffmpeg, openal and zlib libraries. E.g. on Ubuntu:
The only exception is Linux, where we're using SDL2 from the distribution, so you will need to install a development versions of SDL2 before building OpenEnroth. E.g. on Ubuntu:
```bash
sudo apt-get install ffmpeg ffmpeg-devel
sudo apt-get install openal openal-devel
sudo apt-get install zlib zlib-devel
sudo apt-get install SDL2 SDL2-devel
```

On Mac you can install the dependencies from homebrew.

Additional dependencies:
* CMake 3.20.4+ (3.20.21032501-MSVC_2 from VS2019 won't work);
* Python 3.x (optional, for style checks).

Minimum required compiler versions are as follows:
* Visual Studio 2022;
* GCC 11;
* Clang 13.
* GCC 13;
* AppleClang 15 or Clang 15.

The following IDEs have been tested and should work fine:
* Visual Studio (2022 or later);
Expand All @@ -42,8 +37,7 @@ The following IDEs have been tested and should work fine:
Building on \*nix platforms
---------------------------

This project uses the [CMake](https://cmake.org) build system.
Use the following commands to clone repository and build (it is recommended to build in a separate directory as shown here):
This project uses the [CMake](https://cmake.org) build system. Use the following commands to clone the repository and build OpenEnroth:

```
$ git clone https://github.com/OpenEnroth/OpenEnroth.git
Expand All @@ -52,8 +46,7 @@ $ cmake -B build -S .
$ cmake --build build
```

To compile x86 build on x86_64 platform you can pass `-m32` via compiler flags to cmake.
For example instead of just `cmake ..` above execute `CFLAGS="-m32" CXXFLAGS="-m32" cmake ..`
To cross-compile for 32-bit x86, you can pass `-m32` via compiler flags to cmake. In the snipped above that would mean running `export CFLAGS="-m32" CXXFLAGS="-m32"` first.

You can also select platform dependent [generator](https://cmake.org/cmake/help/latest/manual/cmake-generators.7.html) for your favorite IDE.

Expand All @@ -72,7 +65,7 @@ Building on Windows
* Select startup item as `OpenEnroth.exe`.
* Run!

If you wish you can also disable autoresolving main dependencies by turning off `OE_USE_PREBUILT_DEPENDENCIES` cmake option and pass your own dependencies source, e.g. via [vcpkg](https://github.com/microsoft/vcpkg) integration.
If you wish you can also disable prebuilt dependencies by turning off `OE_USE_PREBUILT_DEPENDENCIES` cmake option and pass your own dependencies source, e.g. via [vcpkg](https://github.com/microsoft/vcpkg) integration.

__Be aware__ that Visual Studio has a bug with git submodules not syncing between branches.
So when checking out the branch or switching to different branch you may need to run the following command manually: `git submodule update --init`
Expand All @@ -81,10 +74,9 @@ So when checking out the branch or switching to different branch you may need to
Coding style
------------

For the C++ code we are following the [Google C++ Style Guide](http://google.github.io/styleguide/cppguide.html).\
Source code is automatically checked against it and Pull Request will fail if you don't follow it.
For the C++ code we are following the [Google C++ Style Guide](http://google.github.io/styleguide/cppguide.html). Source code is automatically checked against it and pull request will fail if you don't follow it.

To perform style check before pushing anything you can build `check_style` target. In Visual Studio you can do that by going to ***Solution Explorer → Change Views → CMake targets***. Right click and build `check_style`, errors will be listed in output.
To perform a style check before pushing anything you can build `check_style` target. In Visual Studio you can do that by going to ***Solution Explorer → Change Views → CMake targets***. Right click and build `check_style`, errors will be listed in output.

We also follow some additional style preferences, as listed below.

Expand All @@ -98,8 +90,8 @@ Naming:
* Use `SNAKE_CASE_ALL_CAPS` for `enum` values. E.g. `ITEM_CRATE_OF_ARROWHEADS`, `ITEM_SLOT_RING6`.
* For naming enum values, prefix the name with the name of the type, e.g. `MONSTER_TROLL_A` for a value of `enum class MonsterId`. For values that are then used in array bounds, put the `_FIRST`, `_LAST` and `_COUNT` right after the name of the type, e.g. `ITEM_FIRST_MESSAGE_SCROLL`.
* Use `CamelCase` for everything else.
* Type names should start with a capital letter. E.g. `IndexedArray`, `InputAction`, `PlatformLogLevel`. This applies to all types, including classes, structs, enums and typedefs, with certain exceptions as listed below.
* Method & function names should start with a lowercase letter. E.g. `Vec3::length`, `gridCellToWorldPosX`, `getCeilingHeight`.
* Type names should start with a capital letter. E.g. `IndexedArray`, `InputAction`, `LogLevel`. This applies to all types, including classes, structs, enums and typedefs, with certain exceptions as listed below.
* Method & function names should start with a lowercase letter. E.g. `Vec3::length`, `gridCellToWorldPosX`, `ceilingHeight`.
* Variable names should start with a lowercase letter. E.g. `int monsterCount = level->monsterCount()`.
* Names of private members should start with an underscore to visually distinguish them from variables without having to spell out `this->` every single time. E.g. `_initialized = true`, where `_initialized` is a member field.
* Note that the above doesn't apply to POD-like types as for such types all members are public and are named just like ordinary variables.
Expand All @@ -115,6 +107,12 @@ Language features:
* Use an `int` unless you need something else. Don’t try to avoid negative values by using `unsigned`, this implies many changes to the usual behavior of integers and can be a source of bugs. See a section in [core guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#arithmetic) for more info. However, don't overdo it, use `size_t` for indexing into STL containers.
* We generally refrain from using namespaces, and we consider the approach where namespace hierarchy follows the directory structure an antipattern that might make sense in other languages (e.g. Java), but not in C++. Besides, OpenEnroth is a relatively small codebase and doesn't need the measures advocated by the Google style guide to prevent name clashes. Exception to this rule is `namespace detail` that you're encouraged to use to hide implementation details and to prevent cluttering of the global namespace.

Error handling:
* Use `assert`s to check for coding errors and conditions that must never be false, no matter how the program is run.
* Use exceptions for non-recoverable errors. It's usually OK to just throw an instance of `class Exception`.
* Use `Logger` for warnings and recoverable errors.
* We don't yet have a mechanism for displaying errors to the user through the UI. This document will be updated once this is implemented.

There is a lot of code in the project that doesn't follow these conventions. Please feel free to fix it, preferably not mixing up style and logical changes in the same PR.


Expand All @@ -135,13 +133,11 @@ Our basic guidelines for code organization are:

Testing
-------
We strive for a good test coverage of the project, and while we're not there yet, the current policy is to add tests for all the bugs we fix, as long as the fix is testable. E.g. graphical glitches are generally very hard to test, but we have the infractructure to test game logic and small isolated classes.

Tests are not built by default, and you will need to set `OE_BUILD_TESTS` cmake variable to be able to build them. You can either use your IDE for that, or just run `cmake` with `-DOE_BUILD_TESTS=ON` from the command line.
We strive for a good test coverage of the project, and while we're not there yet, the current policy is to add tests for all the bugs we fix, as long as the fix is testable. E.g. graphical glitches are generally very hard to test, but we have the infrastructure to test game logic and small isolated classes.

Tests in OpenEnroth fall into two categories:
* Unit tests. These are a standard breed of tests, written using Google Test. You can see some examples in `src/Utility/Tests`.
* Game tests. If you're familiar with how testing is done these days for complex mobile apps, then you can consider game tests a variation of UI tests that's specific to our project. Game tests need game assets to run, and are currently **not** run in github actions.
* Game tests. If you're familiar with how testing is done these days for complex mobile apps, then you can consider game tests a variation of UI tests that's specific to our project. Game tests need game assets to run.

Game tests work by instrumenting the engine, and then running test code in between the game frames. This code usually sends events to the engine (e.g. mouse clicks), which are then processed by the engine in the next frame, but it can do pretty much anything else – all of engine's data is accessible and writable from inside the game test.

Expand All @@ -151,18 +147,18 @@ As typing out all the events to send inside the test code can be pretty tedious,
* Perform the steps that used to reproduce the bug.
* Press `Ctrl+Shift+R` again to stop trace recording. You will get two files generated in the current folder – `trace.json` and `trace.mm7`.
* Rename them into something more suiting (e.g. `issue_XXX.json` and `issue_XXX.mm7`) and create a PR to the [OpenEnroth_TestData](https://github.com/OpenEnroth/OpenEnroth_TestData) repo.
* Once it's merged update the reference tag in the corresponding [CMakeLists.txt](https://github.com/OpenEnroth/OpenEnroth/blob/master/test/Bin/GameTest/CMakeLists.txt#L21) in the main repo.
* Create a new test case in one of the game test files in [the game tests folder](https://github.com/OpenEnroth/OpenEnroth/tree/master/test/Tests).
* Once it's merged update the reference tag in the corresponding [CMakeLists.txt](https://github.com/OpenEnroth/OpenEnroth/blob/master/test/Bin/GameTest/CMakeLists.txt) in the main repo.
* Create a new test case in one of the game test files in [the game tests folder](https://github.com/OpenEnroth/OpenEnroth/tree/master/test/Bin/GameTest).
* Use `TestController::playTraceFromTestData` to play back your trace, and add the necessary checks around it.

If you need to record a trace with non-standard FPS (e.g. if an issue doesn't reproduce on lower FPS values), set `debug.trace_frame_time_ms` config value before starting OpenEnroth for recording.

By default, traces are recorded with a random number generator that's just returning a sequence of natural numbers. If you want to record a trace using the default Mersenne Twister random number generator, set `debug.trace_random_engine` config value to `mersenne_twister` before starting OpenEnroth for recording.

To run all unit tests locally, build a `UnitTest` cmake target, or build & run `OpenEnroth_UnitTest`.

To run all game tests locally, set `OPENENROTH_MM7_PATH` environment variable to point to the location of the game assets, then build `GameTest` cmake target. Alternatively, you can build `OpenEnroth_GameTest`, and run it manually, passing the paths to both game assets and the test data via command line.

Note that if you can't find either `UnitTest` or `GameTest` target in the target list of your IDE, this likely means that you haven't set the `OE_BUILD_TESTS` cmake variable as described above.

Changing game logic might result in failures in game tests because they check random number generator state after each frame, and this will show as `Random state desynchronized when playing back trace` message in test logs. This is intentional – we don't want accidental game logic changes. If the change was actually intentional, then you might need to either retrace or re-record the traces for the failing tests. To retrace, run `OpenEnroth retrace <path-to-trace.json>`. Note that you can pass multiple trace paths to this command.


Expand Down

0 comments on commit 8fce171

Please sign in to comment.