From 721925add0f8448824c0388d94f764cb069e6cb1 Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Mon, 28 Sep 2026 23:34:22 -0500 Subject: [PATCH] fix(factions): override_faction 0 keeps the template's factions The client (LWODestroyableComponent::LoadConfigData 0x00c44cb0) uses a level object's set_faction only when override_faction is absent or true; with override_faction=0 LoadDataFromTemplate (0x00c9f900) puts the DestructibleComponent factionList back. 2948 level objects have override_faction=0, and DLU applied their set_faction anyway. The resolution is now DestroyableComponent::GetLevelFactions, with tests. No wire change. In game: enemies and smashables placed in levels with a set_faction but override_faction=0 (most smashables, many enemies) are targeted and aggro as before for the common case; check a few enemies in AG/GF/FV still fight you and friendly objects are not attacked, and that smashables still smash and give credit. Co-Authored-By: Claude Opus 5.5 --- dGame/Entity.cpp | 20 ++------ dGame/dComponents/DestroyableComponent.cpp | 16 +++++++ dGame/dComponents/DestroyableComponent.h | 9 ++++ .../dComponentsTests/CMakeLists.txt | 1 + .../DestroyableLevelConfigTests.cpp | 46 +++++++++++++++++++ 5 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 tests/dGameTests/dComponentsTests/DestroyableLevelConfigTests.cpp diff --git a/dGame/Entity.cpp b/dGame/Entity.cpp index 6ad34c39c..5de645346 100644 --- a/dGame/Entity.cpp +++ b/dGame/Entity.cpp @@ -487,21 +487,11 @@ void Entity::Initialize() { } } - // Level files can replace the factions with set_faction. The client splits the value on - // both ';' and ' ' and replaces its faction list with the result - // (LWODestroyableComponent::LoadConfigData), and many values have a trailing space ("13:6 "). - const auto setFaction = GetVarAsString(u"set_faction"); - std::vector factionsToSet; - for (const auto& semicolonSplit : GeneralUtils::SplitString(setFaction, ';')) { - for (const auto& faction : GeneralUtils::SplitString(semicolonSplit, ' ')) { - const auto factionToAdd = GeneralUtils::TryParse(faction); - if (factionToAdd) factionsToSet.push_back(factionToAdd.value()); - } - } - - if (!factionsToSet.empty()) { - comp->SetFaction(factionsToSet.front(), true); - for (const auto faction : factionsToSet | std::views::drop(1)) comp->AddFaction(faction, true); + // Level files can replace the factions with set_faction, unless override_faction is 0 + const auto levelFactions = DestroyableComponent::GetLevelFactions(*this); + if (levelFactions && !levelFactions->empty()) { + comp->SetFaction(levelFactions->front(), true); + for (const auto faction : *levelFactions | std::views::drop(1)) comp->AddFaction(faction, true); } } diff --git a/dGame/dComponents/DestroyableComponent.cpp b/dGame/dComponents/DestroyableComponent.cpp index 2e9e5ed8d..5b523fefd 100644 --- a/dGame/dComponents/DestroyableComponent.cpp +++ b/dGame/dComponents/DestroyableComponent.cpp @@ -790,6 +790,22 @@ void DestroyableComponent::Smash(const LWOOBJID source, const eKillType killType m_Parent->Kill(owner, killType); } +std::optional> DestroyableComponent::GetLevelFactions(const Entity& entity) { + if (!entity.HasVar(u"set_faction")) return std::nullopt; + // override_faction defaults to true when set_faction is there; false keeps the template's factions + if (entity.HasVar(u"override_faction") && !entity.GetVar(u"override_faction")) return std::nullopt; + + // Many values have a trailing space ("6 ") + std::vector factions; + for (const auto& semicolonSplit : GeneralUtils::SplitString(entity.GetVarAsString(u"set_faction"), ';')) { + for (const auto& faction : GeneralUtils::SplitString(semicolonSplit, ' ')) { + const auto factionToAdd = GeneralUtils::TryParse(faction); + if (factionToAdd) factions.push_back(factionToAdd.value()); + } + } + return factions; +} + void DestroyableComponent::SetFaction(int32_t factionID, bool ignoreChecks) { m_FactionIDs.clear(); m_EnemyFactionIDs.clear(); diff --git a/dGame/dComponents/DestroyableComponent.h b/dGame/dComponents/DestroyableComponent.h index 43c46a56c..25f52d919 100644 --- a/dGame/dComponents/DestroyableComponent.h +++ b/dGame/dComponents/DestroyableComponent.h @@ -2,6 +2,7 @@ #define DESTROYABLECOMPONENT_H #include "RakNetTypes.h" +#include #include #include "tinyxml2.h" #include "Entity.h" @@ -368,6 +369,14 @@ public: void SetCurrencyIndex(int32_t currencyIndex) { m_CurrencyIndex = currencyIndex; } + /** + * The factions a level object's set_faction gives it in place of the template's, resolved as the client does + * (LWODestroyableComponent::LoadConfigData 0x00c44cb0, LoadDataFromTemplate 0x00c9f900): set_faction is used + * unless override_faction is present and false. The value is split on ';' and ' '. + * @return the factions, or nullopt when the template's factions stay + */ + static std::optional> GetLevelFactions(const Entity& entity); + /** * Returns the ID of the entity that killed this entity, if any * @return the ID of the entity that killed this entity, if any diff --git a/tests/dGameTests/dComponentsTests/CMakeLists.txt b/tests/dGameTests/dComponentsTests/CMakeLists.txt index 7772f5ecb..3dba2041a 100644 --- a/tests/dGameTests/dComponentsTests/CMakeLists.txt +++ b/tests/dGameTests/dComponentsTests/CMakeLists.txt @@ -1,6 +1,7 @@ set(DCOMPONENTS_TESTS "DeletionRestrictionTests.cpp" "InventorySaveTests.cpp" + "DestroyableLevelConfigTests.cpp" "VendorBuybackTests.cpp" "RacingWrongWayTests.cpp" "DestroyableComponentTests.cpp" diff --git a/tests/dGameTests/dComponentsTests/DestroyableLevelConfigTests.cpp b/tests/dGameTests/dComponentsTests/DestroyableLevelConfigTests.cpp new file mode 100644 index 000000000..b3637576e --- /dev/null +++ b/tests/dGameTests/dComponentsTests/DestroyableLevelConfigTests.cpp @@ -0,0 +1,46 @@ +#include "GameDependencies.h" + +#include "DestroyableComponent.h" +#include "Entity.h" + +#include + +// How a level object's config changes its DestroyableComponent, as the client's +// LWODestroyableComponent::LoadConfigData (0x00c44cb0) and LoadDataFromTemplate (0x00c9f900) resolve it. +class DestroyableLevelConfigTests : public GameDependenciesTest { +protected: + std::unique_ptr entity; + + void SetUp() override { + SetUpDependencies(); + entity = std::make_unique(1, info); + } + + void TearDown() override { + entity.reset(); + TearDownDependencies(); + } +}; + +TEST_F(DestroyableLevelConfigTests, NoSetFactionKeepsTheTemplateFactions) { + EXPECT_FALSE(DestroyableComponent::GetLevelFactions(*entity)); + entity->SetVar(u"override_faction", true); + EXPECT_FALSE(DestroyableComponent::GetLevelFactions(*entity)); +} + +TEST_F(DestroyableLevelConfigTests, SetFactionWithoutOverrideFactionIsUsed) { + entity->SetVar(u"set_faction", "6 "); + EXPECT_EQ(DestroyableComponent::GetLevelFactions(*entity), std::vector{ 6 }); +} + +TEST_F(DestroyableLevelConfigTests, SetFactionWithOverrideFactionTrueIsUsed) { + entity->SetVar(u"set_faction", "4; 6"); + entity->SetVar(u"override_faction", true); + EXPECT_EQ(DestroyableComponent::GetLevelFactions(*entity), (std::vector{ 4, 6 })); +} + +TEST_F(DestroyableLevelConfigTests, OverrideFactionFalseIgnoresSetFaction) { + entity->SetVar(u"set_faction", "-1"); + entity->SetVar(u"override_faction", false); + EXPECT_FALSE(DestroyableComponent::GetLevelFactions(*entity)); +}