Results of today's code review session.

Makefile

@@ -32,9 +32,6 @@ debug_test: build
 debug_run: build
 	gdb --nx -x .gdbinit --batch --ex run --ex bt --ex q --args builddir/roguish.exe
 
-cover:
-	gcovr --html coverage/index.html  --gcov-ignore-errors=no_working_dir_found --exclude "scratchpad.*" --exclude "subprojects.*" --html-nested coverage/
-
 designer: build
 	powershell "cp ./builddir/designer.exe ."
 	./designer

gui.cpp

@@ -31,6 +31,7 @@ using namespace ftxui;
 using namespace components;
 
 void InventoryUI::create_render() {
+  has_border = true;
   MenuOption option;
   $inventory_box = Menu(&$menu_list, &$selected, option);
 
@@ -45,7 +46,7 @@ void InventoryUI::create_render() {
         vflow({
           paragraph($item_text) | border
         }) | flex
-    }) | border | flex;
+    }) | flex;
   });
 
   set_renderer($inventory_render);

inventory.cpp

@@ -29,4 +29,14 @@ namespace components {
     dbc::check(at < items.size(), fmt::format("inventory index {} too big", at));
     items.erase(items.begin() + at);
   }
+
+  int Inventory::item_index(std::string id) {
+    for(size_t i = 0; i < items.size(); i++) {
+      if(items[i].data["id"] == id) {
+        return i;
+      }
+    }
+
+    return -1;
+  }
 }

inventory.hpp

@@ -26,6 +26,8 @@ namespace components {
 
     InventoryItem& get(size_t at);
 
+    int item_index(std::string id);
+
     void erase_item(size_t at);
   };
 }

lights.cpp

@@ -45,9 +45,7 @@ namespace lighting {
 
 
   void LightRender::reset_light() {
-    for(auto &row : $lightmap) {
+    matrix::assign($lightmap, lighting::MIN);
-      row.assign(row.size(), lighting::MIN);
-    }
   }
 
   void LightRender::clear_light_target(const Point &at) {

main.cpp

@@ -5,7 +5,7 @@
 #include "components.hpp"
 #include "constants.hpp"
 #include "dbc.hpp"
-#include "collider.hpp"
+#include "spatialmap.hpp"
 #include "render.hpp"
 #include "save.hpp"
 #include "lights.hpp"
@@ -91,8 +91,8 @@ int main(int argc, char *argv[]) {
     configure_world(world, game_map);
   }
 
-  spatial_map collider;
+  SpatialMap collider;
-  world.set_the<spatial_map>(collider);
+  world.set_the<SpatialMap>(collider);
   System::init_positions(world);
 
   GUI gui(world, game_map);

map.cpp

@@ -49,7 +49,13 @@ Point Map::place_entity(size_t room_index) {
 
   Room &start = $rooms[room_index];
 
-  return {start.x+start.width/2, start.y+start.height/2};
+  size_t size = std::max(start.width, start.height);
+  for(matrix::box it{$walls, start.x, start.y, size}; it.next();) {
+    if(!iswall(it.x, it.y)) return {it.x, it.y};
+  }
+
+  dbc::sentinel("DIDN'T FIND AN OPEN SPACE!");
+  return {start.x, start.y};
 }
 
 bool Map::iswall(size_t x, size_t y) {

map.hpp

@@ -58,6 +58,7 @@ public:
   bool inmap(size_t x, size_t y);
   bool iswall(size_t x, size_t y);
   bool can_move(Point move_to);
+  // BUG: this isn't really neighbors anymore. Maybe move? Walk?
   bool neighbors(Point &out, bool random=false);
 
   void make_paths();

meson.build

@@ -24,7 +24,7 @@ runtests = executable('runtests', [
   'rand.cpp',
   'sound.cpp',
   'combat.cpp',
-  'collider.cpp',
+  'spatialmap.cpp',
   'ansi_parser.cpp',
   'config.cpp',
   'save.cpp',
@@ -41,7 +41,7 @@ runtests = executable('runtests', [
   'tests/fsm.cpp',
   'tests/dbc.cpp',
   'tests/map.cpp',
-  'tests/collider.cpp',
+  'tests/spatialmap.cpp',
   'tests/components.cpp',
   'tests/dinkyecs.cpp',
   'tests/ansi_parser.cpp',
@@ -67,7 +67,7 @@ roguish = executable('roguish', [
   'gui.cpp',
   'rand.cpp',
   'sound.cpp',
-  'collider.cpp',
+  'spatialmap.cpp',
   'combat.cpp',
   'systems.cpp',
   'ansi_parser.cpp',

panel.cpp

@@ -36,7 +36,6 @@ const std::wstring& Panel::to_string() {
 }
 
 void Panel::mouse_click(ftxui::Mouse::Button btn, Point pos) {
-  fmt::println("CLICK AT {},{}", pos.x, pos.y);
   ftxui::Mouse mev{
     .button=btn,
     .motion=ftxui::Mouse::Motion::Pressed,

pathing.cpp

@@ -74,17 +74,6 @@ void Pathing::clear_target(const Point &at) {
   $input[at.y][at.x] = 1;
 }
 
-void Pathing::random_flood(const Point from, std::function<void(Point at, int dnum)> cb) {
-  // quick hack to try the idea
-  matrix::each_cell it{$paths};
-  it.x = from.x;
-  it.y = from.y;
-
-  while(it.next()) {
-    cb({it.x, it.y}, $paths[it.y][it.x]);
-  }
-}
-
 bool Pathing::INVARIANT() {
   using dbc::check;
 

pathing.hpp

@@ -25,7 +25,6 @@ public:
   Matrix &paths() { return $paths; }
   Matrix &input() { return $input; }
   int distance(Point to) { return $paths[to.y][to.x];}
-  void random_flood(const Point from, std::function<void(Point at, int dnum)> cb);
 
   bool INVARIANT();
 };

render.cpp

@@ -107,7 +107,6 @@ void SFMLRender::render_grid(const std::wstring &text, sf::Color default_fg, sf:
           }
           break;
         default: {
-
           // only get a new sprite if the tile changed
           if(last_tile != tile) {
             sprite = get_text_sprite(tile);

spatialmap.cpp

@@ -1,28 +1,28 @@
-#include "collider.hpp"
+#include "spatialmap.hpp"
 #include <fmt/core.h>
 
 using namespace fmt;
 
 using DinkyECS::Entity;
 
-void spatial_map::insert(Point pos, Entity ent) {
+void SpatialMap::insert(Point pos, Entity ent) {
   table[pos] = ent;
 }
 
-void spatial_map::remove(Point pos) {
+void SpatialMap::remove(Point pos) {
   table.erase(pos);
 }
 
-void spatial_map::move(Point from, Point to, Entity ent) {
+void SpatialMap::move(Point from, Point to, Entity ent) {
   remove(from);
   insert(to, ent);
 }
 
-bool spatial_map::occupied(Point at) const {
+bool SpatialMap::occupied(Point at) const {
   return table.contains(at);
 }
 
-Entity spatial_map::get(Point at) const {
+Entity SpatialMap::get(Point at) const {
   return table.at(at);
 }
 
@@ -45,7 +45,7 @@ inline void find_neighbor(const PointEntityMap &table, EntityList &result, Point
   }
 }
 
-FoundEntities spatial_map::neighbors(Point cell, bool diag) const {
+FoundEntities SpatialMap::neighbors(Point cell, bool diag) const {
   EntityList result;
 
   // just unroll the loop since we only check four directions

spatialmap.hpp

@@ -14,9 +14,9 @@ struct FoundEntities {
   EntityList nearby;
 };
 
-class spatial_map {
+class SpatialMap {
   public:
-    spatial_map() {}
+    SpatialMap() {}
 
     void insert(Point pos, DinkyECS::Entity obj);
     void move(Point from, Point to, DinkyECS::Entity ent);

status.txt

@@ -1,6 +1,6 @@
 TODAY'S GOAL:
 
-* Make Map::place_entity handle entity overlap and also walls.
+* Things are still in walls because I +1 the x,y if they're colliding.
 * Config loader should setup the "id" based on the key to avoid errors.
 * Colision fails when you place two entities on the same square, but the init_positions adds them and one deletes the other.
 * Config needs to do asserts that the key exists

systems.cpp

@@ -3,7 +3,7 @@
 #include <string>
 #include <cmath>
 #include "rand.hpp"
-#include "collider.hpp"
+#include "spatialmap.hpp"
 #include "events.hpp"
 #include "ftxui/screen/color.hpp"
 #include "ftxui/screen/terminal.hpp"  // for SetColorSupport, Color, TrueColor
@@ -23,7 +23,6 @@ void System::lighting(DinkyECS::World &world, Map &game_map, LightRender &light,
     light.set_light_target(position.location);
   });
 
-  // BUG: some light doesn't move, can I not path those?
   light.path_light(game_map.walls());
 
   world.query<Position, LightSource>([&](const auto &ent, auto &position, auto &lightsource) {
@@ -42,7 +41,6 @@ void System::enemy_pathing(DinkyECS::World &world, Map &game_map, Player &player
     if(ent != player.entity) {
       Point out = position.location; // copy
       if(game_map.distance(out) < config.HEARING_DISTANCE) {
-        // BUG: is neighbors really the best name for this?
         game_map.neighbors(out);
         motion = { int(out.x - position.location.x), int(out.y - position.location.y)};
       }
@@ -52,7 +50,7 @@ void System::enemy_pathing(DinkyECS::World &world, Map &game_map, Player &player
 }
 
 void System::init_positions(DinkyECS::World &world) {
-  auto &collider = world.get_the<spatial_map>();
+  auto &collider = world.get_the<SpatialMap>();
 
   // BUG: instead of separate things maybe just one
   // BUG: Collision component that references what is collide
@@ -67,7 +65,7 @@ void System::init_positions(DinkyECS::World &world) {
   });
 }
 
-inline void move_entity(spatial_map &collider, Map &game_map, Position &position, Motion &motion, DinkyECS::Entity ent) {
+inline void move_entity(SpatialMap &collider, Map &game_map, Position &position, Motion &motion, DinkyECS::Entity ent) {
   Point move_to = {
     position.location.x + motion.dx,
     position.location.y + motion.dy
@@ -85,7 +83,7 @@ inline void move_entity(spatial_map &collider, Map &game_map, Position &position
 }
 
 void System::motion(DinkyECS::World &world, Map &game_map) {
-  auto &collider = world.get_the<spatial_map>();
+  auto &collider = world.get_the<SpatialMap>();
 
   world.query<Position, Motion>([&](const auto &ent, auto &position, auto &motion) {
     // don't process entities that don't move
@@ -100,7 +98,7 @@ void System::death(DinkyECS::World &world) {
   // BUG: eachother and overlap their corpse
   // BUG: maybe that can be allowed and looting just shows
   // BUG: all dead things there?
-  auto &collider = world.get_the<spatial_map>();
+  auto &collider = world.get_the<SpatialMap>();
 
   world.query<Position, Combat>([&](const auto &ent, auto &position, auto &combat) {
     // bring out yer dead
@@ -116,7 +114,7 @@ void System::death(DinkyECS::World &world) {
 }
 
 void System::collision(DinkyECS::World &world, Player &player) {
-  auto& collider = world.get_the<spatial_map>();
+  auto& collider = world.get_the<SpatialMap>();
   const auto& player_position = world.get<Position>(player.entity);
   auto& player_combat = world.get<Combat>(player.entity);
 

tests/gui.cpp

@@ -6,7 +6,7 @@
 #include "worldbuilder.hpp"
 #include "save.hpp"
 #include "systems.hpp"
-#include "collider.hpp"
+#include "spatialmap.hpp"
 #include "components.hpp"
 
 using namespace fmt;
@@ -32,8 +32,8 @@ TEST_CASE("load a basic gui run but don't loop", "[gui]") {
   world.set<Inventory>(player.entity, {5});
   world.set<LightSource>(player.entity, {6,1});
 
-  spatial_map collider;
+  SpatialMap collider;
-  world.set_the<spatial_map>(collider);
+  world.set_the<SpatialMap>(collider);
   System::init_positions(world);
 
   GUI gui(world, game_map);

tests/inventory.cpp

@@ -40,10 +40,20 @@ TEST_CASE("basic inventory test", "[inventory]") {
   auto &item1 = inventory.get(0);
   REQUIRE(item1.count == 1);
 
+  int item_at = inventory.item_index("SWORD_RUSTY");
+  REQUIRE(item_at == 0);
+
+  REQUIRE(inventory.item_index("SADFASFSADF") == -1);
+
   System::pickup(world, player, sword);
+  REQUIRE(item1.count == 2);
+
   System::pickup(world, player, sword);
+  REQUIRE(item1.count == 3);
+
   System::pickup(world, player, sword);
   REQUIRE(inventory.count() == 1);
+
   REQUIRE(item1.count == 4);
 
   inventory.decrease(0, 1);

tests/pathing.cpp

@@ -48,8 +48,4 @@ TEST_CASE("random flood", "[pathing]") {
 
   REQUIRE(pathing.INVARIANT());
   pathing.compute_paths(walls);
-
-  pathing.random_flood({1, 2}, [&](Point at, int dnum) {
-      println("FLOOD: at={},{}, dnum={}", at.x, at.y, dnum);
-  });
 }

tests/spatialmap.cpp

@@ -1,13 +1,13 @@
 #include <catch2/catch_test_macros.hpp>
 #include <fmt/core.h>
 #include <string>
-#include "collider.hpp"
+#include "spatialmap.hpp"
 #include "dinkyecs.hpp"
 
 using DinkyECS::Entity;
 using namespace fmt;
 
-EntityList require_found(const spatial_map& collider, Point at, bool diag, size_t expect_size) {
+EntityList require_found(const SpatialMap& 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);
@@ -21,7 +21,7 @@ TEST_CASE("confirm basic collision operations", "[collision]") {
   Entity player = world.entity();
   Entity enemy = world.entity();
 
-  spatial_map collider;
+  SpatialMap collider;
   collider.insert({11,11}, player);
   collider.insert({21,21}, enemy);
 
@@ -70,7 +70,7 @@ TEST_CASE("confirm multiple entities moving", "[collision]") {
   Entity e2 = world.entity();
   Entity e3 = world.entity();
 
-  spatial_map collider;
+  SpatialMap collider;
   collider.insert({11,11}, player);
   collider.insert({10,10}, e2);
   collider.insert({11,10}, e3);
@@ -93,7 +93,7 @@ TEST_CASE("test edge cases that might crash", "[collision]") {
   Entity player = world.entity();
   Entity enemy = world.entity();
 
-  spatial_map collider;
+  SpatialMap collider;
   collider.insert({0,0}, player);
 
   Point enemy_at = {1, 0};
@@ -115,7 +115,7 @@ TEST_CASE("check all diagonal works", "[collision]") {
   Entity player = world.entity();
   Entity enemy = world.entity();
 
-  spatial_map collider;
+  SpatialMap collider;
   Point player_at = {1,1};
   collider.insert(player_at, player);