diff --git a/apps/components_tests/esm3/testsaveload.cpp b/apps/components_tests/esm3/testsaveload.cpp index cffd852c95..41a79313cc 100644 --- a/apps/components_tests/esm3/testsaveload.cpp +++ b/apps/components_tests/esm3/testsaveload.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -216,6 +217,19 @@ namespace ESM for (auto& v : dst) v = std::uniform_real_distribution{ -1.0f, 1.0f }(mRandom); } + + void generateBytes(auto iterator, std::size_t count) + { + std::uniform_int_distribution distribution{ 0, + std::numeric_limits::max() }; + std::generate_n(iterator, count, [&] { return static_cast(distribution(mRandom)); }); + } + + void generateStrings(auto iterator, std::size_t count) + { + std::uniform_int_distribution distribution{ 1, 13 }; + std::generate_n(iterator, count, [&] { return generateRandomString(distribution(mRandom)); }); + } }; TEST_F(Esm3SaveLoadRecordTest, headerShouldNotChange) @@ -265,11 +279,11 @@ namespace ESM Script record; record.blank(); record.mId = generateRandomRefId(33); - record.mData.mNumShorts = 42; + record.mNumShorts = 42; Script result; saveAndLoadRecord(record, CurrentSaveGameFormatVersion, result); EXPECT_EQ(result.mId, record.mId); - EXPECT_EQ(result.mData.mNumShorts, record.mData.mNumShorts); + EXPECT_EQ(result.mNumShorts, record.mNumShorts); } TEST_P(Esm3SaveLoadRecordTest, playerShouldNotChange) @@ -403,11 +417,24 @@ namespace ESM Script record; record.blank(); record.mId = generateRandomRefId(32); - record.mData.mNumShorts = 42; + record.mNumShorts = 3; + record.mNumFloats = 4; + record.mNumLongs = 5; + generateStrings( + std::back_inserter(record.mVarNames), record.mNumShorts + record.mNumFloats + record.mNumLongs); + generateBytes(std::back_inserter(record.mScriptData), 13); + record.mScriptText = generateRandomString(17); + Script result; saveAndLoadRecord(record, GetParam(), result); + EXPECT_EQ(result.mId, record.mId); - EXPECT_EQ(result.mData.mNumShorts, record.mData.mNumShorts); + EXPECT_EQ(result.mNumShorts, record.mNumShorts); + EXPECT_EQ(result.mNumFloats, record.mNumFloats); + EXPECT_EQ(result.mNumShorts, record.mNumShorts); + EXPECT_EQ(result.mVarNames, record.mVarNames); + EXPECT_EQ(result.mScriptData, record.mScriptData); + EXPECT_EQ(result.mScriptText, record.mScriptText); } TEST_P(Esm3SaveLoadRecordTest, quickKeysShouldNotChange) diff --git a/apps/esmtool/record.cpp b/apps/esmtool/record.cpp index cba389568b..b9c64d964a 100644 --- a/apps/esmtool/record.cpp +++ b/apps/esmtool/record.cpp @@ -2,6 +2,7 @@ #include "labels.hpp" #include +#include #include #include @@ -1181,11 +1182,11 @@ namespace EsmTool { std::cout << " Name: " << mData.mId << std::endl; - std::cout << " Num Shorts: " << mData.mData.mNumShorts << std::endl; - std::cout << " Num Longs: " << mData.mData.mNumLongs << std::endl; - std::cout << " Num Floats: " << mData.mData.mNumFloats << std::endl; - std::cout << " Script Data Size: " << mData.mData.mScriptDataSize << std::endl; - std::cout << " Table Size: " << mData.mData.mStringTableSize << std::endl; + std::cout << " Num Shorts: " << mData.mNumShorts << std::endl; + std::cout << " Num Longs: " << mData.mNumLongs << std::endl; + std::cout << " Num Floats: " << mData.mNumFloats << std::endl; + std::cout << " Script Data Size: " << mData.mScriptData.size() << std::endl; + std::cout << " Table Size: " << ESM::computeScriptStringTableSize(mData.mVarNames) << std::endl; for (const std::string& variable : mData.mVarNames) std::cout << " Variable: " << variable << std::endl; diff --git a/apps/essimporter/importscpt.cpp b/apps/essimporter/importscpt.cpp index 8fe4afd336..bb62c61103 100644 --- a/apps/essimporter/importscpt.cpp +++ b/apps/essimporter/importscpt.cpp @@ -7,8 +7,8 @@ namespace ESSImport void SCPT::load(ESM::ESMReader& esm) { - esm.getHNT("SCHD", mSCHD.mName.mData, mSCHD.mData.mNumShorts, mSCHD.mData.mNumLongs, mSCHD.mData.mNumFloats, - mSCHD.mData.mScriptDataSize, mSCHD.mData.mStringTableSize); + esm.getHNT("SCHD", mSCHD.mName.mData, mSCHD.mNumShorts, mSCHD.mNumLongs, mSCHD.mNumFloats, + mSCHD.mScriptDataSize, mSCHD.mStringTableSize); mSCRI.load(esm); diff --git a/apps/essimporter/importscpt.hpp b/apps/essimporter/importscpt.hpp index af383b674c..7c728ee97e 100644 --- a/apps/essimporter/importscpt.hpp +++ b/apps/essimporter/importscpt.hpp @@ -19,7 +19,11 @@ namespace ESSImport struct SCHD { ESM::NAME32 mName; - ESM::Script::SCHDstruct mData; + std::uint32_t mNumShorts; + std::uint32_t mNumLongs; + std::uint32_t mNumFloats; + std::uint32_t mScriptDataSize; + std::uint32_t mStringTableSize; }; // A running global script diff --git a/components/esm3/loadscpt.cpp b/components/esm3/loadscpt.cpp index ae56a7b4f4..91f952f3f0 100644 --- a/components/esm3/loadscpt.cpp +++ b/components/esm3/loadscpt.cpp @@ -1,6 +1,8 @@ #include "loadscpt.hpp" #include +#include +#include #include #include @@ -11,80 +13,93 @@ namespace ESM { - template T> - void decompose(T&& v, const auto& f) + namespace { - f(v.mNumShorts, v.mNumLongs, v.mNumFloats, v.mScriptDataSize, v.mStringTableSize); - } - - void Script::loadSCVR(ESMReader& esm) - { - uint32_t s = mData.mStringTableSize; - - std::vector tmp(s); - // not using getHExact, vanilla doesn't seem to mind unused bytes at the end - esm.getSubHeader(); - uint32_t left = esm.getSubSize(); - if (left < s) - esm.fail("SCVR string list is smaller than specified"); - esm.getExact(tmp.data(), s); - if (left > s) - esm.skip(left - s); // skip the leftover junk - - // Set up the list of variable names - mVarNames.resize(mData.mNumShorts + mData.mNumLongs + mData.mNumFloats); - - // The tmp buffer is a null-byte separated string list, we - // just have to pick out one string at a time. - char* str = tmp.data(); - if (tmp.empty()) + struct SCHD { - if (mVarNames.size() > 0) - Log(Debug::Warning) << "SCVR with no variable names"; - - return; - } - - // Support '\r' terminated strings like vanilla. See Bug #1324. - std::replace(tmp.begin(), tmp.end(), '\r', '\0'); - // Avoid heap corruption - if (tmp.back() != '\0') + /// Data from script-precompling in the editor. + /// \warning Do not use them. OpenCS currently does not precompile scripts. + std::uint32_t mNumShorts = 0; + std::uint32_t mNumLongs = 0; + std::uint32_t mNumFloats = 0; + std::uint32_t mScriptDataSize = 0; + std::uint32_t mStringTableSize = 0; + }; + + template T> + void decompose(T&& v, const auto& f) { - tmp.emplace_back('\0'); - std::stringstream ss; - ss << "Malformed string table"; - ss << "\n File: " << esm.getName(); - ss << "\n Record: " << esm.getContext().recName.toStringView(); - ss << "\n Subrecord: " - << "SCVR"; - ss << "\n Offset: 0x" << std::hex << esm.getFileOffset(); - Log(Debug::Verbose) << ss.str(); - str = tmp.data(); + f(v.mNumShorts, v.mNumLongs, v.mNumFloats, v.mScriptDataSize, v.mStringTableSize); } - const auto tmpEnd = tmp.data() + tmp.size(); - for (size_t i = 0; i < mVarNames.size(); i++) + void loadVarNames(const SCHD& header, std::vector& varNames, ESMReader& esm) { - mVarNames[i] = std::string(str); - str += mVarNames[i].size() + 1; - if (str >= tmpEnd) + uint32_t s = header.mStringTableSize; + + std::vector tmp(s); + // not using getHExact, vanilla doesn't seem to mind unused bytes at the end + esm.getSubHeader(); + uint32_t left = esm.getSubSize(); + if (left < s) + esm.fail("SCVR string list is smaller than specified"); + esm.getExact(tmp.data(), s); + if (left > s) + esm.skip(left - s); // skip the leftover junk + + // Set up the list of variable names + varNames.resize(header.mNumShorts + header.mNumLongs + header.mNumFloats); + + // The tmp buffer is a null-byte separated string list, we + // just have to pick out one string at a time. + if (tmp.empty()) { - if (str > tmpEnd) + if (!varNames.empty()) + Log(Debug::Warning) << "SCVR with no variable names"; + + return; + } + + // Support '\r' terminated strings like vanilla. See Bug #1324. + std::replace(tmp.begin(), tmp.end(), '\r', '\0'); + // Avoid heap corruption + if (tmp.back() != '\0') + { + tmp.push_back('\0'); + std::stringstream ss; + ss << "Malformed string table"; + ss << "\n File: " << esm.getName(); + ss << "\n Record: " << esm.getContext().recName.toStringView(); + ss << "\n Subrecord: " + << "SCVR"; + ss << "\n Offset: 0x" << std::hex << esm.getFileOffset(); + Log(Debug::Verbose) << ss.str(); + } + + const char* str = tmp.data(); + const char* const tmpEnd = tmp.data() + tmp.size(); + for (size_t i = 0; i < varNames.size(); i++) + { + varNames[i] = std::string(str); + str += varNames[i].size() + 1; + if (str >= tmpEnd) { - // SCVR subrecord is unused and variable names are determined - // from the script source, so an overflow is not fatal. - std::stringstream ss; - ss << "String table overflow"; - ss << "\n File: " << esm.getName(); - ss << "\n Record: " << esm.getContext().recName.toStringView(); - ss << "\n Subrecord: " - << "SCVR"; - ss << "\n Offset: 0x" << std::hex << esm.getFileOffset(); - Log(Debug::Verbose) << ss.str(); + if (str > tmpEnd) + { + // SCVR subrecord is unused and variable names are determined + // from the script source, so an overflow is not fatal. + std::stringstream ss; + ss << "String table overflow"; + ss << "\n File: " << esm.getName(); + ss << "\n Record: " << esm.getContext().recName.toStringView(); + ss << "\n Subrecord: " + << "SCVR"; + ss << "\n Offset: 0x" << std::hex << esm.getFileOffset(); + Log(Debug::Verbose) << ss.str(); + } + // Get rid of empty strings in the list. + varNames.resize(i + 1); + break; } - // Get rid of empty strings in the list. - mVarNames.resize(i + 1); - break; } } } @@ -96,7 +111,7 @@ namespace ESM mVarNames.clear(); - bool hasHeader = false; + std::optional header; while (esm.hasMoreSubs()) { esm.getSubName(); @@ -106,22 +121,27 @@ namespace ESM { esm.getSubHeader(); mId = esm.getMaybeFixedRefIdSize(32); - esm.getComposite(mData); - - hasHeader = true; + esm.getComposite(header.emplace()); + mNumShorts = header->mNumShorts; + mNumLongs = header->mNumLongs; + mNumFloats = header->mNumFloats; break; } case fourCC("SCVR"): - // list of local variables - loadSCVR(esm); + if (!header.has_value()) + esm.fail("SCVR is placed before SCHD record"); + loadVarNames(*header, mVarNames, esm); break; case fourCC("SCDT"): { + if (!header.has_value()) + esm.fail("SCDT is placed before SCHD record"); + // compiled script esm.getSubHeader(); uint32_t subSize = esm.getSubSize(); - if (subSize != mData.mScriptDataSize) + if (subSize != header->mScriptDataSize) { std::stringstream ss; ss << "Script data size defined in SCHD subrecord does not match size of SCDT subrecord"; @@ -147,22 +167,21 @@ namespace ESM } } - if (!hasHeader) + if (!header.has_value()) esm.fail("Missing SCHD subrecord"); - // Reported script data size is not always trustworthy, so override it with actual data size - mData.mScriptDataSize = static_cast(mScriptData.size()); } void Script::save(ESMWriter& esm, bool isDeleted) const { - std::string varNameString; - if (!mVarNames.empty()) - for (std::vector::const_iterator it = mVarNames.begin(); it != mVarNames.end(); ++it) - varNameString.append(*it); - esm.startSubRecord("SCHD"); esm.writeMaybeFixedSizeRefId(mId, 32); - esm.writeComposite(mData); + esm.writeComposite(SCHD{ + .mNumShorts = mNumShorts, + .mNumLongs = mNumLongs, + .mNumFloats = mNumFloats, + .mScriptDataSize = static_cast(mScriptData.size()), + .mStringTableSize = computeScriptStringTableSize(mVarNames), + }); esm.endRecord("SCHD"); if (isDeleted) @@ -174,15 +193,13 @@ namespace ESM if (!mVarNames.empty()) { esm.startSubRecord("SCVR"); - for (std::vector::const_iterator it = mVarNames.begin(); it != mVarNames.end(); ++it) - { - esm.writeHCString(*it); - } + for (const std::string& v : mVarNames) + esm.writeHCString(v); esm.endRecord("SCVR"); } esm.startSubRecord("SCDT"); - esm.write(reinterpret_cast(mScriptData.data()), mData.mScriptDataSize); + esm.write(reinterpret_cast(mScriptData.data()), mScriptData.size()); esm.endRecord("SCDT"); esm.writeHNOString("SCTX", mScriptText); @@ -191,9 +208,9 @@ namespace ESM void Script::blank() { mRecordFlags = 0; - mData.mNumShorts = mData.mNumLongs = mData.mNumFloats = 0; - mData.mScriptDataSize = 0; - mData.mStringTableSize = 0; + mNumShorts = 0; + mNumLongs = 0; + mNumFloats = 0; mVarNames.clear(); mScriptData.clear(); @@ -204,4 +221,9 @@ namespace ESM mScriptText = "Begin " + stringId + "\n\nEnd " + stringId + "\n"; } + std::uint32_t computeScriptStringTableSize(const std::vector& varNames) + { + return std::accumulate(varNames.begin(), varNames.end(), static_cast(0), + [](std::uint32_t r, const std::string& v) { return r + 1 + static_cast(v.size()); }); + } } diff --git a/components/esm3/loadscpt.hpp b/components/esm3/loadscpt.hpp index 61b27f1423..2b98807e7b 100644 --- a/components/esm3/loadscpt.hpp +++ b/components/esm3/loadscpt.hpp @@ -1,11 +1,12 @@ #ifndef OPENMW_ESM_SCPT_H #define OPENMW_ESM_SCPT_H +#include #include #include -#include "components/esm/defs.hpp" -#include "components/esm/refid.hpp" +#include +#include namespace ESM { @@ -25,22 +26,12 @@ namespace ESM /// Return a string descriptor for this record type. Currently used for debugging / error logs only. static std::string_view getRecordType() { return "Script"; } - struct SCHDstruct - { - /// Data from script-precompling in the editor. - /// \warning Do not use them. OpenCS currently does not precompile scripts. - uint32_t mNumShorts, mNumLongs, mNumFloats, mScriptDataSize, mStringTableSize; - }; - struct SCHD - { - std::string mName; - Script::SCHDstruct mData; - }; - uint32_t mRecordFlags; RefId mId; - SCHDstruct mData; + std::uint32_t mNumShorts; + std::uint32_t mNumLongs; + std::uint32_t mNumFloats; /// Variable names generated by script-precompiling in the editor. /// \warning Do not use this field. OpenCS currently does not precompile scripts. @@ -58,9 +49,8 @@ namespace ESM void blank(); ///< Set record to default state (does not touch the ID/index). - - private: - void loadSCVR(ESMReader& esm); }; + + std::uint32_t computeScriptStringTableSize(const std::vector& varNames); } #endif