Optimised Registry::find_components by using std::views::filter and s…

…td::views::transform which returns an iterator instead of passing the vector.

Fixed security issue with test_map.cpp where multiplying two ints and them implicitly converting them to size_t could result in an arithmetic overflow.

The headless pyglet problem still exists however, but I'm not sure why.

src/hades_extensions/include/game_objects/registry.hpp

@@ -149,26 +149,21 @@ class Registry {
   /// 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.
+  /// @return The game objects that have the required components.
   template <typename... Ts>
-  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;
-
-    // Iterate over all game objects
-    for (const auto &[game_object_id, game_object_components] : game_objects_) {
-      // Check if the game object has all the components using a fold expression
-      if (!(has_component(game_object_id, typeid(Ts)) && ...)) {
-        continue;
-      }
-
-      // 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)))...)};
-      components.emplace_back(game_object_id, components_result);
-    }
-
-    // Return the components
-    return components;
+  auto find_components() const {
+    // Use ranges::filter to filter out the game objects that have all the components then use ranges::transform to get
+    // only the game object ID and the required components
+    return game_objects_ | std::views::filter([this](const auto &game_object) {
+             const auto &[game_object_id, game_object_components] = game_object;
+             return (has_component(game_object_id, typeid(Ts)) && ...);
+           }) |
+           std::views::transform([](const auto &game_object) {
+             const auto &[game_object_id, game_object_components] = game_object;
+             return std::make_tuple(
+                 game_object_id,
+                 std::make_tuple(std::static_pointer_cast<Ts>(game_object_components.at(typeid(Ts)))...));
+           });
   }
 
   /// Add a system to the registry.

src/hades_extensions/src/game_objects/systems/armour_regen.cpp

@@ -10,8 +10,8 @@ constexpr int ARMOUR_REGEN_AMOUNT{1};
 // ----- FUNCTIONS ------------------------------
 void ArmourRegenSystem::update(const double delta_time) const {
   // Update the time since the last armour regen then check if the armour should be regenerated
-  for (auto &[_, component_tuple] : get_registry()->find_components<Armour, ArmourRegen>()) {
-    auto &[armour, armour_regen]{component_tuple};
+  for (const auto &[_, component_tuple] : get_registry()->find_components<Armour, ArmourRegen>()) {
+    const auto &[armour, armour_regen]{component_tuple};
     armour_regen->time_since_armour_regen += delta_time;
     if (armour_regen->time_since_armour_regen >= armour_regen->get_value()) {
       armour->set_value(armour->get_value() + ARMOUR_REGEN_AMOUNT);

src/hades_extensions/src/game_objects/systems/effects.cpp

@@ -6,7 +6,7 @@
 
 // ----- FUNCTIONS ------------------------------
 void EffectSystem::update(const double delta_time) const {
-  for (auto &[game_object_id, component_tuple] : get_registry()->find_components<StatusEffects>()) {
+  for (const auto &[game_object_id, component_tuple] : get_registry()->find_components<StatusEffects>()) {
     // Create a vector to store the expired status effects
     auto &applied_effects{std::get<0>(component_tuple)->applied_effects};
     std::vector<StatusEffectType> expired_status_effects;

src/hades_extensions/src/game_objects/systems/movements.cpp

@@ -16,7 +16,7 @@ constexpr int MAX_DEGREE{360};
 // ----- FUNCTIONS ------------------------------
 void FootprintSystem::update(const double delta_time) const {
   // Update the time since the last footprint then check if a new footprint should be created
-  for (auto &[game_object_id, component_tuple] : get_registry()->find_components<Footprints>()) {
+  for (const auto &[game_object_id, component_tuple] : get_registry()->find_components<Footprints>()) {
     const auto footprints{std::get<0>(component_tuple)};
     footprints->time_since_last_footprint += delta_time;
     if (footprints->time_since_last_footprint < FOOTPRINT_INTERVAL) {
@@ -100,7 +100,7 @@ auto SteeringMovementSystem::calculate_force(const GameObjectID game_object_id)
 void SteeringMovementSystem::update_path_list(const GameObjectID target_game_object_id,
                                               const std::deque<Vec2d> &footprints) const {
   // Update the path list for all SteeringMovement components that have the correct target ID
-  for (auto &[game_object_id, component_tuple] : get_registry()->find_components<SteeringMovement>()) {
+  for (const auto &[game_object_id, component_tuple] : get_registry()->find_components<SteeringMovement>()) {
     const auto steering_movement{std::get<0>(component_tuple)};
     if (steering_movement->target_id != target_game_object_id) {
       continue;

src/hades_extensions/tests/game_objects/test_registry.cpp

@@ -79,8 +79,8 @@ TEST_F(RegistryFixture, TestRegistryEmptyGameObject) {
   ASSERT_THROW_MESSAGE(
       (registry.get_component(0, typeid(TestGameObjectComponentTwo))), RegistryError,
       "The game object `0` is not registered with the registry or does not have the required component.")
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentOne>().size(), {});
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentTwo>().size(), {});
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne>()), 0);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentTwo>()), 0);
   ASSERT_EQ(registry.get_walls().size(), 0);
   ASSERT_THROW_MESSAGE(registry.get_kinematic_object(0), RegistryError,
                        "The game object `0` is not registered with the registry or is not kinematic.")
@@ -97,11 +97,10 @@ TEST_F(RegistryFixture, TestRegistryGameObjectComponents) {
       {std::make_shared<TestGameObjectComponentOne>(), std::make_shared<TestGameObjectComponentTwo>(test_list)});
   ASSERT_NE(registry.get_component<TestGameObjectComponentOne>(0), nullptr);
   ASSERT_NE(registry.get_component(0, typeid(TestGameObjectComponentTwo)), nullptr);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentOne>().size(), 1);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentTwo>().size(), 1);
-  const auto multiple_result_one{
-      registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>().size()};
-  ASSERT_EQ(multiple_result_one, 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne>()), 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentTwo>()), 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>()),
+            1);
 
   // Test that deleting the game object works correctly
   registry.delete_game_object(0);
@@ -111,11 +110,10 @@ TEST_F(RegistryFixture, TestRegistryGameObjectComponents) {
   ASSERT_THROW_MESSAGE(
       (registry.get_component(0, typeid(TestGameObjectComponentTwo))), RegistryError,
       "The game object `0` is not registered with the registry or does not have the required component.")
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentOne>().size(), 0);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentTwo>().size(), 0);
-  const auto multiple_result_two{
-      registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>().size()};
-  ASSERT_EQ(multiple_result_two, 0);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne>()), 0);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentTwo>()), 0);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>()),
+            0);
 }
 
 /// Test that a kinematic game object is added to the registry correctly.
@@ -141,19 +139,17 @@ TEST_F(RegistryFixture, TestRegistryMultipleGameObjects) {
   ASSERT_EQ(registry.create_game_object({std::make_shared<TestGameObjectComponentOne>(),
                                          std::make_shared<TestGameObjectComponentTwo>(test_list)}),
             1);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentOne>().size(), 2);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentTwo>().size(), 1);
-  const auto multiple_result_one{
-      registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>().size()};
-  ASSERT_EQ(multiple_result_one, 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne>()), 2);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentTwo>()), 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>()),
+            1);
 
   // Test that deleting the first game object works correctly
   registry.delete_game_object(0);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentOne>().size(), 1);
-  ASSERT_EQ(registry.find_components<TestGameObjectComponentTwo>().size(), 1);
-  const auto multiple_result_two{
-      registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>().size()};
-  ASSERT_EQ(multiple_result_two, 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne>()), 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentTwo>()), 1);
+  ASSERT_EQ(std::ranges::distance(registry.find_components<TestGameObjectComponentOne, TestGameObjectComponentTwo>()),
+            1);
 }
 
 /// Test that a game object with duplicate components is added to the registry correctly.

src/hades_extensions/tests/generation/test_map.cpp

@@ -228,7 +228,7 @@ TEST_F(MapFixture, TestMapCreateHallwaysNoConnections) {
 /// Test that running cellular automata on a grid with all empty tiles doesn't do anything.
 TEST_F(MapFixture, TestMapRunCellularAutomataAllEmpty) {
   run_cellular_automata(grid);
-  ASSERT_EQ(*grid.grid, std::vector(grid.width * grid.height, TileType::Empty));
+  ASSERT_EQ(*grid.grid, std::vector(static_cast<std::size_t>(grid.width * grid.height), TileType::Empty));
 }
 
 /// Test that running cellular automata on a grid with all floor tiles sets the edges to walls.
@@ -257,7 +257,7 @@ TEST_F(MapFixture, TestMapRunCellularAutomataAllWalls) {
     }
   }
   run_cellular_automata(grid);
-  ASSERT_EQ(*grid.grid, std::vector(grid.width * grid.height, TileType::Empty));
+  ASSERT_EQ(*grid.grid, std::vector(static_cast<std::size_t>(grid.width * grid.height), TileType::Empty));
 }
 
 /// Test that running cellular automata on a grid with mixed floor and wall tiles works correctly.