[opensource-dev] Review Request: STORM-702 Make it possible to wear partial outfits

[Aleric Inglewood]

> On 2010-12-20 14:40:41, Nyx Linden wrote:
> > I have no technical objections to the code provided.
> > And in fact, the code provided *should* change the functionality back to what the users are reporting is their expectation of what the behavior should be.
> > 
> > The part that makes me nervous is that this enables an outfit operation for a class of folders that we have not allowed with the new outfit system previously. Based on other pieces on the code that would get called by this operation, I believe that the result will be that of the resident request to "revert" behavior (namely remove all clothing, remove body parts that are replaced by the new folder, leave old body parts that are necessary to display the avatar). 
> > 
> > That being said, we need a more comprehensive review of the role of incomplete outfits and how it fits with our technical architecture we've built up in our current outfits system. The code here should implement the correct behavior and I have no technical issues with it. But I want to make sure that we are aware of the risk of edge cases as we have not considered possibly popping up as a result of this patch.

My (mathematical) analysis of the "outfit system"
=================================================

- The outfit system exists of sets and operators.

- Each operator works on two sets, where both sets are an input and one of the sets is used as output; this would be irrelevant for what the operators do except when those operators are not commutative (which is be Bad Thing, but unfortunately they are not). If one of those operators is written as '+' (whatever that does), then for now we can speak about a + b, which doesn't imply that this is the same as b + a and which intuitively extends to the definition a += b ==> a = a + b. Note that the '+' operator is abritrary, replace + with - and the same story holds.

- The elements of each set do not have the same type, and if they don't then an operator can't work on them. Therefore, the correct way to look at these sets is as vectors where an absent type will be represented by '0' (zero). That is an arbitrary choice, but intuitive because our operators will be a lot more like groups than like fields. Of course, this implies that a + 0 = 0 + a = a.

- Our vector (V) has one element per type. The types are: shape, (Linden) hair, (Linden) shoes, eyes, skin, alpha, tatoes, undershirts, shirts, jackets, gloves, undies, pants, socks, skull attachments, nose attachments, etc (one type for each different attachment point).

- The elements of this vector are sets: for some types it is possible to wear multiple of the same type at the same time. Note that these are sets, not vectors: the order in which these wearables are added SHOULD be irrelevant (at the moment it isn't; for example, old viewers only show the first attachment that was attached, so the order is important).

- Some types may only have a single element (in their set) at a time. As a result their operators are clearly not commutative, that is impossible. These types are: shape, hair, shoes, eyes, skin and alpha (I think; the exact list is irrelevant at this point). Lets call this class of types: S (of 'Single').

- The other types can be divided into two classes: wearables (tatoes, (under)shirts, jackets, gloves, pants, socks) and attachments (skull, nose, ...). For an intuitive functioning of the outfit system these SHOULD behave the same mathematically, but at this point we can't be certain if they do. However, it will be clear that all wearables and all attachments respectively behave the same. Lets call these classes W and A. [Note that V doesn't have to be implemented as a linear vector, I think it would be better to implement it as a vector of three vectors: one vector for elements of class S, one for elements of class W and one for elements of class A.]

- For the S class we CAN have the following operations.

  Let x and y be two distinct elements of the same type, where the type is of class S.
  [Note that for the Current Outfit the empty set is not allowed.]
  Picking two arbitrary characters (+ and -) we can write for the possible operators: x + y = y, x - y = x.
  Other operators do not exist, but if they do out of necessity (namely, these are elements of a vector and we will need to define operators of the vector, which then in turn work on all types), then we can choose to let them be equivalent to either + or -. Also note that this choice defines our extended operators += and -= as: x += y ==> x = y, and x -= y ==> nul operator.

- For the A class we CAN have the following operations.

  Let a and b be sets. Note that any distinct element can only be once in a set: { e1 } + { e1 } = { e1 }, but { e1 } - { e1 } = { 0 }.
  Hence, also this class is not a group, but so far the operators are commutative: a + b = b + a, etc, but there is no well defined inverse for this operation.

  This leads us to define the operators of A as '+' and '-' in the following sense:
  a + b = c, where a, b and c can be implemented as the C++ std::set<T>, where T is capable of uniquely identifying equality (ie, if i1 and i2 are two distinct objects in your inventory, than, expressed as T it holds that i1 == i1, and i1 != i2. Ordering can be allowed to be arbitrary (i1 < i2 ==> i1 != i2). Then, adding b to a can be implemented by adding the elements of b to a one by one. Likewise, a - b can be implemented by trying to find and erase all elements of b one by one from a.

- For the W class the order DOES matter.

  If you put on two shirts, the result should be different if you first put on shirt u and then v, or the other way around: u + v != v + u.
  Hence, in this case, on top of the above (for class A) we need to exactly describe operator< so that the ordering is a well known function of the order in which these wearables were put on. Baking can then be performed by running over all elements of a set in order.


Clearly, the above has naturally led us to two operators: + and - to be defined for the complete outfit vector. While we can easily map these two operators on the operators of class W, which naturally maps to the operators of class A. [Note that we do not necessarily have to map them also to the same + and - as defined above for class S. For example, it is possible to define the vector operator + and - both as being the operator + for class S.]

Also clearly, we can map the mathematically found operators (+ and -) to the already existing intuitive phrases in-world as follows:

operator+ : Add to outfit
operator- : Remove from outfit
operator= : Replace outfit (define as: x -= x; x += y) [Note: this is NOT the same as the '=' that used everywhere else, which indeed just completely replaces as usual. I will write explicit operator= to mean this operator, and just '=' everywhere else]

The notion "Incomplete Outfits" now, simply means that we allow the right-hand terms of each operator to have elements of class S that ARE empty.

As a result we just have to refine the operators of class S, as follows: x + 0 = x and x - 0 = x.

Note this makes operator= semi-non intuitive (note that x - x = x, as per our definition x - y = x):
If y is empty, then x 'operator=' 0 ==> x -= x ; x += 0 ==> x = x - x ; x = x + 0 ==> x = x ; x = x ==> nul operator.
In other words, assigning a 0 to an element of class S with operator= has no effect.

I believe that the above definitions, which arise from a mathematical point of view and not from in-world experience, is sufficient to write a good C++ implementation,
define the right classes and operators for those classes.


Hopefully this analysis was helpful for your cause,
Aleric


  


- Aleric


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/14/#review59
-----------------------------------------------------------


On 2010-12-13 07:08:28, Vadim ProductEngine wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/14/
> -----------------------------------------------------------
> 
> (Updated 2010-12-13 07:08:28)
> 
> 
> Review request for Viewer.
> 
> 
> Summary
> -------
> 
> Enabled the "Replace Current Outfit" option for incomplete outfits (i.e. those that don't contain full set of body parts).
> 
> 
> This addresses bug STORM-702.
>     http://jira.secondlife.com/browse/STORM-702
> 
> 
> Diffs
> -----
> 
>   indra/newview/llappearancemgr.cpp 3d2e71443c58 
>   indra/newview/llinventoryfunctions.cpp 3d2e71443c58 
> 
> Diff: http://codereview.secondlife.com/r/14/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vadim
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.secondlife.com/pipermail/opensource-dev/attachments/20101222/34addd5b/attachment-0001.htm