From 8b08973283da92d93b276fd9e474ba9b91af0f14 Mon Sep 17 00:00:00 2001 From: Dmitry Orlov Date: Sun, 25 Apr 2021 15:07:06 +0300 Subject: [PATCH] Fix: ID-collisions when processing large mods collections --- lib/JsonNode.cpp | 34 +++++++++- lib/JsonNode.h | 3 + lib/mapObjects/CObjectClassesHandler.cpp | 79 +++++++++++++++--------- lib/mapObjects/CObjectClassesHandler.h | 3 +- lib/mapObjects/CommonConstructors.cpp | 21 ++++--- 5 files changed, 100 insertions(+), 40 deletions(-) diff --git a/lib/JsonNode.cpp b/lib/JsonNode.cpp index 7af159dc6..57a2c187c 100644 --- a/lib/JsonNode.cpp +++ b/lib/JsonNode.cpp @@ -261,6 +261,28 @@ bool JsonNode::isCompact() const } } +bool JsonNode::TryBoolFromString(bool & success) const +{ + success = true; + if(type == JsonNode::JsonType::DATA_BOOL) + return Bool(); + + success = type == JsonNode::JsonType::DATA_STRING; + if(success) + { + auto boolParamStr = String(); + boost::algorithm::trim(boolParamStr); + boost::algorithm::to_lower(boolParamStr); + success = boolParamStr == "true"; + + if(success) + return true; + + success = boolParamStr == "false"; + } + return false; +} + void JsonNode::clear() { setType(JsonType::DATA_NULL); @@ -623,7 +645,17 @@ std::shared_ptr JsonUtils::parseLimiter(const JsonNode & limiter) { creatureLimiter->setCreature(CreatureID(creature)); }); - creatureLimiter->includeUpgrades = parameters.size() > 1 ? parameters[1].Bool() : false; + auto includeUpgrades = false; + + if(parameters.size() > 1) + { + bool success = true; + includeUpgrades = parameters[1].TryBoolFromString(success); + + if(!success) + logMod->error("Second parameter of '%s' limiter should be Bool", limiterType); + } + creatureLimiter->includeUpgrades = includeUpgrades; return creatureLimiter; } else if(limiterType == "HAS_ANOTHER_BONUS_LIMITER") diff --git a/lib/JsonNode.h b/lib/JsonNode.h index e73f99ad1..6db74d158 100644 --- a/lib/JsonNode.h +++ b/lib/JsonNode.h @@ -87,6 +87,9 @@ public: /// removes all data from node and sets type to null void clear(); + /// returns bool or bool equivalent of string value if 'success' is true, or false otherwise + bool TryBoolFromString(bool & success) const; + /// non-const accessors, node will change type on type mismatch bool & Bool(); double & Float(); diff --git a/lib/mapObjects/CObjectClassesHandler.cpp b/lib/mapObjects/CObjectClassesHandler.cpp index 96dfd6081..a3ca8192a 100644 --- a/lib/mapObjects/CObjectClassesHandler.cpp +++ b/lib/mapObjects/CObjectClassesHandler.cpp @@ -148,30 +148,43 @@ std::vector CObjectClassesHandler::loadLegacyData(size_t dataSize) /// selects preferred ID (or subID) for new object template -si32 selectNextID(const JsonNode & fixedID, const Map & map, si32 defaultID) +si32 selectNextID(const JsonNode & fixedID, const Map & map, si32 fixedObjectsBound) { - if (!fixedID.isNull() && (si32)fixedID.Float() < defaultID) - return static_cast(fixedID.Float()); // H3M object with fixed ID + assert(fixedObjectsBound > 0); + if(fixedID.isNull()) + { + auto lastID = map.empty() ? 0 : map.rbegin()->first; + return lastID < fixedObjectsBound ? fixedObjectsBound : lastID + 1; + } + auto id = static_cast(fixedID.Float()); + if(id >= fixedObjectsBound) + logGlobal->error("Getting next ID overflowed: %d >= %d", id, fixedObjectsBound); - if (map.empty()) - return defaultID; // no objects loaded, keep gap for H3M objects - if (map.rbegin()->first >= defaultID) - return map.rbegin()->first + 1; // some modded objects loaded, return next available - - return defaultID; // some H3M objects loaded, first modded found + return id; } -void CObjectClassesHandler::loadObjectEntry(const std::string & identifier, const JsonNode & entry, ObjectContainter * obj) +void CObjectClassesHandler::loadObjectEntry(const std::string & identifier, const JsonNode & entry, ObjectContainter * obj, bool isSubobject) { - if (!handlerConstructors.count(obj->handlerName)) + static const si32 fixedObjectsBound = 1000; // legacy value for backward compatibilitty + static const si32 fixedSubobjectsBound = 10000000; // large enough arbitrary value to avoid ID-collisions + si32 usedBound = fixedObjectsBound; + + if(!handlerConstructors.count(obj->handlerName)) { logGlobal->error("Handler with name %s was not found!", obj->handlerName); return; } + const auto convertedId = VLC->modh->normalizeIdentifier(entry.meta, "core", identifier); + const auto & entryIndex = entry["index"]; + bool useSelectNextID = !isSubobject || entryIndex.isNull(); - std::string convertedId = VLC->modh->normalizeIdentifier(entry.meta, "core", identifier); - - si32 id = selectNextID(entry["index"], obj->subObjects, 1000); + if(useSelectNextID && isSubobject) + { + usedBound = fixedSubobjectsBound; + logGlobal->error("Subobject index is Null. convertedId = '%s' obj->id = %d", convertedId, obj->id); + } + si32 id = useSelectNextID ? selectNextID(entryIndex, obj->subObjects, usedBound) + : (si32)entryIndex.Float(); auto handler = handlerConstructors.at(obj->handlerName)(); handler->setType(obj->id, id); @@ -196,46 +209,54 @@ void CObjectClassesHandler::loadObjectEntry(const std::string & identifier, cons //some mods redefine content handlers in the decoration.json in such way: //"core:sign" : { "types" : { "forgeSign" : { ... - static const std::vector knownProblemObjects + static const std::vector breakersRMG { "hota.hota decorations:hotaPandoraBox" , "hota.hota decorations:hotaSubterreanGate" }; - bool overrideForce = !obj->subObjects.count(id) || - std::any_of(knownProblemObjects.begin(), knownProblemObjects.end(), [obj, id](const std::string & str) - { - return str.compare(obj->subObjects[id]->subTypeName) == 0; - }); + const bool isExistingKey = obj->subObjects.count(id) > 0; + const bool isBreaker = std::any_of(breakersRMG.begin(), breakersRMG.end(), + [&handler](const std::string & str) + { + return str.compare(handler->subTypeName) == 0; + }); + const bool passedHandler = !isExistingKey && !isBreaker; - if (overrideForce) // DO NOT override mod handlers by default + if(passedHandler) { obj->subObjects[id] = handler; obj->subIds[convertedId] = id; } + else if(isExistingKey) //It's supposed that fan mods handlers are not overridden by default handlers + { + logGlobal->trace("Handler '%s' has not been overridden with handler '%s' in object %s(%d)::%s(%d)", + obj->subObjects[id]->subTypeName, obj->handlerName, obj->identifier, obj->id, convertedId, id); + } else { - logGlobal->warn("Don't override handler %s in object %s(%d)::%s(%d) subTypeName : %s" - , obj->handlerName, obj->identifier, obj->id, convertedId, id, obj->subObjects[id]->subTypeName); + logGlobal->warn("Handler '%s' for object %s(%d)::%s(%d) has not been activated as RMG breaker", + obj->handlerName, obj->identifier, obj->id, convertedId, id); } } CObjectClassesHandler::ObjectContainter * CObjectClassesHandler::loadFromJson(const JsonNode & json, const std::string & name) { auto obj = new ObjectContainter(); + static const si32 fixedObjectsBound = 256; //Legacy value for backward compatibility + obj->identifier = name; obj->name = json["name"].String(); obj->handlerName = json["handler"].String(); obj->base = json["base"]; - obj->id = selectNextID(json["index"], objects, 256); + obj->id = selectNextID(json["index"], objects, fixedObjectsBound); + if(json["defaultAiValue"].isNull()) obj->groupDefaultAiValue = boost::none; else obj->groupDefaultAiValue = static_cast>(json["defaultAiValue"].Integer()); for (auto entry : json["types"].Struct()) - { loadObjectEntry(entry.first, entry.second, obj); - } return obj; } @@ -257,6 +278,8 @@ void CObjectClassesHandler::loadObject(std::string scope, std::string name, cons void CObjectClassesHandler::loadSubObject(const std::string & identifier, JsonNode config, si32 ID, boost::optional subID) { + static const bool isSubObject = true; + config.setType(JsonNode::JsonType::DATA_STRUCT); // ensure that input is not NULL assert(objects.count(ID)); if (subID) @@ -265,10 +288,8 @@ void CObjectClassesHandler::loadSubObject(const std::string & identifier, JsonNo assert(config["index"].isNull()); config["index"].Float() = subID.get(); } - inheritNodeWithMeta(config, objects.at(ID)->base); - - loadObjectEntry(identifier, config, objects[ID]); + loadObjectEntry(identifier, config, objects[ID], isSubObject); } void CObjectClassesHandler::removeSubObject(si32 ID, si32 subID) diff --git a/lib/mapObjects/CObjectClassesHandler.h b/lib/mapObjects/CObjectClassesHandler.h index 9ea070f47..9cfe484ff 100644 --- a/lib/mapObjects/CObjectClassesHandler.h +++ b/lib/mapObjects/CObjectClassesHandler.h @@ -283,8 +283,9 @@ class DLL_LINKAGE CObjectClassesHandler : public IHandlerBase /// format: customNames[primaryID][secondaryID] -> name std::map> customNames; - void loadObjectEntry(const std::string & identifier, const JsonNode & entry, ObjectContainter * obj); + void loadObjectEntry(const std::string & identifier, const JsonNode & entry, ObjectContainter * obj, bool isSubobject = false); ObjectContainter * loadFromJson(const JsonNode & json, const std::string & name); + public: CObjectClassesHandler(); ~CObjectClassesHandler(); diff --git a/lib/mapObjects/CommonConstructors.cpp b/lib/mapObjects/CommonConstructors.cpp index c4350247a..175bf03c6 100644 --- a/lib/mapObjects/CommonConstructors.cpp +++ b/lib/mapObjects/CommonConstructors.cpp @@ -144,21 +144,24 @@ CDwellingInstanceConstructor::CDwellingInstanceConstructor() void CDwellingInstanceConstructor::initTypeData(const JsonNode & input) { const JsonVector & levels = input["creatures"].Vector(); - availableCreatures.resize(levels.size()); - for (size_t i=0; imodh->identifiers.requestIdentifier("creature", creatures[j], [=] (si32 index) + VLC->modh->identifiers.requestIdentifier("creature", creaturesOnLevel[currentCreature], [=] (si32 index) { - availableCreatures[i][j] = VLC->creh->creatures[index]; + availableCreatures[currentLevel][currentCreature] = VLC->creh->creatures[index]; }); } - assert(!availableCreatures[i].empty()); + assert(!availableCreatures[currentLevel].empty()); } - guards = input["guards"]; }