From 9abb39a3bf9392ed0d5e856af1ee6ac724dd92fe Mon Sep 17 00:00:00 2001 From: "Zed A. Shaw" Date: Wed, 4 Dec 2024 21:43:59 -0500 Subject: [PATCH] Did a full code review to identify things to fix and either fixed them or noted BUG where I should come back. --- config.hpp | 2 +- dinkyecs.hpp | 2 -- gui.cpp | 10 +++++++--- lights.cpp | 12 ++++++------ lights.hpp | 4 ++-- map.cpp | 3 +++ pathing.cpp | 6 ++++-- render.cpp | 14 ++++++++------ save.cpp | 9 ++++++--- save.hpp | 6 ++++-- status.txt | 9 +++++++++ systems.cpp | 14 +++++++++++++- tests/worldbuilder.cpp | 4 ---- worldbuilder.cpp | 12 +++++++++--- 14 files changed, 72 insertions(+), 35 deletions(-) diff --git a/config.hpp b/config.hpp index 67edaf9..82be1d5 100644 --- a/config.hpp +++ b/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); diff --git a/dinkyecs.hpp b/dinkyecs.hpp index 5eb4fe9..6c31120 100644 --- a/dinkyecs.hpp +++ b/dinkyecs.hpp @@ -78,7 +78,6 @@ namespace DinkyECS { template bool has(Entity ent) { EntityMap &map = entity_map_for(); - // use .at for bounds checking return map.contains(ent); } @@ -116,7 +115,6 @@ namespace DinkyECS { EventQueue &queue = queue_map_for(); Event evt = queue.front(); queue.pop(); - // I could use tie here to auto extract the any return evt; } diff --git a/gui.cpp b/gui.cpp index cc2dfc6..a50a84e 100644 --- a/gui.cpp +++ b/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(-20,20); - int y = Random::uniform(-20,20); + int x = Random::uniform(-10,10); + int y = Random::uniform(-10,10); // add x/y back to draw screen $renderer.draw($map_view, x, y); $renderer.display(); diff --git a/lights.cpp b/lights.cpp index 23c1485..5c401e2 100644 --- a/lights.cpp +++ b/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) { diff --git a/lights.hpp b/lights.hpp index 1b9cd5e..c70ec39 100644 --- a/lights.hpp +++ b/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(); diff --git a/map.cpp b/map.cpp index 869a635..aed18d3 100644 --- a/map.cpp +++ b/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(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]; diff --git a/pathing.cpp b/pathing.cpp index 5ba9e6b..262f3e3 100644 --- a/pathing.cpp +++ b/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; - starting_pixels.push_back({.x=x,.y=y}); + closed[y][x] = 1; // BUG: value here? + 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; } diff --git a/render.cpp b/render.cpp index 5bc518c..8299332 100644 --- a/render.cpp +++ b/render.cpp @@ -6,6 +6,7 @@ #include "map.hpp" #include #include "color.hpp" + #if defined(_WIN64) || defined(_WIN32) #include #include @@ -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); } diff --git a/save.cpp b/save.cpp index 386adb1..d3c8b0b 100644 --- a/save.cpp +++ b/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(); - 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(world, save_data.position); extract(world, save_data.combat); extract(world, save_data.motion); @@ -71,8 +74,8 @@ void save::from_file(fs::path path, DinkyECS::World &world_out, Map &map_out) { inject(world_out, save_data.tile); inject(world_out, save_data.inventory); - size_t width = save_data.map.walls[0].size(); - size_t height = save_data.map.walls.size(); + size_t width = save_data.map.width; + size_t height = save_data.map.height; int limit = save_data.map.limit; Pathing paths(width, height, limit); diff --git a/save.hpp b/save.hpp index 1309833..14ded25 100644 --- a/save.hpp +++ b/save.hpp @@ -11,11 +11,13 @@ namespace save { namespace fs = std::filesystem; struct MapData { + int limit; + size_t width; + size_t height; std::vector rooms; Matrix walls; - int limit; - DEFINE_SERIALIZABLE(MapData, rooms, walls); + DEFINE_SERIALIZABLE(MapData, limit, width, height, rooms, walls); }; struct Facts { diff --git a/status.txt b/status.txt index 0f74c0b..2d15c60 100644 --- a/status.txt +++ b/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. diff --git a/systems.cpp b/systems.cpp index 16d08ca..299c07e 100644 --- a/systems.cpp +++ b/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([&](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(); + // BUG: instead of separate things maybe just one + // BUG: Collision component that references what is collide world.query([&](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(); world.query([&](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(entity)) { auto& enemy_combat = world.get(entity); @@ -134,6 +141,7 @@ void System::collision(DinkyECS::World &world, Player &player) { auto loot = world.get(entity); auto &loot_pos = world.get(entity); auto &inventory = world.get(player.entity); + // BUG: this should go away and be a part of inventory auto &light = world.get(player.entity); world.send(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([&](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); diff --git a/tests/worldbuilder.cpp b/tests/worldbuilder.cpp index 2c5d3c9..218d133 100644 --- a/tests/worldbuilder.cpp +++ b/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); diff --git a/worldbuilder.cpp b/worldbuilder.cpp index 0e2f9a6..a55e4d2 100644 --- a/worldbuilder.cpp +++ b/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,