From 0ba9cbef74419b5d196354a35e85ef0d034e9385 Mon Sep 17 00:00:00 2001 From: wraitii Date: Sat, 31 Jul 2021 17:55:10 +0000 Subject: [PATCH] Fix units changing appearance when switching animation. Bug introduced in 76acc4e146. The previous CUnit code had logic to select random aesthetic variants once initially. The new code removed that, as I completely missed its purpose, assuming that the random selection, being based on a seed, would pick the same variants every time. This is incorrect because entity selections can change the RNG calls, thus the variants, and thus entity appearance can change when the animation changes (typically, a horse will change color when walking and running). The solution is to re-introduce the choice of actor selections on CUnit creation. This makes sure that units don't change their purely-aesthetic selections when e.g. animations change. Reported by: Wowgetoffyourcellphone Tested By: Stan Differential Revision: https://code.wildfiregames.com/D4205 This was SVN commit r25844. --- source/graphics/ObjectBase.cpp | 8 ++++++++ source/graphics/ObjectBase.h | 5 +++++ source/graphics/Unit.cpp | 12 ++++++++++-- source/graphics/Unit.h | 16 ++++++++++------ source/graphics/UnitManager.cpp | 6 +++--- source/graphics/UnitManager.h | 4 ++-- .../components/CCmpProjectileManager.cpp | 6 ++---- .../simulation2/components/CCmpVisualActor.cpp | 3 +-- 8 files changed, 41 insertions(+), 19 deletions(-) diff --git a/source/graphics/ObjectBase.cpp b/source/graphics/ObjectBase.cpp index 1c02337a0b..aeb8581e58 100644 --- a/source/graphics/ObjectBase.cpp +++ b/source/graphics/ObjectBase.cpp @@ -781,6 +781,14 @@ CActorDef::CActorDef(CObjectManager& objectManager) : m_ObjectManager(objectMana { } +std::set CActorDef::PickSelectionsAtRandom(uint32_t seed) const +{ + // Use the selections from the highest quality actor - this lets artists maintain compatibility (or not) + // when going to lower quality levels. + std::vector> noSelections; + return GetBase(255)->CalculateRandomRemainingSelections(seed, noSelections); +} + std::vector CActorDef::QualityLevels() const { std::vector splits; diff --git a/source/graphics/ObjectBase.h b/source/graphics/ObjectBase.h index a139262ca1..9535eb6481 100644 --- a/source/graphics/ObjectBase.h +++ b/source/graphics/ObjectBase.h @@ -229,6 +229,11 @@ public: VfsPath GetPathname() const { return m_Pathname; } + /** + * Return a list of selections specifying a particular variant in all groups, based on the seed. + */ + std::set PickSelectionsAtRandom(uint32_t seed) const; + // Interface accessible from CObjectManager / CObjectBase protected: /** diff --git a/source/graphics/Unit.cpp b/source/graphics/Unit.cpp index 4c2f43c4d2..89092e9129 100644 --- a/source/graphics/Unit.cpp +++ b/source/graphics/Unit.cpp @@ -31,6 +31,15 @@ CUnit::CUnit(CObjectManager& objectManager, const CActorDef& actor, uint32_t seed) : m_ID(INVALID_ENTITY), m_ObjectManager(objectManager), m_Actor(actor), m_Seed(seed), m_Animation(nullptr) { + /** + * When entity selections change, we might end up with a different layout in terms of variants/groups, + * which means the random key calculation might end up with different results for the same seed. + * This is bad, as it means entities randomly change appearence when changing e.g. animation. + * To fix this, we'll initially pick a random and complete specification based on our seed, + * and then pass that as the lowest priority selections. Thus, if the actor files are properly specified, + * we can ensure that the entities will look the same no matter what happens. + */ + SetActorSelections(m_Actor.PickSelectionsAtRandom(m_Seed)); // Calls ReloadObject(). } CUnit::~CUnit() @@ -39,14 +48,13 @@ CUnit::~CUnit() delete m_Model; } -CUnit* CUnit::Create(const CStrW& actorName, uint32_t seed, const std::set& selections, CObjectManager& objectManager) +CUnit* CUnit::Create(const CStrW& actorName, uint32_t seed, CObjectManager& objectManager) { auto [success, actor] = objectManager.FindActorDef(actorName); UNUSED2(success); CUnit* unit = new CUnit(objectManager, actor, seed); - unit->SetActorSelections(selections); // Calls ReloadObject(). if (!unit->m_Model) { delete unit; diff --git a/source/graphics/Unit.h b/source/graphics/Unit.h index 2b550c0bcd..8a310df1b5 100644 --- a/source/graphics/Unit.h +++ b/source/graphics/Unit.h @@ -42,11 +42,10 @@ private: CUnit(CObjectManager& objectManager, const CActorDef& actor, uint32_t seed); public: - // Attempt to create a unit with the given actor, with a set of - // suggested selections (with the rest being randomised using the - // given random seed). + // Attempt to create a unit with the given actor. + // If specific selections are wanted, call SetEntitySelection or SetActorSelections after creation. // Returns NULL on failure. - static CUnit* Create(const CStrW& actorName, uint32_t seed, const std::set& selections, CObjectManager& objectManager); + static CUnit* Create(const CStrW& actorName, uint32_t seed, CObjectManager& objectManager); // destructor ~CUnit(); @@ -77,6 +76,9 @@ public: const std::set& GetActorSelections() const { return m_ActorSelections; } + /** + * Overwrite the seed-selected actor selections. Likely only useful for Atlas or debugging. + */ void SetActorSelections(const std::set& selections); private: @@ -96,9 +98,11 @@ private: // seed used when creating unit uint32_t m_Seed; - // actor-level selections for this unit + // Actor-level selections for this unit. This is normally set at init time, + // so that we always re-use the same aesthetic variants. + // These have lower priority than entity-level selections. std::set m_ActorSelections; - // entity-level selections for this unit + // Entity-level selections for this unit (used for e.g. animation variants). std::map m_EntitySelections; // object manager which looks after this unit's objectentry diff --git a/source/graphics/UnitManager.cpp b/source/graphics/UnitManager.cpp index 7d866eb683..fc91c8eb3f 100644 --- a/source/graphics/UnitManager.cpp +++ b/source/graphics/UnitManager.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2012 Wildfire Games. +/* Copyright (C) 2021 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -87,12 +87,12 @@ void CUnitManager::DeleteAll() /////////////////////////////////////////////////////////////////////////////// // CreateUnit: create a new unit and add it to the world -CUnit* CUnitManager::CreateUnit(const CStrW& actorName, uint32_t seed, const std::set& selections) +CUnit* CUnitManager::CreateUnit(const CStrW& actorName, uint32_t seed) { if (! m_ObjectManager) return NULL; - CUnit* unit = CUnit::Create(actorName, seed, selections, *m_ObjectManager); + CUnit* unit = CUnit::Create(actorName, seed, *m_ObjectManager); if (unit) AddUnit(unit); return unit; diff --git a/source/graphics/UnitManager.h b/source/graphics/UnitManager.h index 09bbc670bf..4dac265ef7 100644 --- a/source/graphics/UnitManager.h +++ b/source/graphics/UnitManager.h @@ -1,4 +1,4 @@ -/* Copyright (C) 2012 Wildfire Games. +/* Copyright (C) 2021 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -50,7 +50,7 @@ public: void DeleteAll(); // creates a new unit and adds it to the world - CUnit* CreateUnit(const CStrW& actorName, uint32_t seed, const std::set& selections); + CUnit* CreateUnit(const CStrW& actorName, uint32_t seed); // return the units const std::vector& GetUnits() const { return m_Units; } diff --git a/source/simulation2/components/CCmpProjectileManager.cpp b/source/simulation2/components/CCmpProjectileManager.cpp index aae653d760..4594b76fd7 100644 --- a/source/simulation2/components/CCmpProjectileManager.cpp +++ b/source/simulation2/components/CCmpProjectileManager.cpp @@ -202,8 +202,7 @@ uint32_t CCmpProjectileManager::LaunchProjectile(CFixedVector3D launchPoint, CFi projectile.origin = launchPoint; - std::set selections; - projectile.unit = GetSimContext().GetUnitManager().CreateUnit(actorName, m_ActorSeed++, selections); + projectile.unit = GetSimContext().GetUnitManager().CreateUnit(actorName, m_ActorSeed++); if (!projectile.unit) // The error will have already been logged return currentId; @@ -302,8 +301,7 @@ void CCmpProjectileManager::Interpolate(float frameTime) quat.ToMatrix(transform); transform.Translate(m_Projectiles[i].pos); - std::set selections; - CUnit* unit = GetSimContext().GetUnitManager().CreateUnit(m_Projectiles[i].impactActorName, m_ActorSeed++, selections); + CUnit* unit = GetSimContext().GetUnitManager().CreateUnit(m_Projectiles[i].impactActorName, m_ActorSeed++); unit->GetModel().SetTransform(transform); ProjectileImpactAnimation projectileImpactAnimation; diff --git a/source/simulation2/components/CCmpVisualActor.cpp b/source/simulation2/components/CCmpVisualActor.cpp index 91e49e4a88..764f90c953 100644 --- a/source/simulation2/components/CCmpVisualActor.cpp +++ b/source/simulation2/components/CCmpVisualActor.cpp @@ -598,11 +598,10 @@ void CCmpVisualActor::InitModel() if (!GetSimContext().HasUnitManager()) return; - std::set selections; std::wstring actorName = m_ActorName; if (actorName.find(L".xml") == std::wstring::npos) actorName += L".xml"; - m_Unit = GetSimContext().GetUnitManager().CreateUnit(actorName, GetActorSeed(), selections); + m_Unit = GetSimContext().GetUnitManager().CreateUnit(actorName, GetActorSeed()); if (!m_Unit) return;