From 00eb02c1bccf04f8399d762a827fad17f9451a00 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 24 Feb 2023 15:51:11 -0800 Subject: [PATCH 1/2] Implements plugin: channel-safely v1.2.4 - changes report* lookup in `NewReportEvent()` - adds a nullptr check - adds df::coord bound checking in various places - where the `get_*neighbours()` functions are used - `simulate_fall()` - `is_safe_to_dig_down()` and `is_safe_fall()` - adds nullptr checks to the `is_*job()` functions - added todo comments for `is_safe_to_dig_down()` --- plugins/channel-safely/channel-groups.cpp | 38 ++++++++++--------- .../channel-safely/channel-safely-plugin.cpp | 14 +++++-- plugins/channel-safely/include/inlines.h | 30 +++++++++++---- 3 files changed, 54 insertions(+), 28 deletions(-) diff --git a/plugins/channel-safely/channel-groups.cpp b/plugins/channel-safely/channel-groups.cpp index 2650d92d0..c1a5b5953 100644 --- a/plugins/channel-safely/channel-groups.cpp +++ b/plugins/channel-safely/channel-groups.cpp @@ -21,24 +21,27 @@ void ChannelJobs::load_channel_jobs() { } bool ChannelJobs::has_cavein_conditions(const df::coord &map_pos) { - auto p = map_pos; - auto ttype = *Maps::getTileType(p); - if (!DFHack::isOpenTerrain(ttype)) { - // check shared neighbour for cave-in conditions - df::coord neighbours[4]; - get_connected_neighbours(map_pos, neighbours); - int connectedness = 4; - for (auto n: neighbours) { - if (active.count(n) || DFHack::isOpenTerrain(*Maps::getTileType(n))) { - connectedness--; + if likely(Maps::isValidTilePos(map_pos)) { + auto p = map_pos; + auto ttype = *Maps::getTileType(p); + if (!DFHack::isOpenTerrain(ttype)) { + // check shared neighbour for cave-in conditions + df::coord neighbours[4]; + get_connected_neighbours(map_pos, neighbours); + int connectedness = 4; + for (auto n: neighbours) { + if (!Maps::isValidTilePos(n) || active.count(n) || DFHack::isOpenTerrain(*Maps::getTileType(n))) { + connectedness--; + } } - } - if (!connectedness) { - // do what? - p.z--; - ttype = *Maps::getTileType(p); - if (DFHack::isOpenTerrain(ttype) || DFHack::isFloorTerrain(ttype)) { - return true; + if (!connectedness) { + // do what? + p.z--; + if (!Maps::isValidTilePos(p)) return false; + ttype = *Maps::getTileType(p); + if (DFHack::isOpenTerrain(ttype) || DFHack::isFloorTerrain(ttype)) { + return true; + } } } } @@ -88,6 +91,7 @@ void ChannelGroups::add(const df::coord &map_pos) { DEBUG(groups).print(" add(" COORD ")\n", COORDARGS(map_pos)); // and so we begin iterating the neighbours for (auto &neighbour: neighbors) { + if unlikely(!Maps::isValidTilePos(neighbour)) continue; // go to the next neighbour if this one doesn't have a group if (!groups_map.count(neighbour)) { TRACE(groups).print(" -> neighbour is not designated\n"); diff --git a/plugins/channel-safely/channel-safely-plugin.cpp b/plugins/channel-safely/channel-safely-plugin.cpp index adb668468..910e0ee7c 100644 --- a/plugins/channel-safely/channel-safely-plugin.cpp +++ b/plugins/channel-safely/channel-safely-plugin.cpp @@ -112,6 +112,10 @@ enum SettingConfigData { // dig-now.cpp df::coord simulate_fall(const df::coord &pos) { + if unlikely(!Maps::isValidTilePos(pos)) { + ERR(plugin).print("Error: simulate_fall(" COORD ") - invalid coordinate\n", COORDARGS(pos)); + return {}; + } df::coord resting_pos(pos); while (Maps::ensureTileBlock(resting_pos)) { @@ -130,6 +134,7 @@ df::coord simulate_area_fall(const df::coord &pos) { get_neighbours(pos, neighbours); df::coord lowest = simulate_fall(pos); for (auto p : neighbours) { + if unlikely(!Maps::isValidTilePos(p)) continue; auto nlow = simulate_fall(p); if (nlow.z < lowest.z) { lowest = nlow; @@ -299,10 +304,11 @@ namespace CSP { int32_t tick = df::global::world->frame_counter; auto report_id = (int32_t)(intptr_t(r)); if (df::global::world) { - std::vector &reports = df::global::world->status.reports; - size_t idx = -1; - idx = df::report::binsearch_index(reports, report_id); - df::report* report = reports.at(idx); + df::report* report = df::report::find(report_id); + if (!report) { + WARN(plugin).print("Error: NewReportEvent() received an invalid report_id - a report* cannot be found\n"); + return; + } switch (report->type) { case announcement_type::CANCEL_JOB: if (config.insta_dig) { diff --git a/plugins/channel-safely/include/inlines.h b/plugins/channel-safely/include/inlines.h index 172275778..a29f5a04d 100644 --- a/plugins/channel-safely/include/inlines.h +++ b/plugins/channel-safely/include/inlines.h @@ -64,11 +64,13 @@ inline uint8_t count_accessibility(const df::coord &unit_pos, const df::coord &m get_connected_neighbours(map_pos, connections); uint8_t accessibility = Maps::canWalkBetween(unit_pos, map_pos) ? 1 : 0; for (auto n: neighbours) { + if unlikely(!Maps::isValidTilePos(n)) continue; if (Maps::canWalkBetween(unit_pos, n)) { accessibility++; } } for (auto n : connections) { + if unlikely(Maps::isValidTilePos(n)) continue; if (Maps::canWalkBetween(unit_pos, n)) { accessibility++; } @@ -77,22 +79,22 @@ inline uint8_t count_accessibility(const df::coord &unit_pos, const df::coord &m } inline bool isEntombed(const df::coord &unit_pos, const df::coord &map_pos) { - if (Maps::canWalkBetween(unit_pos, map_pos)) { + if likely(Maps::canWalkBetween(unit_pos, map_pos)) { return false; } df::coord neighbours[8]; get_neighbours(map_pos, neighbours); return std::all_of(neighbours+0, neighbours+8, [&unit_pos](df::coord n) { - return !Maps::canWalkBetween(unit_pos, n); + return !Maps::isValidTilePos(n) || !Maps::canWalkBetween(unit_pos, n); }); } inline bool is_dig_job(const df::job* job) { - return job->job_type == df::job_type::Dig || job->job_type == df::job_type::DigChannel; + return job && (job->job_type == df::job_type::Dig || job->job_type == df::job_type::DigChannel); } inline bool is_channel_job(const df::job* job) { - return job->job_type == df::job_type::DigChannel; + return job && (job->job_type == df::job_type::DigChannel); } inline bool is_group_job(const ChannelGroups &groups, const df::job* job) { @@ -111,34 +113,48 @@ inline bool is_safe_fall(const df::coord &map_pos) { df::coord below(map_pos); for (uint8_t zi = 0; zi < config.fall_threshold; ++zi) { below.z--; + // falling out of bounds is probably considerably unsafe for a dwarf + if unlikely(!Maps::isValidTilePos(below)) { + return false; + } + // if we require vision, and we can't see below.. we'll need to assume it's safe to get anything done if (config.require_vision && Maps::getTileDesignation(below)->bits.hidden) { - return true; //we require vision, and we can't see below.. so we gotta assume it's safe + return true; } + // finally, if we're not looking at open space (air to fall through) it's safe to fall to df::tiletype type = *Maps::getTileType(below); if (!DFHack::isOpenTerrain(type)) { return true; } } + // we exceeded the fall threshold, so it's not a safe fall return false; } inline bool is_safe_to_dig_down(const df::coord &map_pos) { df::coord pos(map_pos); + // todo: probably should rely on is_safe_fall, it looks like it could be simplified a great deal for (uint8_t zi = 0; zi <= config.fall_threshold; ++zi) { - // assume safe if we can't see and need vision + // if we're digging out of bounds, the game can handle that (hopefully) + if unlikely(!Maps::isValidTilePos(pos)) { + return true; + } + // if we require vision, and we can't see the tiles in question.. we'll need to assume it's safe to dig to get anything done if (config.require_vision && Maps::getTileDesignation(pos)->bits.hidden) { return true; } + df::tiletype type = *Maps::getTileType(pos); if (zi == 0 && DFHack::isOpenTerrain(type)) { + // todo: remove? this is probably not useful.. and seems like the only considerable difference to is_safe_fall (aside from where each stops looking) // the starting tile is open space, that's obviously not safe return false; } else if (!DFHack::isOpenTerrain(type)) { // a tile after the first one is not open space return true; } - pos.z--; + pos.z--; // todo: this can probably move to the beginning of the loop } return false; } From 4813a15b35ebd0084e9ac892fa0b874990ebf256 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 24 Feb 2023 15:51:23 -0800 Subject: [PATCH 2/2] Updates changelog --- docs/changelog.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changelog.txt b/docs/changelog.txt index 9ee7ff102..fab27741a 100644 --- a/docs/changelog.txt +++ b/docs/changelog.txt @@ -44,6 +44,7 @@ changelog.txt uses a syntax similar to RST, with a few special sequences: - ``Buildings::StockpileIterator``: fix check for stockpile items on block boundary. - `tailor`: block making clothing sized for toads; make replacement clothing orders use the size of the wearer, not the size of the garment -@ `confirm`: fix fps drop when enabled +- `channel-safely`: fix an out of bounds error regarding the REPORT event listener receiving (presumably) stale id's ## Misc Improvements - `autobutcher`: logs activity to the console terminal instead of making disruptive in-game announcements