Finished refactoring primitives.hpp and astar.hpp and started refacto…

…ring bsp.hpp.

src/hades_extensions/include/generation/bsp.hpp

@@ -10,8 +10,8 @@
 // ----- STRUCTURES ------------------------------
 /// A binary spaced partition leaf used to generate the dungeon's rooms.
 struct Leaf {
-  inline bool operator==(const Leaf &lef) const {
-    return container == lef.container && left == lef.left && right == lef.right;
+  inline bool operator==(const Leaf &leaf) const {
+    return container == leaf.container && left == leaf.left && right == leaf.right;
   }
 
   /// The rect object that represents this leaf.
@@ -29,7 +29,7 @@ struct Leaf {
   /// Initialise the object.
   ///
   /// @param container_val - The rect object that represents this leaf.
-  explicit Leaf(Rect container_val) : container(std::make_unique<Rect>(container_val)) {}
+  explicit Leaf(const Rect &container_val) : container(std::make_unique<Rect>(container_val)) {}
 };
 
 // ----- FUNCTIONS -------------------------------
@@ -47,3 +47,8 @@ bool split(Leaf &leaf, std::mt19937 &random_generator);
 /// @param random_generator - The random generator used to generate the bsp.
 /// @return Whether the room creation was successful or not.
 bool create_room(Leaf &leaf, Grid &grid, std::mt19937 &random_generator);
+
+/// TODO: LOOK AT AND MAYBE REMOVE (OR MOVE TO COMMENT IN BSP.HPP)
+/// When creating a container, the split wall is included in the rect size,
+/// whereas, rooms don't so MIN_CONTAINER_SIZE must be bigger than
+/// MIN_ROOM_SIZE.

src/hades_extensions/include/generation/primitives.hpp

@@ -23,13 +23,15 @@ enum class TileType {
 // ----- STRUCTURES ------------------------------
 /// Represents a 2D position.
 struct Position {
-  inline bool operator==(const Position pnt) const { return x == pnt.x && y == pnt.y; }
+  inline bool operator==(const Position &position) const { return x == position.x && y == position.y; }
 
-  inline bool operator!=(const Position pnt) const { return x != pnt.x || y != pnt.y; }
+  inline bool operator!=(const Position &position) const { return x != position.x || y != position.y; }
 
-  inline Position operator+(const Position pnt) const { return {x + pnt.x, y + pnt.y}; }
+  inline Position operator+(const Position &position) const { return {x + position.x, y + position.y}; }
 
-  inline Position operator-(const Position pnt) const { return {std::abs(x - pnt.x), std::abs(y - pnt.y)}; }
+  inline Position operator-(const Position &position) const {
+    return {std::abs(x - position.x), std::abs(y - position.y)};
+  }
 
   /// The x position of the position.
   int x;
@@ -72,6 +74,7 @@ struct Grid {
   /// @param pos - The position to convert.
   /// @throws std::out_of_range - Position must be within range.
   /// @return The 1D grid position.
+  // todo: inline or move to .cpp
   [[nodiscard]] int convert_position(const Position &pos) const {
     if (pos.x < 0 || pos.x >= width || pos.y < 0 || pos.y >= height) {
       throw std::out_of_range("Position must be within range");
@@ -90,19 +93,18 @@ struct Grid {
   ///
   /// @param pos - The position to set.
   /// @throws std::out_of_range - Position must be within range.
-  inline void set_value(const Position &pos, TileType target) const { grid->at(convert_position(pos)) = target; }
+  inline void set_value(const Position &pos, const TileType target) const { grid->at(convert_position(pos)) = target; }
 };
 
-/// Represents a rectangle of any size useful for the interacting with the 2D
-/// grid.
-///
-/// When creating a container, the split wall is included in the rect size,
-/// whereas, rooms don't so MIN_CONTAINER_SIZE must be bigger than
-/// MIN_ROOM_SIZE.
+/// Represents a rectangle in 2D space.
 struct Rect {
-  inline bool operator==(const Rect &rct) const { return top_left == rct.top_left && bottom_right == rct.bottom_right; }
+  inline bool operator==(const Rect &rect) const {
+    return top_left == rect.top_left && bottom_right == rect.bottom_right;
+  }
 
-  inline bool operator!=(const Rect &rct) const { return top_left != rct.top_left || bottom_right != rct.bottom_right; }
+  inline bool operator!=(const Rect &rect) const {
+    return top_left != rect.top_left || bottom_right != rect.bottom_right;
+  }
 
   /// The top left position of the rect.
   Position top_left;
@@ -121,15 +123,14 @@ struct Rect {
 
   /// Initialise the object.
   ///
-  /// @param top_left_val - The top left position of the rect.
-  /// @param bottom_right_val - The bottom right position of the rect.
-  Rect(Position top_left_val, Position bottom_right_val)
-      : top_left(top_left_val),
-        bottom_right(bottom_right_val),
-        centre(static_cast<int>(std::round((top_left_val + bottom_right_val).x / 2.0)),
-               static_cast<int>(std::round((top_left_val + bottom_right_val).y / 2.0))),
-        width((top_left_val - bottom_right_val).x),
-        height((top_left_val - bottom_right_val).y) {}
+  /// @param top_left - The top left position of the rect.
+  /// @param bottom_right - The bottom right position of the rect.
+  Rect(const Position &top_left, const Position &bottom_right)
+      : top_left(top_left),
+        bottom_right(bottom_right),
+        centre(static_cast<int>(std::round((top_left + bottom_right).x / 2.0)), static_cast<int>(std::round((top_left + bottom_right).y / 2.0))),
+        width((top_left - bottom_right).x),
+        height((top_left - bottom_right).y) {}
 
   /// Get the Chebyshev distance to another rect.
   ///
@@ -148,20 +149,20 @@ struct Rect {
 // ----- HASHES ------------------------------
 template <>
 struct std::hash<Position> {
-  std::size_t operator()(const Position &pnt) const {
+  std::size_t operator()(const Position &position) const {
     std::size_t res = 0;
-    hash_combine(res, pnt.x);
-    hash_combine(res, pnt.y);
+    hash_combine(res, position.x);
+    hash_combine(res, position.y);
     return res;
   }
 };
 
 template <>
 struct std::hash<Rect> {
-  std::size_t operator()(const Rect &rct) const {
+  std::size_t operator()(const Rect &rect) const {
     std::size_t res = 0;
-    hash_combine(res, rct.top_left);
-    hash_combine(res, rct.bottom_right);
+    hash_combine(res, rect.top_left);
+    hash_combine(res, rect.bottom_right);
     return res;
   }
 };

src/hades_extensions/src/generation/astar.cpp

@@ -25,7 +25,7 @@ struct Neighbour {
   ///
   /// @param cost - The cost to traverse to this neighbour.
   /// @param destination - The destination position in the grid.
-  Neighbour(int cost, Position destination) : cost(cost), destination(destination) {}
+  Neighbour(int cost, const Position &destination) : cost(cost), destination(destination) {}
 };
 
 // ----- CONSTANTS ------------------------------
@@ -65,11 +65,11 @@ std::vector<Position> calculate_astar_path(const Grid &grid, const Position &sta
     }
 
     // Add all the neighbours to the heap with their cost being f = g + h:
-    //   f - The total cost of traversing the neighbour.
-    //   g - The distance between the start pair and the neighbour pair.
-    //   h - The estimated distance from the neighbour pair to the end pair (this uses the Chebyshev distance).
+    //   f - The total cost of traversing the neighbour
+    //   g - The distance between the start pair and the neighbour pair
+    //   h - The estimated distance from the neighbour pair to the end pair (this uses the Chebyshev distance)
     for (const Position &offset : INTERCARDINAL_OFFSETS) {
-      // Calculate the neighbour's position and check if its valid excluding the boundaries
+      // Move around the neighbour if it is an obstacle as they have an infinite cost
       Position neighbour = current + offset;
       if (neighbour.x < 1 || neighbour.x >= grid.width - 1 || neighbour.y < 1 || neighbour.y >= grid.height - 1) {
         continue;

src/hades_extensions/src/generation/bsp.cpp

@@ -5,10 +5,10 @@
 #include "generation/bsp.hpp"
 
 // ----- CONSTANTS ------------------------------
-#define CONTAINER_RATIO 1.25
-#define MIN_CONTAINER_SIZE 5
-#define MIN_ROOM_SIZE 4
-#define ROOM_RATIO 0.625
+const double CONTAINER_RATIO = 1.25;
+const int MIN_CONTAINER_SIZE = 5;
+const int MIN_ROOM_SIZE = 4;
+const double ROOM_RATIO = 0.625;
 
 // ----- FUNCTIONS ------------------------------
 bool split(Leaf &leaf, std::mt19937 &random_generator) {
@@ -17,15 +17,14 @@ bool split(Leaf &leaf, std::mt19937 &random_generator) {
     return false;
   }
 
-  // To determine the direction of split, we test if the width is 25% larger
-  // than the height, if so we split vertically. However, if the height is 25%
-  // larger than the width, we split horizontally. Otherwise, we split randomly
+  // To determine which direction to split it, we have three options:
+  //   1. Split vertically if the width is 25% larger than the height.
+  //   2. Split horizontally if the height is 25% larger than the width.
+  //   3. Split randomly if neither of the above is true.
   bool split_vertical;
-  if (leaf.container->width > leaf.container->height &&
-      (leaf.container->width >= CONTAINER_RATIO * leaf.container->height)) {
+  if (leaf.container->width >= CONTAINER_RATIO * leaf.container->height) {
     split_vertical = true;
-  } else if (leaf.container->height > leaf.container->width &&
-             (leaf.container->height >= CONTAINER_RATIO * leaf.container->width)) {
+  } else if (leaf.container->height >= CONTAINER_RATIO * leaf.container->width) {
     split_vertical = false;
   } else {
     std::uniform_int_distribution<> split_vertical_distribution{0, 1};

src/hades_extensions/src/generation/primitives.cpp

@@ -7,6 +7,11 @@
 
 // ----- FUNCTIONS ------------------------------
 void Rect::place_rect(Grid &grid) const {
+  // Check if the grid is big enough for the rect to be placed
+  if (bottom_right.x > grid.width || bottom_right.y > grid.height) {
+    throw std::length_error("Rect is larger than the grid");
+  }
+
   // Place the walls
   for (int y = std::max(top_left.y, 0); y < std::min(bottom_right.y + 1, grid.height); y++) {
     for (int x = std::max(top_left.x, 0); x < std::min(bottom_right.x + 1, grid.width); x++) {

src/hades_extensions/test/generation/fixtures.hpp

@@ -13,7 +13,7 @@ class GenerationFixtures : public testing::Test {
   Rect valid_rect_one{valid_position_one, valid_position_two}, valid_rect_two{valid_position_one, boundary_position},
       zero_size_rect{zero_position, zero_position};
   Leaf leaf{{{0, 0}, {19, 19}}};
-  Grid empty_grid, grid = {20, 20}, small_grid = {6, 9}, detailed_grid = {6, 9};
+  Grid empty_grid{0, 0}, grid = {20, 20}, small_grid = {6, 9}, detailed_grid = {6, 9};
   std::mt19937 random_generator;
 
   void SetUp() override {

src/hades_extensions/test/generation/test_astar.cpp

@@ -5,35 +5,84 @@
 #include "gtest/gtest.h"
 
 // Custom includes
-#include "fixtures.hpp"
 #include "generation/astar.hpp"
+#include "macros.hpp"
+
+// ----- FIXTURES ------------------------------
+/// Implements the fixture for the generation/astar.hpp tests.
+class AstarFixture : public testing::Test {
+ protected:
+  /// A 2D grid for use in testing.
+  Grid grid{6, 9};
+
+  /// A position in the middle of the grid for use in testing.
+  Position position_one{3, 7};
+
+  /// An extra position in the middle of the grid for use in testing.
+  Position position_two{4, 1};
+
+  /// A position on the edge of the grid for use in testing.
+  Position position_three{4, 0};
+
+  /// Set up the fixture for the tests.
+  void SetUp() override {
+    grid.grid = std::make_unique<std::vector<TileType>>(std::vector<TileType>{
+        TileType::Obstacle, TileType::Obstacle, TileType::Obstacle, TileType::Obstacle, TileType::Obstacle,
+        TileType::Obstacle, TileType::Obstacle, TileType::Empty,    TileType::Empty,    TileType::Empty,
+        TileType::Empty,    TileType::Obstacle, TileType::Obstacle, TileType::Empty,    TileType::Empty,
+        TileType::Empty,    TileType::Empty,    TileType::Obstacle, TileType::Obstacle, TileType::Empty,
+        TileType::Empty,    TileType::Empty,    TileType::Empty,    TileType::Obstacle, TileType::Obstacle,
+        TileType::Empty,    TileType::Empty,    TileType::Empty,    TileType::Empty,    TileType::Obstacle,
+        TileType::Obstacle, TileType::Empty,    TileType::Empty,    TileType::Empty,    TileType::Empty,
+        TileType::Obstacle, TileType::Obstacle, TileType::Empty,    TileType::Empty,    TileType::Empty,
+        TileType::Empty,    TileType::Obstacle, TileType::Obstacle, TileType::Empty,    TileType::Empty,
+        TileType::Empty,    TileType::Empty,    TileType::Obstacle, TileType::Obstacle, TileType::Obstacle,
+        TileType::Obstacle, TileType::Obstacle, TileType::Obstacle, TileType::Obstacle,
+    });
+  }
+
+  /// Add obstacles to the grid for use in testing.
+  void add_obstacles() {
+    grid.set_value({1, 3}, TileType::Obstacle);
+    grid.set_value({2, 7}, TileType::Obstacle);
+    grid.set_value({3, 2}, TileType::Obstacle);
+    grid.set_value({3, 3}, TileType::Obstacle);
+    grid.set_value({3, 6}, TileType::Obstacle);
+    grid.set_value({4, 3}, TileType::Obstacle);
+    grid.set_value({4, 6}, TileType::Obstacle);
+  }
+};
 
 // ----- TESTS ------------------------------
-TEST_F(GenerationFixtures, TestCalculateAstarPathObstaclesMiddleStart) {
-  // Test A* in a grid with obstacles
-  std::vector<Position> obstacles_result = {{1, 2}, {2, 3}, {2, 4}, {3, 5}};
-  ASSERT_EQ(calculate_astar_path(detailed_grid, valid_position_one, {1, 2}), obstacles_result);
+/// Test that A* works in a grid with no obstacles when started in the middle.
+TEST_F(AstarFixture, TestCalculateAstarPathNoObstaclesMiddleStart) {
+  std::vector<Position> no_obstacles_result{{4, 1}, {3, 2}, {2, 3}, {2, 4}, {2, 5}, {2, 6}, {3, 7}};
+  ASSERT_EQ(calculate_astar_path(grid, position_one, position_two), no_obstacles_result);
 }
 
-TEST_F(GenerationFixtures, TestCalculateAstarPathObstaclesBoundaryStart) {
-  // Test A* in a grid with obstacles
-  std::vector<Position> obstacles_result = {};
-  ASSERT_EQ(calculate_astar_path(detailed_grid, valid_position_one, boundary_position), obstacles_result);
+/// Test that A* fails in a grid with no obstacles when ended on the edge.
+TEST_F(AstarFixture, TestCalculateAstarPathNoObstaclesBoundaryEnd) {
+  std::vector<Position> no_obstacles_result;
+  ASSERT_EQ(calculate_astar_path(grid, position_one, position_three), no_obstacles_result);
 }
 
-TEST_F(GenerationFixtures, TestCalculateAstarPathNoObstaclesMiddleStart) {
-  // Test A* in a grid with no obstacles
-  std::vector<Position> no_obstacles_result = {{5, 7}, {4, 6}, {3, 5}};
-  ASSERT_EQ(calculate_astar_path(grid, valid_position_one, valid_position_two), no_obstacles_result);
+/// Test that A* works in a grid with obstacles when started in the middle.
+TEST_F(AstarFixture, TestCalculateAstarPathObstaclesMiddleStart) {
+  add_obstacles();
+  std::vector<Position> obstacles_result{{4, 1}, {3, 1}, {2, 2}, {2, 3}, {1, 4}, {1, 5}, {2, 6}, {3, 7}};
+  ASSERT_EQ(calculate_astar_path(grid, position_one, position_two), obstacles_result);
 }
 
-TEST_F(GenerationFixtures, TestCalculateAstarPathBoundaryGoal) {
-  // Test A* with a goal on the boundaries
-  std::vector<Position> boundary_result = {};
-  ASSERT_EQ(calculate_astar_path(small_grid, valid_position_one, valid_position_two), boundary_result);
+/// Test that A* fails in a grid with obstacles when ended on the edge.
+TEST_F(AstarFixture, TestCalculateAstarPathObstaclesBoundaryEnd) {
+  add_obstacles();
+  std::vector<Position> obstacles_result;
+  ASSERT_EQ(calculate_astar_path(grid, position_one, position_three), obstacles_result);
 }
 
-TEST_F(GenerationFixtures, TestCalculateAstarPathEmptyGrid) {
-  // Test A* in an empty grid
-  ASSERT_THROW(calculate_astar_path(empty_grid, valid_position_one, boundary_position), std::length_error);
+/// Test that A* fails in an empty grid.
+TEST_F(AstarFixture, TestCalculateAstarPathEmptyGrid) {
+  Grid empty_grid{0, 0};
+  ASSERT_THROW_MESSAGE(calculate_astar_path(empty_grid, position_one, position_two), std::length_error,
+                       "Grid size must be bigger than 0.")
 }