Did a full code review to identify things to fix and either fixed them or noted BUG where I should come back.

config.hpp

@@ -7,7 +7,7 @@ using namespace nlohmann;
 
 struct Config {
   json $config;
-  std::string $src_path = "./config.json";
+  std::string $src_path;
 
   Config(const std::string src_path);
 

dinkyecs.hpp

@@ -78,7 +78,6 @@ namespace DinkyECS {
     template <typename Comp>
       bool has(Entity ent) {
         EntityMap &map = entity_map_for<Comp>();
-        // use .at for bounds checking
         return map.contains(ent);
       }
 
@@ -116,7 +115,6 @@ namespace DinkyECS {
         EventQueue &queue = queue_map_for<Comp>();
         Event evt = queue.front();
         queue.pop();
-        // I could use tie here to auto extract the any
         return evt;
       }
 

gui.cpp

@@ -179,6 +179,7 @@ bool GUI::handle_ui_events() {
 
   while($renderer.poll_event(event)) {
     if(event.type == sf::Event::Closed) {
+      // BUG: This should call a GUI::shutdown so I can do saves and stuff.
       $renderer.close();
     } else if(event.type ==  sf::Event::KeyPressed) {
 
@@ -209,8 +210,11 @@ bool GUI::handle_ui_events() {
       sf::Vector2i pos = MOUSE::getPosition($renderer.$window);
       Mouse mev;
       mev.button = Mouse::Button::Left,
-      mev.x=pos.x / $renderer.$ui_bounds.width; // this needs to be in...panel coordinates?
+      // BUG: renderer should have a function that handles mouse coordinates
+      // BUG: optionally maybe have it in panel? Seems to work though.
+      mev.x=pos.x / $renderer.$ui_bounds.width;
       mev.y=pos.y / $renderer.$ui_bounds.height;
+      // BUG: maybe also handle mouse motion events?
       $status_ui.$component->OnEvent(Event::Mouse("", mev));
     }
   }
@@ -230,8 +234,8 @@ void GUI::run_systems() {
 
 void GUI::shake() {
   for(int i = 0; i < 10; ++i) {
-    int x = Random::uniform<int>(-20,20);
+    int x = Random::uniform<int>(-10,10);
-    int y = Random::uniform<int>(-20,20);
+    int y = Random::uniform<int>(-10,10);
     // add x/y back to draw screen
     $renderer.draw($map_view, x, y);
     $renderer.display();

lights.cpp

@@ -15,7 +15,7 @@ namespace lighting {
 
     for(size_t y = min.y; y <= max.y; ++y) {
       auto &light_row = $lightmap[y];
-      auto &path_row = $light.$paths[y];
+      auto &path_row = $paths.$paths[y];
 
       for(size_t x = min.x; x <= max.x; ++x) {
         if(path_row[x] != UNPATH) {
@@ -28,7 +28,7 @@ namespace lighting {
     const int wall_light = source.strength + WALL_LIGHT_LEVEL;
     for(auto point : has_light) {
       for(int j = -1;point.y+j >= 0 && j <= 1 && point.y+j < $height; j++) {
-        auto &path_row = $light.$paths[point.y+j];
+        auto &path_row = $paths.$paths[point.y+j];
         auto &light_row = $lightmap[point.y+j];
 
         for(int i = -1; point.x+i >= 0 && i <= 1 && point.x+i < $width; i++) {
@@ -41,7 +41,7 @@ namespace lighting {
   }
 
   int LightRender::light_level(int level, size_t x, size_t y) {
-    size_t at = level + $light.$paths[y][x];
+    size_t at = level + $paths.$paths[y][x];
     int cur_level = $lightmap[y][x];
     int new_level = at < lighting::LEVELS.size() ? lighting::LEVELS[at] : lighting::MIN;
     return cur_level < new_level ? new_level : cur_level;
@@ -55,15 +55,15 @@ namespace lighting {
   }
 
   void LightRender::clear_light_target(const Point &at) {
-    $light.clear_target(at);
+    $paths.clear_target(at);
   }
 
   void LightRender::set_light_target(const Point &at, int value) {
-    $light.set_target(at, value);
+    $paths.set_target(at, value);
   }
 
   void LightRender::path_light(Matrix &walls) {
-    $light.compute_paths(walls);
+    $paths.compute_paths(walls);
   }
 
   void LightRender::light_box(LightSource source, Point from, Point &min_out, Point &max_out) {

lights.hpp

@@ -36,14 +36,14 @@ namespace lighting {
     size_t $width;
     size_t $height;
     Matrix $lightmap;
-    Pathing $light;
+    Pathing $paths;
 
     LightRender(size_t width, size_t height, int limit) :
       $limit(limit),
       $width(width),
       $height(height),
       $lightmap(height, MatrixRow(width, 0)),
-      $light(width, height, limit)
+      $paths(width, height, limit)
     {}
 
     void reset_light();

map.cpp

@@ -66,6 +66,7 @@ Point Map::place_entity(size_t room_index) {
   dbc::check(room_index < $rooms.size(), "room_index is out of bounds, not enough rooms");
 
   Room &start = $rooms[room_index];
+  // BUG: this can place someone in a wall on accident, move them if they're stuck
   return {start.x+1, start.y+1};
 }
 
@@ -142,12 +143,14 @@ bool Map::neighbors(Point &out, bool random) {
   int cur = paths[out.y][out.x];
 
   // pick a random start of directions
+  // BUG: is uniform inclusive of the dir.size()?
   int rand_start = Random::uniform<int>(0, dirs.size());
 
   // go through all possible directions
   for(size_t i = 0; i < dirs.size(); i++) {
     // but start at the random start, effectively randomizing
     // which valid direction to go
+    // BUG: this might be wrong given the above ranom from 0-size
     Point dir = dirs[(i + rand_start) % dirs.size()];
     if(!inmap(dir.x, dir.y)) continue; //skip unpathable stuff
     int weight = cur - paths[dir.y][dir.x];

pathing.cpp

@@ -16,6 +16,7 @@ inline void add_neighbors(PointList &neighbors, Matrix &closed, size_t y, size_t
           (0 <= col && col < w) &&
           closed[row][col] == 0)
       {
+        // BUG: maybe value here?
         closed[row][col] = 1;
         neighbors.push_back({.x=col, .y=row});
       }
@@ -43,8 +44,8 @@ void Pathing::compute_paths(Matrix &walls) {
     size_t y = counter / $width;
     if($input[y][x] == 0) {
       $paths[y][x] = 0;
-      closed[y][x] = 1;
+      closed[y][x] = 1;  // BUG: value here?
-      starting_pixels.push_back({.x=x,.y=y});
+      starting_pixels.push_back({x,y});
     }
   }
 
@@ -71,6 +72,7 @@ void Pathing::compute_paths(Matrix &walls) {
 }
 
 void Pathing::set_target(const Point &at, int value) {
+  // BUG: not using value here but it can be < 0 for deeper slopes
   $input[at.y][at.x] = 0;
 }
 

render.cpp

@@ -6,6 +6,7 @@
 #include "map.hpp"
 #include <iostream>
 #include "color.hpp"
+
 #if defined(_WIN64) || defined(_WIN32)
  #include <stdio.h>
  #include <fcntl.h>
@@ -65,7 +66,8 @@ void SFMLRender::resize_grid(int new_size, Panel &panel_out) {
   panel_out.resize(view_x, view_y);
 }
 
-inline void configure_tile(const sf::Sprite &sprite, sf::FloatRect &sp_bounds,  sf::FloatRect bg_bounds, float &width_delta, float &height_delta) {
+inline void configure_tile(const sf::Sprite &sprite, sf::FloatRect &sp_bounds, sf::FloatRect bg_bounds, float &width_delta, float &height_delta) {
+  // BUG: I think I could create a struct that kept this info for all sprites loaded
   // should look into caching all this instead of calcing it each time
   sp_bounds = sprite.getLocalBounds();
 
@@ -75,7 +77,7 @@ inline void configure_tile(const sf::Sprite &sprite, sf::FloatRect &sp_bounds,
 }
 
 void SFMLRender::render_grid(const std::wstring &text, sf::Color default_fg, sf::Color default_bg, float x, float y) {
-  wchar_t last_tile = '#';
+  wchar_t last_tile = $config.bg_tile;
   sf::FloatRect sp_bounds;
   float width_delta = 0;
   float height_delta = 0;
@@ -86,7 +88,7 @@ void SFMLRender::render_grid(const std::wstring &text, sf::Color default_fg, sf:
   sf::Color cur_bg = default_bg;
 
   // make a copy so we don't modify the cached one
-  $ansi.parse(text, [&](sf::Color fg, sf::Color bg) {
+  $ansi.parse(text, [&](auto fg, auto bg) {
       cur_fg = fg;
       cur_bg = bg;
     },
@@ -104,9 +106,9 @@ void SFMLRender::render_grid(const std::wstring &text, sf::Color default_fg, sf:
 
           // only get a new sprite if the tile changed
           if(last_tile != tile) {
-            last_tile = tile; // update last tile seen
             sprite = get_text_sprite(tile);
             configure_tile(sprite, sp_bounds, $bg_bounds, width_delta, height_delta);
+            last_tile = tile; // update last tile seen
           }
 
           sprite.setPosition({x+width_delta, y+height_delta});
@@ -135,6 +137,7 @@ inline sf::FloatRect draw_chunk(sf::RenderWindow& window,
   text.setPosition({x, y});
   // get a base character for the cell size
   sf::FloatRect bounds(x, y, ui_bounds.width * out.size(), ui_bounds.height);
+
   if(default_bg != bgcolor) {
     sf::RectangleShape backing({bounds.width, bounds.height});
     backing.setFillColor(bgcolor);
@@ -157,7 +160,7 @@ void SFMLRender::render_text(const std::wstring &text, sf::Color default_fg, sf:
   $ui_text.setFillColor(default_fg);
 
   $ansi.parse(text,
-    [&](sf::Color fg, sf::Color bg) {
+    [&](auto fg, auto bg) {
         if(out.size() > 0 ) {
           auto bounds = draw_chunk($window,
               $ui_bounds, $ui_text,
@@ -232,6 +235,5 @@ void SFMLRender::init_terminal() {
   _setmode(_fileno(stdout), _O_U16TEXT);
 #endif
 
-  // the parser only handles full color so force it
   ftxui::Terminal::SetColorSupport(ftxui::Terminal::Color::TrueColor);
 }

save.cpp

@@ -21,8 +21,11 @@ void save::to_file(fs::path path, DinkyECS::World &world, Map &map) {
   tser::BinaryArchive archive;
 
   save_data.facts.player = world.get_the<Player>();
-  save_data.map = MapData{map.$rooms, map.$walls, map.$limit};
+  save_data.map = MapData{
+    map.$limit, map.$width, map.$height,
+    map.$rooms, map.$walls};
 
+  // BUG: lights aren't saved/restored
   extract<Position>(world, save_data.position);
   extract<Combat>(world, save_data.combat);
   extract<Motion>(world, save_data.motion);
@@ -71,8 +74,8 @@ void save::from_file(fs::path path, DinkyECS::World &world_out, Map &map_out) {
   inject<Tile>(world_out, save_data.tile);
   inject<Inventory>(world_out, save_data.inventory);
 
-  size_t width = save_data.map.walls[0].size();
+  size_t width = save_data.map.width;
-  size_t height = save_data.map.walls.size();
+  size_t height = save_data.map.height;
   int limit = save_data.map.limit;
 
   Pathing paths(width, height, limit);

save.hpp

@@ -11,11 +11,13 @@ namespace save {
   namespace fs = std::filesystem;
 
   struct MapData {
+    int limit;
+    size_t width;
+    size_t height;
     std::vector<Room> rooms;
     Matrix walls;
-    int limit;
 
-    DEFINE_SERIALIZABLE(MapData, rooms, walls);
+    DEFINE_SERIALIZABLE(MapData, limit, width, height, rooms, walls);
   };
 
   struct Facts {

status.txt

@@ -6,6 +6,15 @@ TODAY'S GOAL:
 * Lua integration
 
 TODO:
+* $limit is pointless, just use a constant.
+* Pathing::compute_paths can take a starting level to implement lower directions, or possibly setting a value lower?
+* Pathing::set_target isn't using value, but that implements the above.
+https://www.roguebasin.com/index.php/Dijkstra_Maps_Visualized
+
+* When fighting two enemies with lots of attacks it crashes because one dies and isn't there.  Test by making enemies immortal.
+* LightRender can just use the Dijkstra map paths to calculate light strenght from the point rather than doing the box thing.
+* $paths.$paths is annoying.
+* Format of pre/post in dbc isn't consistent with the rest of the lib but I also maybe don't need the function version?
 * I can do headless windows in renderer for testing.
   - renderer.$window.setVisible(false);
 * Think up an enemy system.

systems.cpp

@@ -25,6 +25,7 @@ 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) {
@@ -44,6 +45,7 @@ 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)};
       }
@@ -55,6 +57,8 @@ 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>();
 
+  // BUG: instead of separate things maybe just one
+  // BUG: Collision component that references what is collide
   world.query<Position, Combat>([&](const auto &ent, auto &pos, auto &combat) {
       if(!combat.dead) {
         collider.insert(pos.location, ent);
@@ -95,6 +99,10 @@ void System::motion(DinkyECS::World &world, Map &game_map) {
 }
 
 void System::death(DinkyECS::World &world) {
+  // BUG: this is where entities can die on top of
+  // 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>();
 
   world.query<Position, Combat>([&](const auto &ent, auto &position, auto &combat) {
@@ -119,7 +127,6 @@ void System::collision(DinkyECS::World &world, Player &player) {
   auto [found, nearby] = collider.neighbors(player_position.location);
 
   if(found) {
-
     for(auto entity : nearby) {
       if(world.has<Combat>(entity)) {
         auto& enemy_combat = world.get<Combat>(entity);
@@ -134,6 +141,7 @@ void System::collision(DinkyECS::World &world, Player &player) {
         auto loot = world.get<Loot>(entity);
         auto &loot_pos = world.get<Position>(entity);
         auto &inventory = world.get<Inventory>(player.entity);
+        // BUG: this should go away and be a part of inventory
         auto &light = world.get<LightSource>(player.entity);
 
         world.send<Events::GUI>(Events::GUI::LOOT, entity, loot);
@@ -148,7 +156,9 @@ void System::collision(DinkyECS::World &world, Player &player) {
   }
 }
 
+// BUG: this is kind of massive, need to maybe rethink how systems are designed.  I mean...can each system be a callable class instead?
 void System::draw_entities(DinkyECS::World &world, Map &game_map, const Matrix &lighting, ftxui::Canvas &canvas, const Point &cam_orig, size_t view_x, size_t view_y) {
+
   world.query<Position, Tile>([&](const auto &ent, auto &pos, auto &tile) {
     if(pos.location.x >= cam_orig.x && pos.location.x <= cam_orig.x + view_x
         && pos.location.y >= cam_orig.y && pos.location.y <= cam_orig.y + view_y) {
@@ -156,6 +166,8 @@ void System::draw_entities(DinkyECS::World &world, Map &game_map, const Matrix &
 
       int light_value = lighting[pos.location.y][pos.location.x];
 
+      // BUG: foreground color needs to come from entity and background color from the surface they're on
+
       // the 2 and 4 are from ftxui::Canvas since it does a kind of "subpixel" drawing
       canvas.DrawText(loc.x*2, loc.y*4, tile.chr, [light_value](auto &pixel) {
           pixel.foreground_color = Color::HSV(255, 200, light_value + 20);

tests/worldbuilder.cpp

@@ -13,7 +13,6 @@ TEST_CASE("bsp algo test", "[builder]") {
   Map map(20, 20);
   WorldBuilder builder(map);
   builder.generate();
-  REQUIRE(map.$rooms.size() >= 2);
 }
 
 TEST_CASE("dumping and debugging", "[builder]") {
@@ -21,8 +20,6 @@ TEST_CASE("dumping and debugging", "[builder]") {
   WorldBuilder builder(map);
   builder.generate();
 
-  REQUIRE(map.$rooms.size() >= 2);
-
   dump_map("GENERATED", map.paths());
   map.dump();
 }
@@ -31,7 +28,6 @@ TEST_CASE("pathing", "[builder]") {
   Map map(20, 20);
   WorldBuilder builder(map);
   builder.generate();
-  REQUIRE(map.$rooms.size() >= 2);
 
   REQUIRE(map.can_move({0,0}) == false);
   REQUIRE(map.iswall(0,0) == true);

worldbuilder.cpp

@@ -6,6 +6,7 @@ using namespace fmt;
 
 inline int make_split(Room &cur, bool horiz) {
   size_t dimension = horiz ? cur.height : cur.width;
+  // BUG: this might be better as a configurable 4 number
   int min = dimension / 4;
   int max = dimension - min;
 
@@ -77,11 +78,13 @@ void WorldBuilder::partition_map(Room &cur, int depth) {
     right.width = size_t(cur.width - split);
   }
 
+  // BUG: min room size should be configurable
   if(depth > 0  && left.width > 2 && left.height > 2) {
     left.depth = depth - 1;
     partition_map(left, depth-1);
   }
 
+  // BUG: min room size should be configurable
   if(depth > 0 && right.width > 2 && right.height > 2) {
     right.depth = depth - 1;
     partition_map(right, depth-1);
@@ -104,6 +107,7 @@ void WorldBuilder::tunnel_doors(PointList &holes, Room &src, Room &target) {
 }
 
 void WorldBuilder::generate() {
+  PointList holes;
   Room root{
     .x = 0,
     .y = 0,
@@ -111,13 +115,11 @@ void WorldBuilder::generate() {
     .height = $map.$height
   };
 
+  // BUG: depth should be configurable
   partition_map(root, 10);
-  $map.INVARIANT();
 
   place_rooms();
 
-  PointList holes;
-
   for(size_t i = 0; i < $map.$rooms.size() - 1; i++) {
     tunnel_doors(holes, $map.$rooms[i], $map.$rooms[i+1]);
   }
@@ -159,6 +161,8 @@ void WorldBuilder::place_rooms() {
     cur.width -= 4;
     cur.height -= 4;
 
+    // BUG: should I do this each time I connect rooms
+    // BUG: rather than once when the room is created?
     add_door(cur);
     make_room(cur.x, cur.y, cur.width, cur.height);
   }
@@ -166,6 +170,8 @@ void WorldBuilder::place_rooms() {
 
 bool WorldBuilder::dig_tunnel(PointList &holes, Point &src, Point &target) {
   Matrix &paths = $map.paths();
+
+  // BUG: limit should be a constant since it never changes
   int limit = $map.limit();
 
   dbc::check(paths[src.y][src.x] != limit,