From b39ccbeeeff99e480d89548cd4495a9bdd85cc15 Mon Sep 17 00:00:00 2001 From: elsid Date: Sun, 18 Sep 2022 13:19:21 +0200 Subject: [PATCH] Remove NIFFile::warn and NIFFile::fail functions These functions use NIFFile only as context, they are not really a part of either reading nor state invariant. And they only confuse reader because it's not immediatelly obvious that no code is executed after fail. --- components/nif/data.cpp | 12 +++++++---- components/nif/exception.hpp | 21 +++++++++++++++++++ components/nif/niffile.cpp | 36 +++++++++++---------------------- components/nif/niffile.hpp | 6 ------ components/nif/nifkey.hpp | 4 +++- components/nif/node.cpp | 4 +++- components/nifosg/nifloader.cpp | 19 ++++++++++------- 7 files changed, 59 insertions(+), 43 deletions(-) create mode 100644 components/nif/exception.hpp diff --git a/components/nif/data.cpp b/components/nif/data.cpp index f2d533cbce..72395a0c54 100644 --- a/components/nif/data.cpp +++ b/components/nif/data.cpp @@ -1,7 +1,10 @@ #include "data.hpp" +#include "exception.hpp" #include "nifkey.hpp" #include "node.hpp" +#include + namespace Nif { void NiSkinInstance::read(NIFStream* nif) @@ -21,16 +24,16 @@ namespace Nif bones.post(nif); if (data.empty() || root.empty()) - nif->fail("NiSkinInstance missing root or data"); + throw Nif::Exception("NiSkinInstance missing root or data", nif->getFilename()); size_t bnum = bones.length(); if (bnum != data->bones.size()) - nif->fail("Mismatch in NiSkinData bone count"); + throw Nif::Exception("Mismatch in NiSkinData bone count", nif->getFilename()); for (size_t i = 0; i < bnum; i++) { if (bones[i].empty()) - nif->fail("Oops: Missing bone! Don't know how to handle this."); + throw Nif::Exception("Oops: Missing bone! Don't know how to handle this.", nif->getFilename()); bones[i]->setBone(); } } @@ -470,7 +473,8 @@ namespace Nif { palette = nif->getString(); if (nif->getUInt() != palette.size()) - nif->file->warn("Failed size check in NiStringPalette"); + Log(Debug::Warning) << "NIFFile Warning: Failed size check in NiStringPalette. File: " + << nif->file->getFilename(); } void NiBoolData::read(NIFStream* nif) diff --git a/components/nif/exception.hpp b/components/nif/exception.hpp new file mode 100644 index 0000000000..15f0e76d70 --- /dev/null +++ b/components/nif/exception.hpp @@ -0,0 +1,21 @@ +#ifndef OPENMW_COMPONENTS_NIF_EXCEPTION_HPP +#define OPENMW_COMPONENTS_NIF_EXCEPTION_HPP + +#include +#include +#include + +#include + +namespace Nif +{ + struct Exception : std::runtime_error + { + explicit Exception(const std::string& message, const std::filesystem::path& path) + : std::runtime_error("NIFFile Error: " + message + " when reading " + Files::pathToUnicodeString(path)) + { + } + }; +} + +#endif diff --git a/components/nif/niffile.cpp b/components/nif/niffile.cpp index 8aa1000a6d..5afc10eb32 100644 --- a/components/nif/niffile.cpp +++ b/components/nif/niffile.cpp @@ -15,6 +15,7 @@ #include "controller.hpp" #include "data.hpp" #include "effect.hpp" +#include "exception.hpp" #include "extra.hpp" #include "physics.hpp" #include "property.hpp" @@ -200,7 +201,7 @@ namespace Nif const bool supportedHeader = std::any_of(verStrings.begin(), verStrings.end(), [&](const std::string& verString) { return head.compare(0, verString.size(), verString) == 0; }); if (!supportedHeader) - fail("Invalid NIF header: " + head); + throw Nif::Exception("Invalid NIF header: " + head, filename); // Get BCD version ver = nif.getUInt(); @@ -214,9 +215,10 @@ namespace Nif if (!supportedVersion) { if (sLoadUnsupportedFiles) - warn("Unsupported NIF version: " + printVersion(ver) + ". Proceed with caution!"); + Log(Debug::Warning) << " NIFFile Warning: Unsupported NIF version: " << printVersion(ver) + << ". Proceed with caution! File: " << filename; else - fail("Unsupported NIF version: " + printVersion(ver)); + throw Nif::Exception("Unsupported NIF version: " + printVersion(ver), filename); } // NIF data endianness @@ -224,7 +226,7 @@ namespace Nif { unsigned char endianness = nif.getChar(); if (endianness == 0) - fail("Big endian NIF files are unsupported"); + throw Nif::Exception("Big endian NIF files are unsupported", filename); } // User version @@ -296,24 +298,19 @@ namespace Nif { std::stringstream error; error << "Record number " << i << " out of " << recNum << " is blank."; - fail(error.str()); + throw Nif::Exception(error.str(), filename); } // Record separator. Some Havok records in Oblivion do not have it. if (hasRecordSeparators && rec.compare(0, 3, "bhk")) - { if (nif.getInt()) - { - std::stringstream warning; - warning << "Record number " << i << " out of " << recNum << " is preceded by a non-zero separator."; - warn(warning.str()); - } - } + Log(Debug::Warning) << "NIFFile Warning: Record number " << i << " out of " << recNum + << " is preceded by a non-zero separator. File: " << filename; const auto entry = factories.find(rec); if (entry == factories.end()) - fail("Unknown record type " + rec); + throw Nif::Exception("Unknown record type " + rec, filename); r = entry->second(); @@ -343,7 +340,8 @@ namespace Nif else { roots[i] = nullptr; - warn("Root " + std::to_string(i + 1) + " does not point to a record: index " + std::to_string(idx)); + Log(Debug::Warning) << "NIFFile Warning: Root " << i + 1 << " does not point to a record: index " << idx + << ". File: " << filename; } } @@ -369,16 +367,6 @@ namespace Nif sLoadUnsupportedFiles = load; } - void NIFFile::warn(const std::string& msg) const - { - Log(Debug::Warning) << " NIFFile Warning: " << msg << "\nFile: " << filename; - } - - [[noreturn]] void NIFFile::fail(const std::string& msg) const - { - throw std::runtime_error(" NIFFile Error: " + msg + "\nFile: " + Files::pathToUnicodeString(filename)); - } - std::string NIFFile::getString(uint32_t index) const { if (index == std::numeric_limits::max()) diff --git a/components/nif/niffile.hpp b/components/nif/niffile.hpp index 21b0fa6892..8a3938a00f 100644 --- a/components/nif/niffile.hpp +++ b/components/nif/niffile.hpp @@ -94,12 +94,6 @@ namespace Nif BETHVER_FO4 = 130 // Fallout 4 }; - /// Used if file parsing fails - [[noreturn]] void fail(const std::string& msg) const; - - /// Used when something goes wrong, but not catastrophically so - void warn(const std::string& msg) const; - /// Open a NIF stream. The name is used for error messages. NIFFile(Files::IStreamPtr&& stream, const std::filesystem::path& name); diff --git a/components/nif/nifkey.hpp b/components/nif/nifkey.hpp index eda98eed76..8698cc4510 100644 --- a/components/nif/nifkey.hpp +++ b/components/nif/nifkey.hpp @@ -5,6 +5,7 @@ #include +#include "exception.hpp" #include "niffile.hpp" #include "nifstream.hpp" @@ -112,7 +113,8 @@ namespace Nif } else if (count != 0) { - nif->file->fail("Unhandled interpolation type: " + std::to_string(mInterpolationType)); + throw Nif::Exception( + "Unhandled interpolation type: " + std::to_string(mInterpolationType), nif->file->getFilename()); } } diff --git a/components/nif/node.cpp b/components/nif/node.cpp index 8c604a59fa..05165a1a48 100644 --- a/components/nif/node.cpp +++ b/components/nif/node.cpp @@ -3,6 +3,7 @@ #include #include "data.hpp" +#include "exception.hpp" #include "physics.hpp" #include "property.hpp" @@ -68,7 +69,8 @@ namespace Nif } default: { - nif->file->fail("Unhandled NiBoundingVolume type: " + std::to_string(type)); + throw Nif::Exception( + "Unhandled NiBoundingVolume type: " + std::to_string(type), nif->file->getFilename()); } } } diff --git a/components/nifosg/nifloader.cpp b/components/nifosg/nifloader.cpp index 55be6a1dc3..7d89e9de8a 100644 --- a/components/nifosg/nifloader.cpp +++ b/components/nifosg/nifloader.cpp @@ -42,6 +42,7 @@ #include #include +#include #include #include #include @@ -265,15 +266,17 @@ namespace NifOsg if (!seq) { - nif->warn("Found no NiSequenceStreamHelper root record"); + Log(Debug::Warning) << "NIFFile Warning: Found no NiSequenceStreamHelper root record. File: " + << nif->getFilename(); return; } Nif::ExtraPtr extra = seq->extra; if (extra.empty() || extra->recType != Nif::RC_NiTextKeyExtraData) { - nif->warn("First extra data was not a NiTextKeyExtraData, but a " - + (extra.empty() ? std::string("nil") : extra->recName) + "."); + Log(Debug::Warning) << "NIFFile Warning: First extra data was not a NiTextKeyExtraData, but a " + << (extra.empty() ? std::string_view("nil") : std::string_view(extra->recName)) + << ". File: " << nif->getFilename(); return; } @@ -285,7 +288,8 @@ namespace NifOsg { if (extra->recType != Nif::RC_NiStringExtraData || ctrl->recType != Nif::RC_NiKeyframeController) { - nif->warn("Unexpected extra data " + extra->recName + " with controller " + ctrl->recName); + Log(Debug::Warning) << "NIFFile Warning: Unexpected extra data " << extra->recName + << " with controller " << ctrl->recName << ". File: " << nif->getFilename(); continue; } @@ -328,7 +332,7 @@ namespace NifOsg roots.emplace_back(nifNode); } if (roots.empty()) - nif->fail("Found no root nodes"); + throw Nif::Exception("Found no root nodes", nif->getFilename()); osg::ref_ptr textkeys(new SceneUtil::TextKeyMapHolder); @@ -1198,8 +1202,9 @@ namespace NifOsg osg::Group* emitterNode = findEmitterNode.mFound; if (!emitterNode) { - nif->warn("Failed to find particle emitter emitter node (node record index " - + std::to_string(recIndex) + ")"); + Log(Debug::Warning) + << "NIFFile Warning: Failed to find particle emitter emitter node (node record index " + << recIndex << "). File: " << nif->getFilename(); continue; }