# [blog] Improving a simple game using C++ and SFML – refactor step by step - part 1

## What and why?

Recently I decided to take a look at all my repositories on [my GitHub](https://github.com/whiskeyo?tab=repositories) and I came across a ["The Adventure in Caves"](https://github.com/whiskeyo/The-Adventure-in-Caves) game that I implemented for my Object-Oriented Programming course while I was at the uni. As it was my first "big" project in C++, I did not know a lot about proper programming, thus it contains a lot of stuff that could be changed. This post is about everything I have done to improve the game to make it actually a good project that others could learn from.

By the time of implementing the game I recorded a [short gameplay](https://www.youtube.com/watch?v=XgQRgbAI9ow), feel free to check this out so you know what we're going to work with!

## What should be improved?

1. Introducing CMake
2. Adding Github Action to automate building
3. Changing repository structure + embedding assets into binary
4. Introducing `clang-format`
5. Cleaning up the code - the same coding practices in the whole repository
6. Introducing `clang-tidy`
7. Introducing `googletest`

## Introducing CMake

As mentioned in the previous paragraph, I was lacking a lot of knowledge about doing things properly. The first thing that had to be changed is a way of building the project. I have not used any build system back then, all that README stated was:

$ sudo apt-get install libsfml-dev
$ g++ -c *.cpp 
$ g++ -o TAiC *.o -lsfml-graphics -lsfml-window -lsfml-system
$ ./TAiC

This caused multiple problems, such as:

1. no control over the workflow,
2. it is easy to make a mistake in one of these commands, leading to improper build output
3. no compile flags passed to `g++` meaning that the implementation could contain a code that could lead to unexpected behaviour
4. all "artifacts" (meaning `*.o` files and the binary) landed in the root directory, making a mess without any easy way of removing files instead of calling `rm -rf *.o TAiC` every time.

I decided to use CMake, as it is what I work with on daily basis, so I started my way by creating a simple `CMakeLists.txt` file in the root directory of the repository:

# basic information about the minimum CMake version and the name of the project
cmake_minimum_required(VERSION 3.20)
project(the-adventure-in-caves)

# setting up compiler standard and flags, -Werror skipped due to compilation errors
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_FLAGS "-O3 -Wall -Wextra -Wpedantic")

# glob for finding all cpp files, compiling them and linking SFML
file(GLOB SOURCE_FILES *.cpp)
add_executable(the-adventure-in-caves ${SOURCE_FILES})
target_link_libraries(the-adventure-in-caves sfml-graphics sfml-window sfml-system)

With that configuration (it's not the final version yet!) I could try to compile the project using `cmake -S . -B build && cmake --build build -j24`. It worked and this is the output I've got:

-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /home/whiskeyo/repos/taic/build
[ 14%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Block.cpp.o
[ 14%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Coin.cpp.o
[ 21%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Collider.cpp.o
[ 28%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Enemy.cpp.o
[ 35%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Game.cpp.o
[ 42%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/HUD.cpp.o
[ 50%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Heart.cpp.o
[ 57%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Player.cpp.o
[ 64%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/ResourceManager.cpp.o
[ 71%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Window.cpp.o
[ 78%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/Portal.cpp.o
[ 85%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/main.cpp.o
[ 92%] Building CXX object CMakeFiles/the-adventure-in-caves.dir/World.cpp.o
/home/whiskeyo/repos/taic/HUD.cpp: In member function ‘void HUD::update()’:
/home/whiskeyo/repos/taic/HUD.cpp:29:31: warning: comparison of integer expressions of different signedness: ‘int’ and ‘std::__cxx11::list<HeartHUD>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare]
   29 |     if (m_player->getHealth() == m_health.size())
      |         ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
/home/whiskeyo/repos/taic/HUD.cpp:39:27: warning: comparison of integer expressions of different signedness: ‘int’ and ‘std::__cxx11::list<HeartHUD>::size_type’ {aka ‘long unsigned int’} [-Wsign-compare]
   39 |         for (int i = 0; i < m_player->getHealth() - m_health.size(); i++)
      |                         ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[100%] Linking CXX executable the-adventure-in-caves
[100%] Built target the-adventure-in-caves

So far so good, there is one thing that we have to take a look later on, but it should not be hard. Also, we could try to introduce fetching SFML while configuring the project, but as it is not the most important thing now, we may continue.

## Adding Github Action to automate building

Since a few years, GitHub started to suggest workflows that could help developers integrate the application easily. It means that we can simply use the suggested Action (from the [Actions tab](https://github.com/whiskeyo/The-Adventure-in-Caves/actions/new)) and the one that fits the project the most is called "CMake based, single-platform projects". From the suggested `.yml` file we can extract what we need the most:

name: the-adventure-in-caves - release build

on:
  push:
    branches: [ "master" ]
  pull_request:
    branches: [ "master" ]

env:
  BUILD_TYPE: Release

jobs:
  build:
    runs-on: ubuntu-latest

    steps:
    - uses: actions/checkout@v4

    - name: Configure CMake
      run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}}

    - name: Build
      run: cmake --build ${{github.workspace}}/build --config ${{env.BUILD_TYPE}}

That file should be placed in `.github/workflows` directory, let's call it `release.yml`. As it works on push and pull request to `master` branch, we have to create the [Pull Request](https://github.com/whiskeyo/The-Adventure-in-Caves/pull/1) that would run the job. Aaaaand... it failed, since we did not satisfy SFML dependency for the project. How to fix it? Simply add another step after the `actions/checkout@v4` that looks like this:

      - name: Install dependencies
        run: sudo apt-get install libsfml-dev

and it should work just fine. The final state of the file can be found in commit [`5e5089e`](https://github.com/whiskeyo/The-Adventure-in-Caves/pull/1/commits/5e5089e2c0c6e237426f542b083c8d064d7251b4).

## Changing repository structure

You probably know how hard it is to navigate in a project that has basically no structure - finding files is complicated, there is no split between responsibilities, everything is just a big mess.

Now it's time to clean it up, so let's get on - the first step is to split the actual source code from all assets, as well as take it out of the root folder. What should we get at the first moment? Single `src` directory with all `*.{hpp,cpp}` files in it, as well as nested `assets` directory with `fonts`, `levels` and `textures`.

This of course requires some alignments, as the first attempt of building the project returns an error:

CMake Error at CMakeLists.txt:8 (add_executable):
  No SOURCES given to target: the-adventure-in-caves

Avoiding it is pretty simple, the `CMakeLists.txt` needs another definition of the `file` call:

file(GLOB SOURCE_FILES src/*.cpp)
                       ^^^^
                    this is new

Right now compilation goes well, so let's run the game and see what's going on.


Something went horribly wrong! The first suspicion I have is that assets are not loading properly, since now the path is quite different to the previous path. Also, there were some logs printed:

$ build/the-adventure-in-caves 
Setting vertical sync not supported
Failed to load image "textures/background1.jpg". Reason: Unable to open file
Failed to load image "textures/background2.jpg". Reason: Unable to open file
Failed to load image "textures/background3.png". Reason: Unable to open file
Failed to load image "textures/background4.png". Reason: Unable to open file
Failed to load image "textures/background5.jpg". Reason: Unable to open file
Failed to load image "textures/background6.jpg". Reason: Unable to open file
Failed to load image "textures/player.png". Reason: Unable to open file
Failed to load image "textures/coin.png". Reason: Unable to open file
Failed to load image "textures/heart.png". Reason: Unable to open file
Failed to load image "textures/portal_bright.png". Reason: Unable to open file
Failed to load image "textures/portal_dark.png". Reason: Unable to open file
Failed to load image "textures/enemy_weak_left.png". Reason: Unable to open file
Failed to load image "textures/enemy_weak_right.png". Reason: Unable to open file
Failed to load image "textures/enemy_strong_left.png". Reason: Unable to open file
Failed to load image "textures/enemy_strong_right.png". Reason: Unable to open file
Failed to load font "fonts/04B_30__.TTF" (failed to create the font face)

This caused a lot of headache, since with that approach I would have to store all resources in a way that they are available to the end user, and I wanted to avoid it on all costs, so I decided to look for some information about embedding any resources into header files.

### Embedding assets into binary

For that one I had to do some research in order to find a solution of "porting" any files such as `*.png`, `*.ttf` and others into `*.hpp` headers. I had one requirement - it has to work in CMake, so the solution is portable. Happily I quickly found a [solution posted by Amir Saniyan](https://stackoverflow.com/a/62645161/10714380) that allowed me to integrate it into the project.

With some help of Github Copilot, I managed to prepare `assets/CMakeLists.txt` that loops through all files in `fonts`, `levels` and `textures` directories and creates header files for every file. The last line with `include_directories` statement allows including assets from the code.

function(embed_resource resource_file_name source_file_name variable_name)
    message("Embedding resource: ${resource_file_name} -> ${source_file_name}")
    if(EXISTS "${source_file_name}")
        if("${source_file_name}" IS_NEWER_THAN "${resource_file_name}")
            return()
        endif()
    endif()

    file(READ "${resource_file_name}" hex_content HEX)

    string(REPEAT "[0-9a-f]" 32 pattern)
    string(REGEX REPLACE "(${pattern})" "\\1\n" content "${hex_content}")
    string(REGEX REPLACE "([0-9a-f][0-9a-f])" "0x\\1, " content "${content}")
    string(REGEX REPLACE ", $" "" content "${content}")

    set(array_definition "constexpr std::uint8_t ${variable_name}[] =\n{\n${content}\n};")
    set(source "#pragma once\n#include <cstdint>\n${array_definition}\n")

    file(WRITE "${source_file_name}" "${source}")
endfunction()

file(GLOB_RECURSE level_files "${CMAKE_CURRENT_SOURCE_DIR}/levels/*")
foreach(level_file ${level_files})
    get_filename_component(level_name ${level_file} NAME_WE)
    set(level_variable_name "LEVEL_${level_name}")
    set(level_header_file "${CMAKE_CURRENT_BINARY_DIR}/levels/${level_name}.hpp")
    embed_resource("${level_file}" "${level_header_file}" "${level_variable_name}")
endforeach()

file(GLOB_RECURSE texture_files "${CMAKE_CURRENT_SOURCE_DIR}/textures/*")
foreach(texture_file ${texture_files})
    get_filename_component(texture_name ${texture_file} NAME_WE)
    set(texture_variable_name "TEXTURE_${texture_name}")
    set(texture_header_file "${CMAKE_CURRENT_BINARY_DIR}/textures/${texture_name}.hpp")
    embed_resource("${texture_file}" "${texture_header_file}" "${texture_variable_name}")
endforeach()

file(GLOB_RECURSE font_files "${CMAKE_CURRENT_SOURCE_DIR}/fonts/*")
foreach(font_file ${font_files})
    get_filename_component(font_name ${font_file} NAME_WE)
    set(font_variable_name "FONT_${font_name}")
    set(font_header_file "${CMAKE_CURRENT_BINARY_DIR}/fonts/${font_name}.hpp")
    embed_resource("${font_file}" "${font_header_file}" "${font_variable_name}")
endforeach()

include_directories(${CMAKE_CURRENT_BINARY_DIR})

Then, from the `src/ResourceManager.cpp` I simply included all assets, created new functions for loading textures and fonts from the memory, and called them from the constructor:

#include <assets/fonts/04B_30__.hpp>
// all other assets go here :)
#include <assets/textures/portal_dark.hpp>

void ResourceManager::loadTextureFromMemory(
    const void* data, std::size_t size, const std::string& textureName) 
{
    sf::Texture texture;
    if (texture.loadFromMemory(data, size)) 
    {
        this->m_textures[textureName] = texture;
    }
}

void ResourceManager::loadFontFromMemory(
    const void* data, std::size_t size, const std::string& fontName)
{
    sf::Font font;
    if (font.loadFromMemory(data, size)) 
    {
        this->m_fonts[fontName] = font;
    }
}

ResourceManager::ResourceManager()
{
    loadTextureFromMemory(
        &TEXTURE_background1, sizeof(TEXTURE_background1), "background1");
    loadTextureFromMemory(
        &TEXTURE_background2, sizeof(TEXTURE_background2), "background2");
    // and so on
}

Let's quickly recompile the game and see if it works:


The game works, so let's move on to a bit simpler stuff now.

## Introducing `clang-format`

I decided to create my own `.clang-format` config that fits my needs, and for that puprose I used an online tool called [clang-format configurator](https://zed0.co.uk/clang-format-configurator/) where you can prepare your own config and check your changes on some provided code snippet.

To be honest, there is nothing more to talk about - after the config is ready, we should create a `.clang-format` file in the root directory of the project, paste all contents from the prepared config in the configurator and call this command to format all source files:

$ sudo apt install clang-format # if you don't have it yet
$ clang-format -i style=file src/*.{hpp,cpp}

The `.clang-format` file can be found [here](https://github.com/whiskeyo/The-Adventure-in-Caves/pull/1/commits/14e1f25481527450d47800085a1d97a1d76bb056#diff-1026e0038b722990204a42bed8a6f7c0ec2302aa79e3fad1959d62ba968edfa2).

### Don't we have too much commands we have to care about?

Another tool came in, so remembering all these commands or navigating through them using `^R` in the terminal can get annoying, so let's create a simple `Makefile` that would wrap all these commands under some "names". Again, create the file in the root directory and fill it up with:

SHELL := /bin/bash
.PHONY: all configure compile format install-dependencies

install-dependencies:
	@echo "Installing required dependencies"
	sudo apt-get install libsfml-dev clang-format

configure:
	@echo "Configuring project"
	cmake -S . -B build

compile:
	@echo "Building project"
	cmake --build build --parallel $(nproc)

format:
	@echo "Formatting code"
	clang-format -style=file -i $(wildcard src/*.cpp src/*.hpp)

all: configure compile

This way, by calling `make <target>` everything will be done by `make`, so we do not have to remember commands anymore (but it's still worth knowing them!).

### Removing commented out code

In order to get a bit cleaner code before further changes, I decided to remove all comments with some commented out code. In case I think it could be useful, I will keep the code documented instead, so we can easily get what the particular function/class is responsible for.

### Removing unnecessary includes & using C++ includes

Another thing to add is that I am using `clangd` as my LSP, which has a handy feature of suggesting which `#include`s are not required, and this is something that I will clean up, too. Less dependencies mean faster linking, so we may be happy!

Last, but not least, in order to keep myself chill, I also swapped all includes of C libraries (such as `math.h`) and swapped them with their C++ "versions" (in this case `<cmath>`).

### Splitting up implementation from `*.hpp` files

That change is pretty easy to understand - what compilers do when they see `#include "something.hpp"`? They simply paste the code into the file that includes them. But what does it actually mean?

Imagine you have the code:

/* log.hpp */
#pragma once
#include <string>
#include <iostream>

void log(const std::string& message)
{
    std::cout << "[Our good logger] " << message << std::endl;
}

Then, you want to switch the content of printed stuff inside brackets, so you do the change:

-    std::cout << "[Our good logger] " << message << std::endl;
+    std::cout << "[Our amazing logger] " << message << std::endl;

And all `*.cpp` files that included the `log.hpp` file have to be recompiled, because the implementation in `log.hpp` has changed. This is something that we want to avoid, especially when the codebase grows - you will say "thank you" to yourself!

What can we do instead? Split it into `log.hpp` and `log.cpp`. If you need to change the implementation (meaning the contents of the function, not its declaration), the compiler will only recompile the `log.cpp` file, and the compiled object file will get linked where it needs to be - without the need of recompiling the file that included `log.hpp`!

/* log.hpp */
#pragma once
#include <string>

void log(const std::string& message);

/* log.cpp */
#include "log.hpp"
#include <iostream>

void log(const std::string& message)
{
    std::cout << "[Our good logger] " << message << std::endl;
}