From f9c5edf6b9e1c403b544034298e06a1d44cce310 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Fri, 6 Oct 2023 16:44:18 +0200 Subject: [PATCH 1/2] Replace more sized reads and unsized ints --- components/esm3/loadmisc.cpp | 2 +- components/esm3/loadmisc.hpp | 6 +++--- components/esm3/loadprob.cpp | 2 +- components/esm3/loadprob.hpp | 6 +++--- components/esm3/loadsoun.cpp | 2 +- components/esm3/loadsoun.hpp | 2 +- components/esm3/loadspel.cpp | 6 ++---- components/esm3/loadspel.hpp | 8 ++++---- 8 files changed, 16 insertions(+), 18 deletions(-) diff --git a/components/esm3/loadmisc.cpp b/components/esm3/loadmisc.cpp index 780e8077fe..b38ce63294 100644 --- a/components/esm3/loadmisc.cpp +++ b/components/esm3/loadmisc.cpp @@ -28,7 +28,7 @@ namespace ESM mName = esm.getHString(); break; case fourCC("MCDT"): - esm.getHTSized<12>(mData); + esm.getHT(mData.mWeight, mData.mValue, mData.mFlags); hasData = true; break; case fourCC("SCRI"): diff --git a/components/esm3/loadmisc.hpp b/components/esm3/loadmisc.hpp index 9c46b7494e..f3eaf7d10f 100644 --- a/components/esm3/loadmisc.hpp +++ b/components/esm3/loadmisc.hpp @@ -27,8 +27,8 @@ namespace ESM struct MCDTstruct { float mWeight; - int mValue; - int mFlags; + int32_t mValue; + int32_t mFlags; }; enum Flags @@ -38,7 +38,7 @@ namespace ESM MCDTstruct mData; - unsigned int mRecordFlags; + uint32_t mRecordFlags; RefId mId, mScript; std::string mName, mModel, mIcon; diff --git a/components/esm3/loadprob.cpp b/components/esm3/loadprob.cpp index 779f0d1534..3f9ba95bf1 100644 --- a/components/esm3/loadprob.cpp +++ b/components/esm3/loadprob.cpp @@ -28,7 +28,7 @@ namespace ESM mName = esm.getHString(); break; case fourCC("PBDT"): - esm.getHTSized<16>(mData); + esm.getHT(mData.mWeight, mData.mValue, mData.mQuality, mData.mUses); hasData = true; break; case fourCC("SCRI"): diff --git a/components/esm3/loadprob.hpp b/components/esm3/loadprob.hpp index 328c1eaecd..74e02e7d10 100644 --- a/components/esm3/loadprob.hpp +++ b/components/esm3/loadprob.hpp @@ -22,14 +22,14 @@ namespace ESM struct Data { float mWeight; - int mValue; + int32_t mValue; float mQuality; - int mUses; + int32_t mUses; }; // Size = 16 Data mData; - unsigned int mRecordFlags; + uint32_t mRecordFlags; RefId mId, mScript; std::string mName, mModel, mIcon; diff --git a/components/esm3/loadsoun.cpp b/components/esm3/loadsoun.cpp index af7d7b3710..fd403e3429 100644 --- a/components/esm3/loadsoun.cpp +++ b/components/esm3/loadsoun.cpp @@ -25,7 +25,7 @@ namespace ESM mSound = esm.getHString(); break; case fourCC("DATA"): - esm.getHTSized<3>(mData); + esm.getHT(mData.mVolume, mData.mMinRange, mData.mMaxRange); hasData = true; break; case SREC_DELE: diff --git a/components/esm3/loadsoun.hpp b/components/esm3/loadsoun.hpp index 0da915b0f1..0129d1fe40 100644 --- a/components/esm3/loadsoun.hpp +++ b/components/esm3/loadsoun.hpp @@ -25,7 +25,7 @@ namespace ESM static std::string_view getRecordType() { return "Sound"; } SOUNstruct mData; - unsigned int mRecordFlags; + uint32_t mRecordFlags; std::string mSound; RefId mId; diff --git a/components/esm3/loadspel.cpp b/components/esm3/loadspel.cpp index d94073d33c..e4f63b8219 100644 --- a/components/esm3/loadspel.cpp +++ b/components/esm3/loadspel.cpp @@ -27,13 +27,11 @@ namespace ESM mName = esm.getHString(); break; case fourCC("SPDT"): - esm.getHTSized<12>(mData); + esm.getHT(mData.mType, mData.mCost, mData.mFlags); hasData = true; break; case fourCC("ENAM"): - ENAMstruct s; - esm.getHTSized<24>(s); - mEffects.mList.push_back(s); + mEffects.add(esm); break; case SREC_DELE: esm.skipHSub(); diff --git a/components/esm3/loadspel.hpp b/components/esm3/loadspel.hpp index 50ed65d3de..6477f8c7df 100644 --- a/components/esm3/loadspel.hpp +++ b/components/esm3/loadspel.hpp @@ -39,13 +39,13 @@ namespace ESM struct SPDTstruct { - int mType; // SpellType - int mCost; // Mana cost - int mFlags; // Flags + int32_t mType; // SpellType + int32_t mCost; // Mana cost + int32_t mFlags; // Flags }; SPDTstruct mData; - unsigned int mRecordFlags; + uint32_t mRecordFlags; std::string mName; RefId mId; EffectList mEffects; From b99f58613efa044627c5b55583744b44bbaa5233 Mon Sep 17 00:00:00 2001 From: Evil Eye Date: Fri, 6 Oct 2023 16:46:09 +0200 Subject: [PATCH 2/2] Remove signed/unsigned conversions in pathgrid loading code and use meaningful member names --- apps/esmtool/record.cpp | 8 +-- apps/opencs/model/tools/pathgridcheck.cpp | 4 +- .../model/world/nestedcoladapterimp.cpp | 6 +- components/esm3/loadpgrd.cpp | 59 +++++++++---------- components/esm3/loadpgrd.hpp | 11 ++-- 5 files changed, 42 insertions(+), 46 deletions(-) diff --git a/apps/esmtool/record.cpp b/apps/esmtool/record.cpp index 55a850cf7e..b13668dafc 100644 --- a/apps/esmtool/record.cpp +++ b/apps/esmtool/record.cpp @@ -1133,9 +1133,9 @@ namespace EsmTool { std::cout << " Cell: " << mData.mCell << std::endl; std::cout << " Coordinates: (" << mData.mData.mX << "," << mData.mData.mY << ")" << std::endl; - std::cout << " Unknown S1: " << mData.mData.mS1 << std::endl; - if (static_cast(mData.mData.mS2) != mData.mPoints.size()) - std::cout << " Reported Point Count: " << mData.mData.mS2 << std::endl; + std::cout << " Granularity: " << mData.mData.mGranularity << std::endl; + if (mData.mData.mPoints != mData.mPoints.size()) + std::cout << " Reported Point Count: " << mData.mData.mPoints << std::endl; std::cout << " Point Count: " << mData.mPoints.size() << std::endl; std::cout << " Edge Count: " << mData.mEdges.size() << std::endl; @@ -1154,7 +1154,7 @@ namespace EsmTool for (const ESM::Pathgrid::Edge& edge : mData.mEdges) { std::cout << " Edge[" << i << "]: " << edge.mV0 << " -> " << edge.mV1 << std::endl; - if (edge.mV0 >= static_cast(mData.mData.mS2) || edge.mV1 >= static_cast(mData.mData.mS2)) + if (edge.mV0 >= mData.mData.mPoints || edge.mV1 >= mData.mData.mPoints) std::cout << " BAD POINT IN EDGE!" << std::endl; i++; } diff --git a/apps/opencs/model/tools/pathgridcheck.cpp b/apps/opencs/model/tools/pathgridcheck.cpp index 6420c1c83c..f03b896321 100644 --- a/apps/opencs/model/tools/pathgridcheck.cpp +++ b/apps/opencs/model/tools/pathgridcheck.cpp @@ -44,9 +44,9 @@ void CSMTools::PathgridCheckStage::perform(int stage, CSMDoc::Messages& messages CSMWorld::UniversalId id(CSMWorld::UniversalId::Type_Pathgrid, pathgrid.mId); // check the number of pathgrid points - if (pathgrid.mData.mS2 < static_cast(pathgrid.mPoints.size())) + if (pathgrid.mData.mPoints < pathgrid.mPoints.size()) messages.add(id, "Less points than expected", "", CSMDoc::Message::Severity_Error); - else if (pathgrid.mData.mS2 > static_cast(pathgrid.mPoints.size())) + else if (pathgrid.mData.mPoints > pathgrid.mPoints.size()) messages.add(id, "More points than expected", "", CSMDoc::Message::Severity_Error); std::vector pointList(pathgrid.mPoints.size()); diff --git a/apps/opencs/model/world/nestedcoladapterimp.cpp b/apps/opencs/model/world/nestedcoladapterimp.cpp index 9572d8de39..0f3670431d 100644 --- a/apps/opencs/model/world/nestedcoladapterimp.cpp +++ b/apps/opencs/model/world/nestedcoladapterimp.cpp @@ -41,7 +41,7 @@ namespace CSMWorld point.mUnknown = 0; points.insert(points.begin() + position, point); - pathgrid.mData.mS2 = pathgrid.mPoints.size(); + pathgrid.mData.mPoints = pathgrid.mPoints.size(); record.setModified(pathgrid); } @@ -58,7 +58,7 @@ namespace CSMWorld // Do not remove dangling edges, does not work with current undo mechanism // Do not automatically adjust indices, what would be done with dangling edges? points.erase(points.begin() + rowToRemove); - pathgrid.mData.mS2 = pathgrid.mPoints.size(); + pathgrid.mData.mPoints = pathgrid.mPoints.size(); record.setModified(pathgrid); } @@ -67,7 +67,7 @@ namespace CSMWorld { Pathgrid pathgrid = record.get(); pathgrid.mPoints = static_cast&>(nestedTable).mNestedTable; - pathgrid.mData.mS2 = pathgrid.mPoints.size(); + pathgrid.mData.mPoints = pathgrid.mPoints.size(); record.setModified(pathgrid); } diff --git a/components/esm3/loadpgrd.cpp b/components/esm3/loadpgrd.cpp index ea0b53d8d2..8d60d25524 100644 --- a/components/esm3/loadpgrd.cpp +++ b/components/esm3/loadpgrd.cpp @@ -7,18 +7,18 @@ namespace ESM { Pathgrid::Point& Pathgrid::Point::operator=(const float rhs[3]) { - mX = static_cast(rhs[0]); - mY = static_cast(rhs[1]); - mZ = static_cast(rhs[2]); + mX = static_cast(rhs[0]); + mY = static_cast(rhs[1]); + mZ = static_cast(rhs[2]); mAutogenerated = 0; mConnectionNum = 0; mUnknown = 0; return *this; } Pathgrid::Point::Point(const float rhs[3]) - : mX(static_cast(rhs[0])) - , mY(static_cast(rhs[1])) - , mZ(static_cast(rhs[2])) + : mX(static_cast(rhs[0])) + , mY(static_cast(rhs[1])) + , mZ(static_cast(rhs[2])) , mAutogenerated(0) , mConnectionNum(0) , mUnknown(0) @@ -42,7 +42,7 @@ namespace ESM mEdges.clear(); // keep track of total connections so we can reserve edge vector size - int edgeCount = 0; + size_t edgeCount = 0; bool hasData = false; while (esm.hasMoreSubs()) @@ -54,21 +54,20 @@ namespace ESM mCell = esm.getRefId(); break; case fourCC("DATA"): - esm.getHTSized<12>(mData); + esm.getHT(mData.mX, mData.mY, mData.mGranularity, mData.mPoints); hasData = true; break; case fourCC("PGRP"): { esm.getSubHeader(); - int size = esm.getSubSize(); - // Check that the sizes match up. Size = 16 * s2 (path points) - if (size != static_cast(sizeof(Point) * mData.mS2)) + uint32_t size = esm.getSubSize(); + // Check that the sizes match up. Size = 16 * path points + if (size != sizeof(Point) * mData.mPoints) esm.fail("Path point subrecord size mismatch"); else { - int pointCount = mData.mS2; - mPoints.reserve(pointCount); - for (int i = 0; i < pointCount; ++i) + mPoints.reserve(mData.mPoints); + for (uint16_t i = 0; i < mData.mPoints; ++i) { Point p; esm.getExact(&p, sizeof(Point)); @@ -81,21 +80,19 @@ namespace ESM case fourCC("PGRC"): { esm.getSubHeader(); - int size = esm.getSubSize(); - if (size % sizeof(int) != 0) + uint32_t size = esm.getSubSize(); + if (size % sizeof(uint32_t) != 0) esm.fail("PGRC size not a multiple of 4"); else { - int rawConnNum = size / sizeof(int); + size_t rawConnNum = size / sizeof(uint32_t); std::vector rawConnections; rawConnections.reserve(rawConnNum); - for (int i = 0; i < rawConnNum; ++i) + for (size_t i = 0; i < rawConnNum; ++i) { - int currentValue; + uint32_t currentValue; esm.getT(currentValue); - if (currentValue < 0) - esm.fail("currentValue is less than 0"); - rawConnections.push_back(static_cast(currentValue)); + rawConnections.push_back(currentValue); } auto rawIt = rawConnections.begin(); @@ -138,7 +135,7 @@ namespace ESM // Correct connection count and sort edges by point // Can probably be optimized PointList correctedPoints = mPoints; - std::vector sortedEdges; + std::vector sortedEdges; sortedEdges.reserve(mEdges.size()); @@ -150,7 +147,7 @@ namespace ESM { if (edge.mV0 == point) { - sortedEdges.push_back(static_cast(edge.mV1)); + sortedEdges.push_back(static_cast(edge.mV1)); ++correctedPoints[point].mConnectionNum; } } @@ -162,16 +159,16 @@ namespace ESM if (isDeleted) { - esm.writeHNString("DELE", "", 3); + esm.writeHNString("DELE", {}, 3); return; } if (!correctedPoints.empty()) { esm.startSubRecord("PGRP"); - for (PointList::const_iterator it = correctedPoints.begin(); it != correctedPoints.end(); ++it) + for (const Point& point : correctedPoints) { - esm.writeT(*it); + esm.writeT(point); } esm.endRecord("PGRP"); } @@ -179,9 +176,9 @@ namespace ESM if (!sortedEdges.empty()) { esm.startSubRecord("PGRC"); - for (std::vector::const_iterator it = sortedEdges.begin(); it != sortedEdges.end(); ++it) + for (const uint32_t& edge : sortedEdges) { - esm.writeT(*it); + esm.writeT(edge); } esm.endRecord("PGRC"); } @@ -192,8 +189,8 @@ namespace ESM mCell = ESM::RefId(); mData.mX = 0; mData.mY = 0; - mData.mS1 = 0; - mData.mS2 = 0; + mData.mGranularity = 0; + mData.mPoints = 0; mPoints.clear(); mEdges.clear(); } diff --git a/components/esm3/loadpgrd.hpp b/components/esm3/loadpgrd.hpp index 464358fef4..a343552efb 100644 --- a/components/esm3/loadpgrd.hpp +++ b/components/esm3/loadpgrd.hpp @@ -25,18 +25,17 @@ namespace ESM struct DATAstruct { - int mX, mY; // Grid location, matches cell for exterior cells - short mS1; // ?? Usually but not always a power of 2. Doesn't seem - // to have any relation to the size of PGRC. - short mS2; // Number of path points. + int32_t mX, mY; // Grid location, matches cell for exterior cells + int16_t mGranularity; // Granularity with which the graph was autogenerated + uint16_t mPoints; // Number of path points. }; // 12 bytes struct Point // path grid point { - int mX, mY, mZ; // Location of point + int32_t mX, mY, mZ; // Location of point unsigned char mAutogenerated; // autogenerated vs. user coloring flag? unsigned char mConnectionNum; // number of connections for this point - short mUnknown; + int16_t mUnknown; Point& operator=(const float[3]); Point(const float[3]); Point();