From 3305b400dcbeff24c39247c3317f1a88be1d296b Mon Sep 17 00:00:00 2001 From: elsid Date: Fri, 28 Jan 2022 18:40:17 +0100 Subject: [PATCH] Use ESM::NAME instead of const char* and std::string as argument type --- apps/esmtool/esmtool.cpp | 6 +-- .../esm/test_fixed_string.cpp | 31 ++++++++------ components/esm/esmcommon.hpp | 42 +++++++++++++++++++ components/esm3/activespells.cpp | 4 +- components/esm3/aipackage.cpp | 2 +- components/esm3/cellref.cpp | 8 ++-- components/esm3/cellref.hpp | 5 ++- components/esm3/esmreader.cpp | 16 +++---- components/esm3/esmreader.hpp | 22 +++++----- components/esm3/esmwriter.cpp | 31 +++++--------- components/esm3/esmwriter.hpp | 32 +++++++------- components/esm3/loadlevlist.hpp | 16 ++++--- components/esm3/weatherstate.cpp | 22 +++++----- 13 files changed, 137 insertions(+), 100 deletions(-) diff --git a/apps/esmtool/esmtool.cpp b/apps/esmtool/esmtool.cpp index 4fe0b248af..a7946c77a6 100644 --- a/apps/esmtool/esmtool.cpp +++ b/apps/esmtool/esmtool.cpp @@ -484,9 +484,9 @@ int clone(Arguments& info) if (i <= 0) break; - const ESM::NAME& typeName = record->getType(); + const ESM::NAME typeName = record->getType(); - esm.startRecord(typeName.toString(), record->getFlags()); + esm.startRecord(typeName, record->getFlags()); record->save(esm); if (typeName.toInt() == ESM::REC_CELL) { @@ -498,7 +498,7 @@ int clone(Arguments& info) } } - esm.endRecord(typeName.toString()); + esm.endRecord(typeName); saved++; int perc = recordCount == 0 ? 100 : (int)((saved / (float)recordCount)*100); diff --git a/apps/openmw_test_suite/esm/test_fixed_string.cpp b/apps/openmw_test_suite/esm/test_fixed_string.cpp index 26fb1590d5..1189f667e5 100644 --- a/apps/openmw_test_suite/esm/test_fixed_string.cpp +++ b/apps/openmw_test_suite/esm/test_fixed_string.cpp @@ -1,12 +1,12 @@ #include #include "components/esm/esmcommon.hpp" +#include "components/esm/defs.hpp" TEST(EsmFixedString, operator__eq_ne) { { SCOPED_TRACE("asdc == asdc"); - ESM::NAME name; - name.assign("asdc"); + constexpr ESM::NAME name("asdc"); char s[4] = {'a', 's', 'd', 'c'}; std::string ss(s, 4); @@ -16,8 +16,7 @@ TEST(EsmFixedString, operator__eq_ne) } { SCOPED_TRACE("asdc == asdcx"); - ESM::NAME name; - name.assign("asdc"); + constexpr ESM::NAME name("asdc"); char s[5] = {'a', 's', 'd', 'c', 'x'}; std::string ss(s, 5); @@ -27,8 +26,7 @@ TEST(EsmFixedString, operator__eq_ne) } { SCOPED_TRACE("asdc == asdc[NULL]"); - ESM::NAME name; - name.assign("asdc"); + const ESM::NAME name("asdc"); char s[5] = {'a', 's', 'd', 'c', '\0'}; std::string ss(s, 5); @@ -41,8 +39,7 @@ TEST(EsmFixedString, operator__eq_ne_const) { { SCOPED_TRACE("asdc == asdc (const)"); - ESM::NAME name; - name.assign("asdc"); + constexpr ESM::NAME name("asdc"); const char s[4] = { 'a', 's', 'd', 'c' }; std::string ss(s, 4); @@ -52,8 +49,7 @@ TEST(EsmFixedString, operator__eq_ne_const) } { SCOPED_TRACE("asdc == asdcx (const)"); - ESM::NAME name; - name.assign("asdc"); + constexpr ESM::NAME name("asdc"); const char s[5] = { 'a', 's', 'd', 'c', 'x' }; std::string ss(s, 5); @@ -63,8 +59,7 @@ TEST(EsmFixedString, operator__eq_ne_const) } { SCOPED_TRACE("asdc == asdc[NULL] (const)"); - ESM::NAME name; - name.assign("asdc"); + constexpr ESM::NAME name("asdc"); const char s[5] = { 'a', 's', 'd', 'c', '\0' }; std::string ss(s, 5); @@ -148,3 +143,15 @@ TEST(EsmFixedString, assignment_operator_is_supported_for_uint32) value = static_cast(0xFEDCBA98u); EXPECT_EQ(value, static_cast(0xFEDCBA98u)) << value.toInt(); } + +TEST(EsmFixedString, construction_from_uint32_is_supported) +{ + constexpr ESM::NAME value(0xFEDCBA98u); + EXPECT_EQ(value, static_cast(0xFEDCBA98u)) << value.toInt(); +} + +TEST(EsmFixedString, construction_from_RecNameInts_is_supported) +{ + constexpr ESM::NAME value(ESM::RecNameInts::REC_ACTI); + EXPECT_EQ(value, static_cast(ESM::RecNameInts::REC_ACTI)) << value.toInt(); +} diff --git a/components/esm/esmcommon.hpp b/components/esm/esmcommon.hpp index 921eeed744..7ca059020f 100644 --- a/components/esm/esmcommon.hpp +++ b/components/esm/esmcommon.hpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include namespace ESM { @@ -30,6 +32,41 @@ struct FixedString char mData[capacity]; + FixedString() = default; + + template + constexpr FixedString(const char (&value)[size]) noexcept + : mData() + { + if constexpr (capacity == sizeof(std::uint32_t)) + { + static_assert(capacity == size || capacity + 1 == size); + if constexpr (capacity + 1 == size) + assert(value[capacity] == '\0'); + for (std::size_t i = 0; i < capacity; ++i) + mData[i] = value[i]; + } + else + { + const std::size_t length = std::min(capacity, size); + for (std::size_t i = 0; i < length; ++i) + mData[i] = value[i]; + mData[std::min(capacity - 1, length)] = '\0'; + } + } + + constexpr explicit FixedString(std::uint32_t value) noexcept + : mData() + { + static_assert(capacity == sizeof(std::uint32_t)); + for (std::size_t i = 0; i < capacity; ++i) + mData[i] = static_cast((value >> (i * std::numeric_limits::digits)) & std::numeric_limits::max()); + } + + template + constexpr explicit FixedString(T value) noexcept + : FixedString(static_cast(value)) {} + std::string_view toStringView() const noexcept { return std::string_view(mData, strnlen(mData, capacity)); @@ -116,6 +153,11 @@ inline bool operator==(const FixedString<4>& lhs, std::uint32_t rhs) noexcept return lhs.toInt() == rhs; } +inline bool operator==(const FixedString<4>& lhs, const FixedString<4>& rhs) noexcept +{ + return lhs.toInt() == rhs.toInt(); +} + template inline bool operator!=(const FixedString& lhs, const Rhs& rhs) noexcept { diff --git a/components/esm3/activespells.cpp b/components/esm3/activespells.cpp index 22f862b6e4..61be6a7838 100644 --- a/components/esm3/activespells.cpp +++ b/components/esm3/activespells.cpp @@ -5,7 +5,7 @@ namespace { - void save(ESM::ESMWriter& esm, const std::vector& spells, const std::string& tag) + void save(ESM::ESMWriter& esm, const std::vector& spells, ESM::NAME tag) { for (const auto& params : spells) { @@ -38,7 +38,7 @@ namespace } } - void load(ESM::ESMReader& esm, std::vector& spells, const char* tag) + void load(ESM::ESMReader& esm, std::vector& spells, ESM::NAME tag) { int format = esm.getFormat(); diff --git a/components/esm3/aipackage.cpp b/components/esm3/aipackage.cpp index fa20d271c0..5a95e58ca8 100644 --- a/components/esm3/aipackage.cpp +++ b/components/esm3/aipackage.cpp @@ -65,7 +65,7 @@ namespace ESM case AI_Escort: case AI_Follow: { - const char *name = (it->mType == AI_Escort) ? "AI_E" : "AI_F"; + const ESM::NAME name = (it->mType == AI_Escort) ? ESM::NAME("AI_E") : ESM::NAME("AI_F"); esm.writeHNT(name, it->mTarget, sizeof(it->mTarget)); esm.writeHNOCString("CNDT", it->mCellName); break; diff --git a/components/esm3/cellref.cpp b/components/esm3/cellref.cpp index 002a885d92..8487b3c0f0 100644 --- a/components/esm3/cellref.cpp +++ b/components/esm3/cellref.cpp @@ -5,15 +5,15 @@ #include "esmreader.hpp" #include "esmwriter.hpp" -void ESM::RefNum::load (ESMReader& esm, bool wide, const std::string& tag) +void ESM::RefNum::load(ESMReader& esm, bool wide, ESM::NAME tag) { if (wide) - esm.getHNT (*this, tag.c_str(), 8); + esm.getHNT(*this, tag, 8); else - esm.getHNT (mIndex, tag.c_str()); + esm.getHNT(mIndex, tag); } -void ESM::RefNum::save (ESMWriter &esm, bool wide, const std::string& tag) const +void ESM::RefNum::save(ESMWriter &esm, bool wide, ESM::NAME tag) const { if (wide) esm.writeHNT (tag, *this, 8); diff --git a/components/esm3/cellref.hpp b/components/esm3/cellref.hpp index 55e4b700fc..cff635f455 100644 --- a/components/esm3/cellref.hpp +++ b/components/esm3/cellref.hpp @@ -5,6 +5,7 @@ #include #include "components/esm/defs.hpp" +#include "components/esm/esmcommon.hpp" namespace ESM { @@ -18,9 +19,9 @@ namespace ESM unsigned int mIndex; int mContentFile; - void load (ESMReader& esm, bool wide = false, const std::string& tag = "FRMR"); + void load(ESMReader& esm, bool wide = false, ESM::NAME tag = "FRMR"); - void save (ESMWriter &esm, bool wide = false, const std::string& tag = "FRMR") const; + void save(ESMWriter &esm, bool wide = false, ESM::NAME tag = "FRMR") const; inline bool hasContentFile() const { return mContentFile >= 0; } diff --git a/components/esm3/esmreader.cpp b/components/esm3/esmreader.cpp index 165263d6e4..47974e45a8 100644 --- a/components/esm3/esmreader.cpp +++ b/components/esm3/esmreader.cpp @@ -113,14 +113,14 @@ void ESMReader::open(const std::string &file) open (Files::openConstrainedFileStream (file.c_str ()), file); } -std::string ESMReader::getHNOString(const char* name) +std::string ESMReader::getHNOString(NAME name) { if (isNextSub(name)) return getHString(); return ""; } -std::string ESMReader::getHNString(const char* name) +std::string ESMReader::getHNString(NAME name) { getSubNameIs(name); return getHString(); @@ -156,21 +156,21 @@ void ESMReader::getHExact(void*p, int size) } // Read the given number of bytes from a named subrecord -void ESMReader::getHNExact(void*p, int size, const char* name) +void ESMReader::getHNExact(void*p, int size, NAME name) { getSubNameIs(name); getHExact(p, size); } // Get the next subrecord name and check if it matches the parameter -void ESMReader::getSubNameIs(const char* name) +void ESMReader::getSubNameIs(NAME name) { getSubName(); if (mCtx.subName != name) - fail("Expected subrecord " + std::string(name) + " but got " + mCtx.subName.toString()); + fail("Expected subrecord " + name.toString() + " but got " + mCtx.subName.toString()); } -bool ESMReader::isNextSub(const char* name) +bool ESMReader::isNextSub(NAME name) { if (!hasMoreSubs()) return false; @@ -185,7 +185,7 @@ bool ESMReader::isNextSub(const char* name) return !mCtx.subCached; } -bool ESMReader::peekNextSub(const char *name) +bool ESMReader::peekNextSub(NAME name) { if (!hasMoreSubs()) return false; @@ -226,7 +226,7 @@ void ESMReader::skipHSubSize(int size) reportSubSizeMismatch(mCtx.leftSub, size); } -void ESMReader::skipHSubUntil(const char *name) +void ESMReader::skipHSubUntil(NAME name) { while (hasMoreSubs() && !isNextSub(name)) { diff --git a/components/esm3/esmreader.hpp b/components/esm3/esmreader.hpp index 3b384dc603..e370946ee9 100644 --- a/components/esm3/esmreader.hpp +++ b/components/esm3/esmreader.hpp @@ -98,7 +98,7 @@ public: // Read data of a given type, stored in a subrecord of a given name template - void getHNT(X &x, const char* name) + void getHNT(X &x, NAME name) { getSubNameIs(name); getHT(x); @@ -106,7 +106,7 @@ public: // Optional version of getHNT template - void getHNOT(X &x, const char* name) + void getHNOT(X &x, NAME name) { if(isNextSub(name)) getHT(x); @@ -115,7 +115,7 @@ public: // Version with extra size checking, to make sure the compiler // doesn't mess up our struct padding. template - void getHNT(X &x, const char* name, int size) + void getHNT(X &x, NAME name, int size) { assert(sizeof(X) == size); getSubNameIs(name); @@ -123,7 +123,7 @@ public: } template - void getHNOT(X &x, const char* name, int size) + void getHNOT(X &x, NAME name, int size) { assert(sizeof(X) == size); if(isNextSub(name)) @@ -150,10 +150,10 @@ public: } // Read a string by the given name if it is the next record. - std::string getHNOString(const char* name); + std::string getHNOString(NAME name); // Read a string with the given sub-record name - std::string getHNString(const char* name); + std::string getHNString(NAME name); // Read a string, including the sub-record header (but not the name) std::string getHString(); @@ -162,7 +162,7 @@ public: void getHExact(void*p, int size); // Read the given number of bytes from a named subrecord - void getHNExact(void*p, int size, const char* name); + void getHNExact(void*p, int size, NAME name); /************************************************************************* * @@ -171,16 +171,16 @@ public: *************************************************************************/ // Get the next subrecord name and check if it matches the parameter - void getSubNameIs(const char* name); + void getSubNameIs(NAME name); /** Checks if the next sub record name matches the parameter. If it does, it is read into 'subName' just as if getSubName() was called. If not, the read name will still be available for future calls to getSubName(), isNextSub() and getSubNameIs(). */ - bool isNextSub(const char* name); + bool isNextSub(NAME name); - bool peekNextSub(const char* name); + bool peekNextSub(NAME name); // Store the current subrecord name for the next call of getSubName() void cacheSubName() {mCtx.subCached = true; }; @@ -197,7 +197,7 @@ public: void skipHSubSize(int size); // Skip all subrecords until the given subrecord or no more subrecords remaining - void skipHSubUntil(const char* name); + void skipHSubUntil(NAME name); /* Sub-record header. This updates leftRec beyond the current sub-record as well. leftSub contains size of current sub-record. diff --git a/components/esm3/esmwriter.cpp b/components/esm3/esmwriter.cpp index d0137c5131..77ba5fc6aa 100644 --- a/components/esm3/esmwriter.cpp +++ b/components/esm3/esmwriter.cpp @@ -86,7 +86,7 @@ namespace ESM throw std::runtime_error ("Unclosed record remaining"); } - void ESMWriter::startRecord(const std::string& name, uint32_t flags) + void ESMWriter::startRecord(ESM::NAME name, uint32_t flags) { mRecordCount++; @@ -105,15 +105,10 @@ namespace ESM void ESMWriter::startRecord (uint32_t name, uint32_t flags) { - std::string type; - for (int i=0; i<4; ++i) - /// \todo make endianess agnostic - type += reinterpret_cast (&name)[i]; - - startRecord (type, flags); + startRecord(ESM::NAME(name), flags); } - void ESMWriter::startSubRecord(const std::string& name) + void ESMWriter::startSubRecord(ESM::NAME name) { // Sub-record hierarchies are not properly supported in ESMReader. This should be fixed later. assert (mRecords.size() <= 1); @@ -129,7 +124,7 @@ namespace ESM assert(mRecords.back().size == 0); } - void ESMWriter::endRecord(const std::string& name) + void ESMWriter::endRecord(ESM::NAME name) { RecordData rec = mRecords.back(); assert(rec.name == name); @@ -147,22 +142,17 @@ namespace ESM void ESMWriter::endRecord (uint32_t name) { - std::string type; - for (int i=0; i<4; ++i) - /// \todo make endianess agnostic - type += reinterpret_cast (&name)[i]; - - endRecord (type); + endRecord(ESM::NAME(name)); } - void ESMWriter::writeHNString(const std::string& name, const std::string& data) + void ESMWriter::writeHNString(ESM::NAME name, const std::string& data) { startSubRecord(name); writeHString(data); endRecord(name); } - void ESMWriter::writeHNString(const std::string& name, const std::string& data, size_t size) + void ESMWriter::writeHNString(ESM::NAME name, const std::string& data, size_t size) { assert(data.size() <= size); startSubRecord(name); @@ -177,7 +167,7 @@ namespace ESM endRecord(name); } - void ESMWriter::writeFixedSizeString(const std::string &data, int size) + void ESMWriter::writeFixedSizeString(const std::string& data, int size) { std::string string; if (!data.empty()) @@ -206,10 +196,9 @@ namespace ESM write("\0", 1); } - void ESMWriter::writeName(const std::string& name) + void ESMWriter::writeName(ESM::NAME name) { - assert((name.size() == 4 && name[3] != '\0')); - write(name.c_str(), name.size()); + write(name.mData, ESM::NAME::sCapacity); } void ESMWriter::write(const char* data, size_t size) diff --git a/components/esm3/esmwriter.hpp b/components/esm3/esmwriter.hpp index 3073daf15e..8ee507f39d 100644 --- a/components/esm3/esmwriter.hpp +++ b/components/esm3/esmwriter.hpp @@ -19,7 +19,7 @@ class ESMWriter { struct RecordData { - std::string name; + ESM::NAME name; std::streampos position; uint32_t size; }; @@ -56,27 +56,27 @@ class ESMWriter void close(); ///< \note Does not close the stream. - void writeHNString(const std::string& name, const std::string& data); - void writeHNString(const std::string& name, const std::string& data, size_t size); - void writeHNCString(const std::string& name, const std::string& data) + void writeHNString(ESM::NAME name, const std::string& data); + void writeHNString(ESM::NAME name, const std::string& data, size_t size); + void writeHNCString(ESM::NAME name, const std::string& data) { startSubRecord(name); writeHCString(data); endRecord(name); } - void writeHNOString(const std::string& name, const std::string& data) + void writeHNOString(ESM::NAME name, const std::string& data) { if (!data.empty()) writeHNString(name, data); } - void writeHNOCString(const std::string& name, const std::string& data) + void writeHNOCString(ESM::NAME name, const std::string& data) { if (!data.empty()) writeHNCString(name, data); } template - void writeHNT(const std::string& name, const T& data) + void writeHNT(ESM::NAME name, const T& data) { startSubRecord(name); writeT(data); @@ -84,7 +84,7 @@ class ESMWriter } template - void writeHNT(const std::string& name, const T (&data)[size]) + void writeHNT(ESM::NAME name, const T (&data)[size]) { startSubRecord(name); writeT(data); @@ -94,15 +94,15 @@ class ESMWriter // Prevent using writeHNT with strings. This already happened by accident and results in // state being discarded without any error on writing or reading it. :( // writeHNString and friends must be used instead. - void writeHNT(const std::string& name, const std::string& data) = delete; + void writeHNT(ESM::NAME name, const std::string& data) = delete; - void writeT(const std::string& data) = delete; + void writeT(ESM::NAME data) = delete; template - void writeHNT(const std::string& name, const T (&data)[size], int) = delete; + void writeHNT(ESM::NAME name, const T (&data)[size], int) = delete; template - void writeHNT(const std::string& name, const T& data, int size) + void writeHNT(ESM::NAME name, const T& data, int size) { startSubRecord(name); writeT(data, size); @@ -129,16 +129,16 @@ class ESMWriter write((char*)&data, size); } - void startRecord(const std::string& name, uint32_t flags = 0); + void startRecord(ESM::NAME name, uint32_t flags = 0); void startRecord(uint32_t name, uint32_t flags = 0); /// @note Sub-record hierarchies are not properly supported in ESMReader. This should be fixed later. - void startSubRecord(const std::string& name); - void endRecord(const std::string& name); + void startSubRecord(ESM::NAME name); + void endRecord(ESM::NAME name); void endRecord(uint32_t name); void writeFixedSizeString(const std::string& data, int size); void writeHString(const std::string& data); void writeHCString(const std::string& data); - void writeName(const std::string& data); + void writeName(ESM::NAME data); void write(const char* data, size_t size); private: diff --git a/components/esm3/loadlevlist.hpp b/components/esm3/loadlevlist.hpp index 0ebd7a64cb..aea85b20e0 100644 --- a/components/esm3/loadlevlist.hpp +++ b/components/esm3/loadlevlist.hpp @@ -4,6 +4,8 @@ #include #include +#include + namespace ESM { @@ -27,7 +29,7 @@ struct LevelledListBase // Record name used to read references. Must be set before load() is // called. - const char *mRecName; + ESM::NAME mRecName; struct LevelItem { @@ -37,6 +39,8 @@ struct LevelledListBase std::vector mList; + explicit LevelledListBase(ESM::NAME recName) : mRecName(recName) {} + void load(ESMReader &esm, bool &isDeleted); void save(ESMWriter &esm, bool isDeleted = false) const; @@ -58,10 +62,7 @@ struct CreatureLevList: LevelledListBase // player. }; - CreatureLevList() - { - mRecName = "CNAM"; - } + CreatureLevList() : LevelledListBase("CNAM") {} }; struct ItemLevList: LevelledListBase @@ -84,10 +85,7 @@ struct ItemLevList: LevelledListBase // player. }; - ItemLevList() - { - mRecName = "INAM"; - } + ItemLevList() : LevelledListBase("INAM") {} }; } diff --git a/components/esm3/weatherstate.cpp b/components/esm3/weatherstate.cpp index cd1a82b0b7..c791aac145 100644 --- a/components/esm3/weatherstate.cpp +++ b/components/esm3/weatherstate.cpp @@ -5,17 +5,17 @@ namespace { - const char* currentRegionRecord = "CREG"; - const char* timePassedRecord = "TMPS"; - const char* fastForwardRecord = "FAST"; - const char* weatherUpdateTimeRecord = "WUPD"; - const char* transitionFactorRecord = "TRFC"; - const char* currentWeatherRecord = "CWTH"; - const char* nextWeatherRecord = "NWTH"; - const char* queuedWeatherRecord = "QWTH"; - const char* regionNameRecord = "RGNN"; - const char* regionWeatherRecord = "RGNW"; - const char* regionChanceRecord = "RGNC"; + constexpr ESM::NAME currentRegionRecord = "CREG"; + constexpr ESM::NAME timePassedRecord = "TMPS"; + constexpr ESM::NAME fastForwardRecord = "FAST"; + constexpr ESM::NAME weatherUpdateTimeRecord = "WUPD"; + constexpr ESM::NAME transitionFactorRecord = "TRFC"; + constexpr ESM::NAME currentWeatherRecord = "CWTH"; + constexpr ESM::NAME nextWeatherRecord = "NWTH"; + constexpr ESM::NAME queuedWeatherRecord = "QWTH"; + constexpr ESM::NAME regionNameRecord = "RGNN"; + constexpr ESM::NAME regionWeatherRecord = "RGNW"; + constexpr ESM::NAME regionChanceRecord = "RGNC"; } namespace ESM