From d1f4a370970066effee87b79a7a690f3a0888ac9 Mon Sep 17 00:00:00 2001 From: yyc12345 Date: Mon, 2 Mar 2026 12:30:02 +0800 Subject: [PATCH] feat: finish dup mesh/mtl/tex check rule in BMapInspector - also fix "use after free" issue in QuoteObjectNames. --- Ballance/BMapInspector/Rule.cpp | 1 + Ballance/BMapInspector/Rule/Shared.hpp | 8 +- Ballance/BMapInspector/Rule/YYCRules.cpp | 111 +++++++++++++++++++---- Ballance/BMapInspector/Rule/ZZQRules.cpp | 2 - 4 files changed, 102 insertions(+), 20 deletions(-) diff --git a/Ballance/BMapInspector/Rule.cpp b/Ballance/BMapInspector/Rule.cpp index 5917a6f..5c900a7 100644 --- a/Ballance/BMapInspector/Rule.cpp +++ b/Ballance/BMapInspector/Rule.cpp @@ -28,6 +28,7 @@ namespace BMapInspector::Rule { rules.emplace_back(new ChirsRule1()); rules.emplace_back(new YYCRule1()); rules.emplace_back(new YYCRule2()); + rules.emplace_back(new YYCRule3()); rules.emplace_back(new BBugRule1()); rules.emplace_back(new ZZQRule1()); rules.emplace_back(new ZZQRule2()); diff --git a/Ballance/BMapInspector/Rule/Shared.hpp b/Ballance/BMapInspector/Rule/Shared.hpp index b515754..aef2fd8 100644 --- a/Ballance/BMapInspector/Rule/Shared.hpp +++ b/Ballance/BMapInspector/Rule/Shared.hpp @@ -164,10 +164,14 @@ namespace BMapInspector::Rule::Shared { requires std::is_pointer_v> && std::is_base_of_v>> std::u8string QuoteObjectNames(InputIt first, InputIt last) { + std::u8string cache; return yycc::string::op::join( - [&first, &last]() -> std::optional { + [&cache, &first, &last]() -> std::optional { if (first == last) return std::nullopt; - return QuoteObjectName(*(first++)); + // YYC MARK: + // We must use "cache", otherwise "use after free" will occur. + cache = QuoteObjectName(*(first++)); + return cache; }, u8", "); } diff --git a/Ballance/BMapInspector/Rule/YYCRules.cpp b/Ballance/BMapInspector/Rule/YYCRules.cpp index 1bdbd4d..a091efc 100644 --- a/Ballance/BMapInspector/Rule/YYCRules.cpp +++ b/Ballance/BMapInspector/Rule/YYCRules.cpp @@ -8,6 +8,7 @@ #include #include #include +#include namespace L = LibCmo; namespace C = LibCmo::CK2; @@ -182,7 +183,7 @@ namespace BMapInspector::Rule { void update_array(const T* addr, size_t cnt) { std::hash hasher; for (size_t i = 0; i < cnt; ++i) { - combine(hasher(addr[i])) + combine(hasher(addr[i])); } } @@ -416,7 +417,10 @@ namespace BMapInspector::Rule { // Compare face data arrays if (!std::equal(lhs->GetFaceIndices(), lhs->GetFaceIndices() + face_count * 3, rhs->GetFaceIndices())) return false; - if (!std::equal(lhs->GetFaceMaterialSlotIndexs(), lhs->GetFaceMaterialSlotIndexs() + face_count, rhs->GetFaceMaterialSlotIndexs())) return false; + if (!std::equal(lhs->GetFaceMaterialSlotIndexs(), + lhs->GetFaceMaterialSlotIndexs() + face_count, + rhs->GetFaceMaterialSlotIndexs())) + return false; // Compare material slot count auto material_slot_count = lhs->GetMaterialSlotCount(); @@ -441,7 +445,7 @@ namespace BMapInspector::Rule { public: O::CKTexture* GetTexture() const { return texture; } - size_t GetHash() const { + size_t GetHash() const { if (!hash.has_value()) hash = hasher(texture); return hash.value(); } @@ -460,7 +464,7 @@ namespace BMapInspector::Rule { public: O::CKMaterial* GetMaterial() const { return material; } - size_t GetHash() const { + size_t GetHash() const { if (!hash.has_value()) hash = hasher(material); return hash.value(); } @@ -476,10 +480,10 @@ namespace BMapInspector::Rule { CKMeshWrapper(O::CKMesh* mesh) : mesh(mesh), hasher(), hash(std::nullopt) {} ~CKMeshWrapper() {} YYCC_DEFAULT_COPY_MOVE(CKMeshWrapper) - + public: O::CKMesh* GetMesh() const { return mesh; } - size_t GetHash() const { + size_t GetHash() const { if (!hash.has_value()) hash = hasher(mesh); return hash.value(); } @@ -495,26 +499,21 @@ namespace BMapInspector::Rule { #pragma region CKObject Wrapper Hash and Equal struct CKTextureWrapperHash { - [[nodiscard]] size_t operator()(const CKTextureWrapper& tex) const noexcept { - return tex.GetHash(); - } + [[nodiscard]] size_t operator()(const CKTextureWrapper& tex) const noexcept { return tex.GetHash(); } }; struct CKMaterialWrapperHash { - [[nodiscard]] size_t operator()(const CKMaterialWrapper& mtl) const noexcept { - return mtl.GetHash(); - } + [[nodiscard]] size_t operator()(const CKMaterialWrapper& mtl) const noexcept { return mtl.GetHash(); } }; struct CKMeshWrapperHash { - [[nodiscard]] size_t operator()(const CKMeshWrapper& mesh) const noexcept { - return mesh.GetHash(); - } + [[nodiscard]] size_t operator()(const CKMeshWrapper& mesh) const noexcept { return mesh.GetHash(); } }; struct CKTextureWrapperEqualTo { CKTextureEqualTo equal_to; [[nodiscard]] bool operator()(const CKTextureWrapper& lhs, const CKTextureWrapper& rhs) const { + if (lhs.GetHash() != rhs.GetHash()) return false; return equal_to(lhs.GetTexture(), rhs.GetTexture()); } }; @@ -522,6 +521,7 @@ namespace BMapInspector::Rule { struct CKMaterialWrapperEqualTo { CKMaterialEqualTo equal_to; [[nodiscard]] bool operator()(const CKMaterialWrapper& lhs, const CKMaterialWrapper& rhs) const { + if (lhs.GetHash() != rhs.GetHash()) return false; return equal_to(lhs.GetMaterial(), rhs.GetMaterial()); } }; @@ -529,6 +529,7 @@ namespace BMapInspector::Rule { struct CKMeshWrapperEqualTo { CKMeshEqualTo equal_to; [[nodiscard]] bool operator()(const CKMeshWrapper& lhs, const CKMeshWrapper& rhs) const { + if (lhs.GetHash() != rhs.GetHash()) return false; return equal_to(lhs.GetMesh(), rhs.GetMesh()); } }; @@ -545,7 +546,85 @@ namespace BMapInspector::Rule { return YYC3; } - void YYCRule3::Check(Reporter::Reporter& reporter, Map::Level& level) const {} + void YYCRule3::Check(Reporter::Reporter& reporter, Map::Level& level) const { + // Check textures + std::unordered_multiset textures; + for (auto* tex : level.GetTextures()) { + textures.emplace(CKTextureWrapper(tex)); + } + // Show result + for (auto it = textures.begin(); it != textures.end();) { + size_t count = textures.count(*it); + + // all count elements have equivalent keys + if (count > 1) { + std::vector dup_texs; + for (size_t i = 0; i < count; ++i) { + dup_texs.emplace_back(it->GetTexture()); + ++it; + } + + reporter.FormatInfo( + YYC3, + u8"Some textures are exactly same. Please consider merge them into one to reduce the final size of map. These textures are: %s", + Shared::QuoteObjectNames(dup_texs.begin(), dup_texs.end()).c_str()); + } else { + ++it; + } + } + + // Check materials + std::unordered_multiset materials; + for (auto* mat : level.GetMaterials()) { + materials.emplace(CKMaterialWrapper(mat)); + } + // Show result + for (auto it = materials.begin(); it != materials.end();) { + size_t count = materials.count(*it); + + // all count elements have equivalent keys + if (count > 1) { + std::vector dup_mtls; + for (size_t i = 0; i < count; ++i) { + dup_mtls.emplace_back(it->GetMaterial()); + ++it; + } + + reporter.FormatInfo( + YYC3, + u8"Some materials are exactly same. Please consider merge them into one to reduce the final size of map. These materials are: %s", + Shared::QuoteObjectNames(dup_mtls.begin(), dup_mtls.end()).c_str()); + } else { + ++it; + } + } + + // Check meshes + std::unordered_multiset meshes; + for (auto* mesh : level.GetMeshes()) { + meshes.emplace(CKMeshWrapper(mesh)); + } + // Show result + for (auto it = meshes.begin(); it != meshes.end();) { + size_t count = meshes.count(*it); + + // all count elements have equivalent keys + if (count > 1) { + std::vector dup_meshes; + for (size_t i = 0; i < count; ++i) { + dup_meshes.emplace_back(it->GetMesh()); + ++it; + } + + reporter.FormatInfo( + YYC3, + u8"Some meshes are exactly same. Please consider merge them into one to reduce the final size of map. These meshes are: %s", + Shared::QuoteObjectNames(dup_meshes.begin(), dup_meshes.end()).c_str()); + } else { + ++it; + } + } + } #pragma endregion diff --git a/Ballance/BMapInspector/Rule/ZZQRules.cpp b/Ballance/BMapInspector/Rule/ZZQRules.cpp index 4abdcce..7b57e7c 100644 --- a/Ballance/BMapInspector/Rule/ZZQRules.cpp +++ b/Ballance/BMapInspector/Rule/ZZQRules.cpp @@ -129,8 +129,6 @@ namespace BMapInspector::Rule { auto left_sector_idx = static_cast(i + 1); auto right_sector_idx = static_cast(j + 1); - // Join object together - // Output result. reporter.FormatWarning(ZZQ2, u8"Some objects are grouped into sector %" PRIuCKDWORD " and sector %" PRIuCKDWORD