Better unit test for the collision system.

collider.cpp

@@ -1,4 +1,7 @@
 #include "collider.hpp"
+#include <fmt/core.h>
+
+using namespace fmt;
 
 using DinkyECS::Entity;
 
@@ -19,8 +22,20 @@ bool SpatialHashTable::occupied(Point at) const {
   return table.contains(at);
 }
 
-inline void find_neighbor(const PointEntityMap &table, FoundList &result, Point at) {
-  auto it = table.find(at);
+/*
+ * Avoid doing work by using the dy,dx and confirming that
+ * at.x or at.y is > 0.  If either is 0 then there can't be
+ * a neighbor since that's out of bounds.
+ */
+inline void find_neighbor(const PointEntityMap &table, FoundList &result, Point at, int dy, int dx) {
+  // don't bother checking for cells out of bounds
+  if((dx < 0 && at.x <= 0) || (dy < 0 && at.y <= 0)) {
+    return;
+  }
+
+  Point cell = {at.x + dx, at.y + dy};
+
+  auto it = table.find(cell);
   if (it != table.end()) {
     result.insert(result.end(), it->second);
   }
@@ -31,16 +46,16 @@ std::tuple<bool, FoundList> SpatialHashTable::neighbors(Point cell, bool diag) c
 
   // just unroll the loop since we only check four directions
   // this also solves the problem that it was detecting that the cell was automatically included as a "neighbor" but it's not
-  find_neighbor(table, result, {cell.x, cell.y+1}); // north
-  find_neighbor(table, result, {cell.x, cell.y-1}); // south
-  find_neighbor(table, result, {cell.x+1, cell.y}); // east
-  find_neighbor(table, result, {cell.x-1, cell.y}); // west
-  find_neighbor(table, result, {cell.x+1, cell.y-1}); // south east
+  find_neighbor(table, result, cell, 0, 1); // north
+  find_neighbor(table, result, cell, 0, -1); // south
+  find_neighbor(table, result, cell, 1, 0); // east
+  find_neighbor(table, result, cell, -1, 0); // west
 
   if(diag) {
-    find_neighbor(table, result, {cell.x-1, cell.y-1}); // south west
-    find_neighbor(table, result, {cell.x+1, cell.y+1}); // north east
-    find_neighbor(table, result, {cell.x-1, cell.y+1}); // north west
+    find_neighbor(table, result, cell, 1, -1); // south east
+    find_neighbor(table, result, cell, -1, -1); // south west
+    find_neighbor(table, result, cell, 1, 1); // north east
+    find_neighbor(table, result, cell, -1, 1); // north west
   }
 
   return std::tuple(!result.empty(), result);

status.txt

@@ -4,6 +4,4 @@ TODO:
 * Lua integration?
 * Text is not actually cleared when rendered either in FTXUI or SFML.
 
-* Work on collision detection with either a coordinate map or morton codes.
 * Bring back sounds, check out SoLoud.
-* getNearby does size_t - int

tests/collider.cpp

@@ -7,55 +7,57 @@
 using DinkyECS::Entity;
 using namespace fmt;
 
+FoundList require_found(const SpatialHashTable& collider, Point at, bool diag, size_t expect_size) {
+  println("TEST require_found at={},{}", at.x, at.y);
+  auto [found, nearby] = collider.neighbors(at, diag);
+  REQUIRE(found == true);
+  REQUIRE(nearby.size() == expect_size);
+  return nearby;
+}
+
+
 TEST_CASE("confirm basic collision operations", "[collision]") {
   DinkyECS::World world;
   Entity player = world.entity();
   Entity enemy = world.entity();
 
-  SpatialHashTable coltable;
-  coltable.insert({11,11}, player);
-  coltable.insert({21,21}, enemy);
+  SpatialHashTable collider;
+  collider.insert({11,11}, player);
+  collider.insert({21,21}, enemy);
 
   { // not found
-    auto [found, nearby] = coltable.neighbors({1,1});
+    auto [found, nearby] = collider.neighbors({1,1});
     REQUIRE(!found);
     REQUIRE(nearby.empty());
   }
 
-  { // found
-    auto [found, nearby] = coltable.neighbors({10,10}, true);
-
-    REQUIRE(found);
-    REQUIRE(nearby[0] == player);
-  }
+  // found
+  FoundList nearby = require_found(collider, {10,10}, true, 1);
+  REQUIRE(nearby[0] == player);
 
   { // removed
-    coltable.remove({11,11});
-    auto [found, nearby] = coltable.neighbors({10,10}, true);
+    collider.remove({11,11});
+    auto [found, nearby] = collider.neighbors({10,10}, true);
     REQUIRE(!found);
     REQUIRE(nearby.empty());
   }
 
-  coltable.insert({11,11}, player); // setup for the move test
-
-  { // moving
-    coltable.move({11,11}, {12, 12}, player);
-    auto [found, nearby] = coltable.neighbors({10,10}, true);
+  collider.insert({11,11}, player); // setup for the move test
+  { // moving, not found
+    collider.move({11,11}, {12, 12}, player);
+    auto [found, nearby] = collider.neighbors({10,10}, true);
     REQUIRE(!found);
     REQUIRE(nearby.empty());
   }
 
-  { // find it after move
-    auto [found, nearby] = coltable.neighbors({11,11}, true);
-    REQUIRE(found);
-    REQUIRE(nearby[0] == player);
-  }
+  nearby = require_found(collider, {11,11}, true, 1);
+  REQUIRE(nearby[0] == player);
 
-  {
-    REQUIRE(coltable.occupied({12,12}));
-    REQUIRE(coltable.occupied({21,21}));
-    REQUIRE(!coltable.occupied({1,10}));
-  }
+
+  // confirm occupied works
+  REQUIRE(collider.occupied({12,12}));
+  REQUIRE(collider.occupied({21,21}));
+  REQUIRE(!collider.occupied({1,10}));
 }
 
 
@@ -66,27 +68,68 @@ TEST_CASE("confirm multiple entities moving", "[collision]") {
   Entity e2 = world.entity();
   Entity e3 = world.entity();
 
-  SpatialHashTable coltable;
-  coltable.insert({11,11}, player);
-  coltable.insert({10,10}, e2);
-  coltable.insert({11,10}, e3);
-  coltable.insert({21,21}, e1);
-
-  { // find e3 and e2
-    auto [found, nearby] = coltable.neighbors({11, 11}, true);
-    REQUIRE(found);
-    REQUIRE(nearby.size() == 2);
-    // BUG: replace this with std::find/std::search
-    REQUIRE(nearby[0] == e3);
-    REQUIRE(nearby[1] == e2);
-  }
+  SpatialHashTable collider;
+  collider.insert({11,11}, player);
+  collider.insert({10,10}, e2);
+  collider.insert({11,10}, e3);
+  collider.insert({21,21}, e1);
+
+  FoundList nearby = require_found(collider, {11,11}, false, 1);
+  REQUIRE(nearby[0] == e3);
+
+  nearby = require_found(collider, {11,11}, true, 2);
+  REQUIRE(nearby[0] == e3);
+  REQUIRE(nearby[1] == e2);
+
+  collider.move({11,11}, {20,20}, player);
+  nearby = require_found(collider, {20,20}, true, 1);
+  REQUIRE(nearby[0] == e1);
+}
+
+TEST_CASE("test edge cases that might crash", "[collision]") {
+  DinkyECS::World world;
+  Entity player = world.entity();
+  Entity enemy = world.entity();
+
+  SpatialHashTable collider;
+  collider.insert({0,0}, player);
+
+  Point enemy_at = {1, 0};
+  collider.insert(enemy_at, enemy);
+
+  FoundList nearby = require_found(collider, {0,0}, true, 1);
+
+  collider.move({1,0}, {1,1}, enemy);
+  nearby = require_found(collider, {0,0}, true, 1);
+  REQUIRE(nearby[0] == enemy);
+
+  collider.move({1,1}, {0,1}, enemy);
+  nearby = require_found(collider, {0,0}, true, 1);
+  REQUIRE(nearby[0] == enemy);
+}
+
+TEST_CASE("check all diagonal works", "[collision]") {
+  DinkyECS::World world;
+  Entity player = world.entity();
+  Entity enemy = world.entity();
+
+  SpatialHashTable collider;
+  Point player_at = {1,1};
+  collider.insert(player_at, player);
+
+  Point enemy_at = {1, 0};
+  collider.insert(enemy_at, enemy);
+
+  for(size_t x = 0; x <= 2; x++) {
+    for(size_t y = 0; y <= 2; y++) {
+      if(enemy_at.x == player_at.x && enemy_at.y == player_at.y) continue; // skip player spot
+      FoundList nearby = require_found(collider, player_at, true, 1);
+      REQUIRE(nearby[0] == enemy);
 
-  coltable.move({11,11}, {20,20}, player);
-  { // should only find the e1
-    auto [found, nearby] = coltable.neighbors({20,20}, true);
-    REQUIRE(found);
-    REQUIRE(nearby.size() == 1);
-    // BUG: replace this with std::find/std::search
-    REQUIRE(nearby[0] == e1);
+      // move the enemy to a new spot around the player
+      Point move_to = {enemy_at.x + x, enemy_at.y + y};
+      collider.move(enemy_at, move_to, enemy);
+      enemy_at = move_to;
+    }
   }
 }