From 9a8430144a99f9e80c1433cec98c042f7a0d6258 Mon Sep 17 00:00:00 2001 From: Benjamen Meyer Date: Sat, 12 Aug 2023 15:06:20 -0400 Subject: [PATCH 1/4] Bug Fixes - Log which build is being used - For DEBUG builds do not do *any* optimization so that source can be easily inspected under GDB/Debugger - Add logging of JSON data to help with Debugging - Use an iterable loop in the GFX Mesh instead of index based for better reliability; old loop could end up in weird circumstances --- engine/CMakeLists.txt | 6 +++++- engine/src/cmd/json.cpp | 8 ++++++++ engine/src/gfx/mesh_gfx.cpp | 6 +++--- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/engine/CMakeLists.txt b/engine/CMakeLists.txt index 0202e810fd..200c9cb565 100644 --- a/engine/CMakeLists.txt +++ b/engine/CMakeLists.txt @@ -178,6 +178,8 @@ ENDIF(NOT POW_FUNCTION_EXISTS AND NOT NEED_LINKING_AGAINST_LIBM) IF (NOT CMAKE_BUILD_TYPE) SET(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Release, RelWithDebInfo, Debug, Profiler" FORCE ) ENDIF (NOT CMAKE_BUILD_TYPE) +MESSAGE("** Build Type: ${CMAKE_BUILD_TYPE}") + #IF (NOT BUILD_OPT) # SET(BUILD_OPT -O2 CACHE STRING "-O0, -O1, -O2, -O3, -Os, -Ofast" FORCE ) @@ -296,10 +298,12 @@ add_compile_options("$<$:/FI${Vega_Strike_BINARY_DIR}/conf "$<$,$>:/Z7>" ) add_link_options("$<$:/DEBUG>") +# for DEBUG remove all optimizations add_compile_options("$<$,$,$>:-pipe>" "$<$,$,$>:-Wall>" "$<$,$,$>:-fvisibility=hidden>" - "$<$,$,$>,$,$,$>>:-Og>" + "$<$,$,$>,$>:-O0>" + "$<$,$,$>,$,$>>:-Og>" "$<$,$,$>,$,$>>:-g3>" "$<$,$,$>,$>:-pg>" "$<$,$,$>,$>:-g2>" diff --git a/engine/src/cmd/json.cpp b/engine/src/cmd/json.cpp index e5710b0515..cb576a1d68 100644 --- a/engine/src/cmd/json.cpp +++ b/engine/src/cmd/json.cpp @@ -5,6 +5,8 @@ #include "json.h" #include +#include "vs_logging.h" + /*! \brief Checks for an empty string * * @param str The string to check @@ -388,6 +390,12 @@ std::vector json::parsing::parse_array(const char *input) // Initalize the result std::vector result; + if (input != nullptr) { + VS_LOG_AND_FLUSH(debug, boost::format("JSON Data: %s") % input); + } else { + VS_LOG_AND_FLUSH(debug, "Invalid JSON Input - NULL Pointer"); + } + const char *index = json::parsing::tlws(input); if (*index != '[') throw json::parsing_error("Input was not an array"); index++; diff --git a/engine/src/gfx/mesh_gfx.cpp b/engine/src/gfx/mesh_gfx.cpp index d9f8ba0018..eebbdd70a4 100644 --- a/engine/src/gfx/mesh_gfx.cpp +++ b/engine/src/gfx/mesh_gfx.cpp @@ -438,9 +438,9 @@ Mesh::~Mesh() { vector *hashers = bfxmHashTable.Get(hash_name); vector::iterator finder; if (hashers) { - for (size_t i = hashers->size() - 1; i >= 0; --i) { - if (hashers->at(i) == this) { - hashers->erase(hashers->begin() + i); + for (auto hashItem = hashers->begin(); hashItem != hashers->end(); ++hashItem) { + if (*hashItem == this) { + hashers->erase(hashItem); if (hashers->empty()) { bfxmHashTable.Delete(hash_name); delete hashers; From ff9bf4e56d394e6675f3f59d2db24dac4e184099 Mon Sep 17 00:00:00 2001 From: Benjamen Meyer Date: Sun, 13 Aug 2023 02:04:53 -0400 Subject: [PATCH 2/4] Safe Iterator Removal --- engine/src/gfx/mesh_gfx.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/engine/src/gfx/mesh_gfx.cpp b/engine/src/gfx/mesh_gfx.cpp index eebbdd70a4..23edb8db44 100644 --- a/engine/src/gfx/mesh_gfx.cpp +++ b/engine/src/gfx/mesh_gfx.cpp @@ -438,8 +438,26 @@ Mesh::~Mesh() { vector *hashers = bfxmHashTable.Get(hash_name); vector::iterator finder; if (hashers) { + // the foollowing loop has several tricks to it: + // 1. `std::vector::erase()` can take an interator and remove it from the vector; but invalidates + // the iterator in the process + // 2. To overcome the invalid iterator issue, the next previous iterator is cached + // 3. In the case that the previous iterator would be invalid (e.g it's at the start) then it needs + // to restart the loop without the loop conditions, therefore a simple GOTO is used instead to + // avoid the incrementing the iterator so that values are not skipped + // A reverse iterator could kind of help this; however, `std::vector::erase` unfortunately + // does not work on reverse iterators. for (auto hashItem = hashers->begin(); hashItem != hashers->end(); ++hashItem) { +retryEraseItem: if (*hashItem == this) { + bool resetIter = false; + auto cachedHashItem = hashers->begin(); + if (hashItem != hashers->begin()) { + cachedHashItem = hashItem - 1; + } else { + resetIter = true; + cachedHashItem = hashItem + 1; + } hashers->erase(hashItem); if (hashers->empty()) { bfxmHashTable.Delete(hash_name); @@ -447,6 +465,14 @@ Mesh::~Mesh() { hashers = nullptr; break; } + + if (resetIter) { + hashItem = hashers->begin(); + // a necessary use of Goto as we do not want to use ++hashItem + goto retryEraseItem; + } else { + hashItem = cachedHashItem; + } } } } From 58f8e71b7170f8c5b166c863d6c94199aabdbad4 Mon Sep 17 00:00:00 2001 From: Benjamen Meyer Date: Mon, 14 Aug 2023 00:28:17 -0400 Subject: [PATCH 3/4] Bug Fix: Protect & Log Protect from dividing by zero, and Log when it's attempted --- engine/src/cmd/upgradeable_unit.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/engine/src/cmd/upgradeable_unit.cpp b/engine/src/cmd/upgradeable_unit.cpp index 891b485cab..210b3fceb4 100644 --- a/engine/src/cmd/upgradeable_unit.cpp +++ b/engine/src/cmd/upgradeable_unit.cpp @@ -38,6 +38,7 @@ #include "unit_generic.h" #include "weapon_info.h" #include "vega_cast_utils.h" +#include "vs_logging.h" std::vector ParseUnitUpgrades(const std::string &upgrades) { if(upgrades.size() == 0) { @@ -141,6 +142,14 @@ bool UpgradeableUnit::UpgradeMounts(const Unit *up, return true; } + // there needs to be some mounts to be able to mount to + if (num_mounts == 0) { + // would be nice to make this more meaningful but that's a little harder given + // the casting of `unit` from `this`. + VS_LOG(debug, "No mounts to attach to."); + return false; + } + int j = mountoffset; int i = 0; bool cancompletefully = true; From ea6147274f01e93f1d4a1e1fd9b7136d66685841 Mon Sep 17 00:00:00 2001 From: Benjamen Meyer Date: Thu, 7 Sep 2023 00:41:56 -0400 Subject: [PATCH 4/4] Bug Fix: Windows CI Builds - Add SDL2 to the VCPkg configuration used by the Windows CI --- engine/vcpkg.json | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/vcpkg.json b/engine/vcpkg.json index 3d66e9e900..4f949e94b8 100644 --- a/engine/vcpkg.json +++ b/engine/vcpkg.json @@ -27,6 +27,7 @@ "version>=": "1.1.1m" }, "sdl1", + "sdl2", "zlib" ] }