Skip to content

Commit

Permalink
Fixed the remaining clang-tidy errors and warnings.
Browse files Browse the repository at this point in the history
Changed and removed some comments, definitions, and constructors, so they're more efficient and easier to use.

Fixed coverage configuration. It should now generate the coverage files for gcov (which still needs to be setup).

Used list initialisation wherever possible.

Reformatted .clang-tidy.
  • Loading branch information
JackAshwell11 committed Nov 6, 2023
1 parent 8147be7 commit 388fe95
Show file tree
Hide file tree
Showing 41 changed files with 633 additions and 566 deletions.
34 changes: 33 additions & 1 deletion src/hades_extensions/.clang-tidy
Original file line number Diff line number Diff line change
@@ -1,2 +1,34 @@
Checks: "-*,clang-analyzer-*,bugprone-*,modernize-*,cppcoreguidelines-*,readability-*,misc-*,google-*,performance-*,hicpp-*,cert-*"
Checks: "
bugprone-*,
-bugprone-easily-swappable-parameters,
cert-*,
-cert-err58-cpp
clang-analyzer-*,
cppcoreguidelines-*,
google-*,
hicpp-*,
misc-*,
modernize-*,
performance-*,
portability-*,
readability-*,
"
WarningsAsErrors: "*"
CheckOptions:
- key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
value: 1
- key: readability-identifier-length.IgnoredParameterNames
value: "^[ijkxy_]$"
- key: readability-identifier-length.IgnoredLoopCounterNames
value: "^[ijkxy_]$"
19 changes: 11 additions & 8 deletions src/hades_extensions/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_EXTENSIONS OFF)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
set(CMAKE_COMPILE_WARNING_AS_ERROR ON)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

# Define the module names and initialise the project
Expand All @@ -18,36 +17,40 @@ project(${PY_MODULE} LANGUAGES CXX)
# Enable coverage if supported by the compiler
if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
message(STATUS "Compiler is GNU or Clang, coverage enabled")
set(CMAKE_CXX_FLAGS "-fprofile-arcs -ftest-coverage --coverage")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -O0 --coverage")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --coverage")
else ()
message(STATUS "Compiler is not GNU or Clang, coverage disabled")
endif ()

# TODO: Clang-tidy just fails the build for some reason
## Enable clang-tidy if found
#find_program(CLANG_TIDY_EXE NAMES "clang-tidy")
#if (CLANG_TIDY_EXE)
# message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}")
# set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXE}")
#else ()
# message(STATUS "clang-tidy not found")
# message(STATUS "No clang-tidy found")
#endif ()
#

## Enable cppcheck if found
#find_program(CPPCHECK_EXE NAMES "cppcheck")
#if (CPPCHECK_EXE)
# message(STATUS "Found cppcheck: ${CPPCHECK_EXE}")
# set(CMAKE_CXX_CPPCHECK "${CPPCHECK_EXE}")
# list(APPEND CMAKE_CXX_CPPCHECK "--enable=all" "--suppress=missingInclude" "--suppress=missingIncludeSystem")
#else ()
# message(STATUS "cppcheck not found")
# message(STATUS "No cppcheck found")
#endif ()

# Add the subdirectories for the different parts of the project
include(FetchContent)
add_subdirectory(src)
add_subdirectory(test)

# TODO: Go over all clang-tidy and cppcheck rules and decide which ones to enable
# TODO: Get coverage fully working (could get temporary github repo to test it)
# TODO: Try catch2
# TODO: Try out vcpkg (idk)

# TODO: Should clang-tidy run in CI and local or just CI or just local? If so, should a new action be made or integrated into an existing one?
# TODO: Add stub files and modify Python game to use new ECS
# TODO: Should clang-tidy and cppcheck only be run in CI?
# TODO: Could have yaml/toml/json templates for game objects. Could have it specify tile size, image, attributes, components, etc and they're loaded on startup. Could maybe have this even as public api
87 changes: 43 additions & 44 deletions src/hades_extensions/include/game_objects/registry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,72 +21,70 @@ using ObjectType = std::type_index;
// Add a forward declaration for the registry class
class Registry;

// todo: see if these need to be virtual or are needed

/// The base class for all components.
struct ComponentBase {
/// The virtual destructor.
virtual ~ComponentBase() = default;
/// The copy assignment operator.
///
/// @param other - The other component to copy.
auto operator=(const ComponentBase &) -> ComponentBase & = default;

/// The move assignment operator.
///
/// @param other - The other component to move.
auto operator=(ComponentBase &&) -> ComponentBase & = default;

/// The default constructor.
ComponentBase() = default;

/// The virtual destructor.
virtual ~ComponentBase() = default;

/// The copy constructor.
///
/// @param other - The other component to copy.
ComponentBase(const ComponentBase &) = default;

/// The copy assignment operator.
///
/// @param other - The other component to copy.
ComponentBase &operator=(const ComponentBase &) = default;

/// The move constructor.
///
/// @param other - The other component to move.
ComponentBase(ComponentBase &&) = default;

/// The move assignment operator.
///
/// @param other - The other component to move.
ComponentBase &operator=(ComponentBase &&) = default;
};

/// The base class for all systems.
class SystemBase {
public:
/// The virtual destructor.
virtual ~SystemBase() = default;
/// The copy assignment operator.
///
/// @param other - The other system to copy.
auto operator=(const SystemBase &) -> SystemBase & = default;

/// The move assignment operator.
///
/// @param other - The other system to move.
auto operator=(SystemBase &&) -> SystemBase & = default;

/// Initialise the object.
///
/// @param registry - The registry that manages the game objects, components, and systems.
explicit SystemBase(Registry *registry) : registry(registry) {}

/// The virtual destructor.
virtual ~SystemBase() = default;

/// The copy constructor.
///
/// @param other - The other system to copy.
SystemBase(const SystemBase &) = default;

/// The copy assignment operator.
///
/// @param other - The other system to copy.
SystemBase &operator=(const SystemBase &) = default;

/// The move constructor.
///
/// @param other - The other system to move.
SystemBase(SystemBase &&) = default;

/// The move assignment operator.
///
/// @param other - The other system to move.
SystemBase &operator=(SystemBase &&) = default;

/// Get the registry that manages the game objects, components, and systems.
///
/// @return The registry that manages the game objects, components, and systems.
[[nodiscard]] inline Registry *get_registry() const { return registry; }
[[nodiscard]] inline auto get_registry() const -> Registry * { return registry; }

/// Process update logic for a system.
///
Expand Down Expand Up @@ -118,15 +116,15 @@ class RegistryException : public std::runtime_error {
///
/// @param value - The value to convert to a string.
/// @return The value as a string.
static inline std::string to_string(const char *value) { return {value}; }
static inline auto to_string(const char *value) -> std::string { return {value}; }

/// Convert a value to a string.
///
/// @tparam T - The type of value to convert to a string.
/// @param value - The value to convert to a string.
/// @return The value as a string.
template <typename T>
static inline std::string to_string(const T &value) {
static inline auto to_string(const T &value) -> std::string {
return std::to_string(value);
}
};
Expand All @@ -139,7 +137,7 @@ class Registry {
///
/// @param kinematic - Whether the game object should have a kinematic object or not.
/// @return The game object ID.
GameObjectID create_game_object(bool kinematic = false);
auto create_game_object(bool kinematic = false) -> GameObjectID;

/// Delete a game object.
///
Expand All @@ -152,7 +150,8 @@ class Registry {
/// @param game_object_id - The game object ID.
/// @param component_type - The type of component to check for.
/// @return Whether the game object has the component or not.
[[nodiscard]] inline bool has_component(GameObjectID game_object_id, ObjectType component_type) const {
[[nodiscard]] inline auto has_component(const GameObjectID game_object_id, const ObjectType component_type) const
-> bool {
return components_.contains(component_type) && components_.at(component_type).contains(game_object_id);
}

Expand All @@ -161,7 +160,7 @@ class Registry {
/// @param game_object_id - The game object ID.
/// @param components - The components to add to the game object.
/// @throws RegistryException - If the game object is not registered.
void add_components(GameObjectID game_object_id, const std::vector<std::shared_ptr<ComponentBase>> &components);
void add_components(GameObjectID game_object_id, const std::vector<std::shared_ptr<ComponentBase>> &&components);

/// Get a component from the registry.
///
Expand All @@ -171,7 +170,7 @@ class Registry {
/// component.
/// @return The component from the registry.
template <typename T>
inline std::shared_ptr<T> get_component(GameObjectID game_object_id) const {
inline auto get_component(const GameObjectID game_object_id) const -> std::shared_ptr<T> {
return std::static_pointer_cast<T>(get_component(game_object_id, typeid(T)));
}

Expand All @@ -180,15 +179,15 @@ class Registry {
/// @param game_object_id - The game object ID.
/// @param component_type - The type of component to get.
/// @return The component from the registry.
[[nodiscard]] std::shared_ptr<ComponentBase> get_component(GameObjectID game_object_id,
ObjectType component_type) const;
[[nodiscard]] auto get_component(GameObjectID game_object_id, ObjectType component_type) const
-> std::shared_ptr<ComponentBase>;

/// Find all the game objects that have the required components.
///
/// @tparam Ts - The types of components to find.
/// @return A vector of tuples containing the game object ID and the required components.
template <typename... Ts>
std::vector<std::tuple<GameObjectID, std::tuple<std::shared_ptr<Ts>...>>> find_components() const {
auto find_components() const -> std::vector<std::tuple<GameObjectID, std::tuple<std::shared_ptr<Ts>...>>> {
// Create a vector of tuples to store the components
std::vector<std::tuple<GameObjectID, std::tuple<std::shared_ptr<Ts>...>>> components;

Expand All @@ -200,7 +199,7 @@ class Registry {
}

// Game object has all the components, so cast them to T and add them to the vector
auto components_result = std::make_tuple(std::static_pointer_cast<Ts>(game_object_components.at(typeid(Ts)))...);
auto components_result{std::make_tuple(std::static_pointer_cast<Ts>(game_object_components.at(typeid(Ts)))...)};
components.emplace_back(game_object_id, components_result);
}

Expand All @@ -215,7 +214,7 @@ class Registry {
template <typename SystemType>
void add_system() {
// Check if the system is already registered
const std::type_info &system_type = typeid(SystemType);
const std::type_info &system_type{typeid(SystemType)};
if (systems_.contains(system_type)) {
throw RegistryException("system", system_type.name(), "is already registered with the registry");
}
Expand All @@ -230,9 +229,9 @@ class Registry {
/// @throws RegistryException - If the system is not registered.
/// @return The system.
template <typename T>
std::shared_ptr<T> find_system() const {
auto find_system() const -> std::shared_ptr<T> {
// Check if the system is registered
auto system_result = systems_.find(typeid(T));
auto system_result{systems_.find(typeid(T))};
if (system_result == systems_.end()) {
throw RegistryException("system", typeid(T).name());
}
Expand All @@ -244,7 +243,7 @@ class Registry {
/// Update all the systems in the registry.
///
/// @param delta_time - The time interval since the last time the function was called.
inline void update(double delta_time) const {
inline void update(const double delta_time) const {
for (const auto &[_, system] : systems_) {
system->update(delta_time);
}
Expand All @@ -256,7 +255,7 @@ class Registry {
/// @throws RegistryException - If the game object is not registered or if the game object does not have a kinematic
/// object.
/// @return The kinematic object.
[[nodiscard]] std::shared_ptr<KinematicObject> get_kinematic_object(GameObjectID game_object_id) const;
[[nodiscard]] auto get_kinematic_object(GameObjectID game_object_id) const -> std::shared_ptr<KinematicObject>;

/// Add a wall to the registry.
///
Expand All @@ -266,11 +265,11 @@ class Registry {
/// Get the walls in the registry.
///
/// @return The walls in the registry.
[[nodiscard]] inline const std::unordered_set<Vec2d> &get_walls() const { return walls_; }
[[nodiscard]] inline auto get_walls() const -> const std::unordered_set<Vec2d> & { return walls_; }

private:
/// The next game object ID to use.
GameObjectID next_game_object_id_ = 0;
GameObjectID next_game_object_id_{0};

/// The components registered with the registry.
std::unordered_map<ObjectType, std::unordered_set<GameObjectID>> components_;
Expand Down
Loading

0 comments on commit 388fe95

Please sign in to comment.