#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 )
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)
Change History (16)
comment:1 by , 12 years ago
Priority: | Should Have → Nice to Have |
---|
comment:2 by , 12 years ago
Milestone: | Alpha 8 → Alpha 9 |
---|
comment:3 by , 12 years ago
Priority: | Nice to Have → Should Have |
---|---|
Summary: | Decal randomizations act strangely → Prop variations not working when attached to specific animations |
comment:4 by , 12 years ago
Description: | modified (diff) |
---|
comment:7 by , 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.
by , 12 years ago
Attachment: | entity_randomization_svn_25feb12.patch added |
---|
comment:8 by , 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 , 12 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:10 by , 12 years ago
Milestone: | Alpha 9 → Alpha 10 |
---|
comment:11 by , 12 years ago
Priority: | Should Have → Nice to Have |
---|
comment:12 by , 12 years ago
Keywords: | patch added |
---|
comment:13 by , 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:15 by , 8 years ago
Keywords: | review removed |
---|
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:
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:
So, I believe what we should do is change
CUnit::ReloadObject
so that before callingFindObjectVariation
, it first merges the prioritized list of selector sets into a single set, and passes them to a call toObjectBase::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 toFindObjectVariation
, to complete the variation specification.Or something like that.