[PATCH] Prop variations not working when attached to specific animations

[michael]

When a unit has been set up to attach a prop only when a certain animation is playing, it will only show the first variant, rather than randomly choosing a variant from the list.

Examples:

• All the fauna units' blood decals don't randomize.
• If you set multiple people farming, there ought to be two variants of the hoe being used, but only one is ever chosen.

[vts]

The problem appears to be that random variations for a unit are chosen only when it is created. Afterwards, when the variant selector changes (as is the case for e.g. changing to the 'death' animation), the call stack goes:

CObjectManager::FindObjectVariation
CUnit::ReloadObject
CUnit::SetEntitySelection
CCmpVisualActor::SelectAnimation
<...>

The argument to FindObjectVariation is a vector of selector string sets, where higher-priority sets appear earlier in the list and are hence applied first. Currently there are two sets being used; "entity selections" first, then "actor selections". FindObjectVariation assumes that all these selectors together fully identify a unique variation, i.e. that no further randomization is needed.

If, nonetheless, there are variant groups for which it cannot select a variant based on one of the selector strings, then it will arbitrarily pick one, which in the current implementation happens to be the first one it finds. This is the reason why the same blood decal is always used.

Philip had this to say on IRC:

19:07 <@Philip`> When the explicit selections change (e.g. the animation-dependent selection), it should choose new variants based on the explicit selections, and then pick any still-undecided variants in the same way that it picked last time (i.e. it needs to store the previous picks so it can use them after the change), and then pick any still-undecided randomly
19:08 <@Philip`> When a unit dies, the corpse entity's actor should act exactly the same as the original unit's actor would have (i.e. copy all that internal state and the random seed etc)
19:09 <@Philip`> The 'actor selections' (if I remember correctly) are what the scenario designer has manually chosen in Atlas, so they should never be changed automatically
19:11 <@Philip`> In general, the important thing is that the variation remains consistent after a change as far as possible (it doesn't change hair colour just because it picked up a hoe, and if it raises a shield then drops the shield then raises the shield again it should be same shield pattern, etc)
19:11 <@Philip`> and that in all cases where consistency doesn't determine the behaviour, it should be random

So, I believe what we should do is change CUnit::ReloadObject so that before calling FindObjectVariation, it first merges the prioritized list of selector sets into a single set, and passes them to a call to ObjectBase::CalculateRandomVariation to find any remaining (randomized) selectors that are needed to fully identify a variation. It can then append this set to the end of the priority list being passed to FindObjectVariation, to complete the variation specification.

Or something like that.

[vts]

See also #1041.

[vts]

Have a patch for this ready, will attach it soon. It's gonna need some playtesting to make sure nothing broke though, since we don't seem to have any tests related to the variation/selection stuff.

[Philip]

With the proviso that I think the whole actor variation system is an inefficient ugly mess (my fault since I wrote it), and that I haven't actually tested this patch, it looks fine to me.

"using std::set;" etc is unconventional (and ugly in my opinion). Better to keep the std:: explicit, and if there's a frequently-used overly-verbose type then give it a typedef for readability (e.g. put a "typedef std::vector<std::vector<CStr> > VariantGroups_t" inside CObjectBase and use it for all the relevant methods and members and iterators etc).

[vts]

In 11523:

Calculate new random variations where necessary upon graphics object reloads. Fixes #979.