Opened 13 years ago

Closed 12 years ago

Last modified 8 years ago

#979 closed defect (fixed)

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

Reported by: michael Owned by: vts
Priority: Nice to Have Milestone: Alpha 10
Component: Core engine Keywords: patch
Cc: Patch:

Description (last modified by Zaggy1024)

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.

Attachments (1)

entity_randomization_svn_25feb12.patch (10.3 KB ) - added by vts 12 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 by Kieran P, 12 years ago

Priority: Should HaveNice to Have

comment:2 by Kieran P, 12 years ago

Milestone: Alpha 8Alpha 9

comment:3 by Zaggy1024, 12 years ago

Priority: Nice to HaveShould Have
Summary: Decal randomizations act strangelyProp variations not working when attached to specific animations
Last edited 12 years ago by Zaggy1024 (previous) (diff)

comment:4 by Zaggy1024, 12 years ago

Description: modified (diff)

comment:5 by vts, 12 years ago

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.

Last edited 12 years ago by vts (previous) (diff)

comment:6 by vts, 12 years ago

See also #1041.

comment:7 by vts, 12 years ago

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.

comment:8 by vts, 12 years ago

Keywords: review added
Summary: Prop variations not working when attached to specific animations[PATCH] Prop variations not working when attached to specific animations

comment:9 by vts, 12 years ago

Owner: set to vts
Status: newassigned

comment:10 by historic_bruno, 12 years ago

Milestone: Alpha 9Alpha 10

comment:11 by Kieran P, 12 years ago

Priority: Should HaveNice to Have

comment:12 by Kieran P, 12 years ago

Keywords: patch added

comment:13 by Philip Taylor, 12 years ago

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).

comment:14 by vts, 12 years ago

Resolution: fixed
Status: assignedclosed

In 11523:

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

comment:15 by sanderd17, 8 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.