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

2011-01-12 Thread Nyx Linden

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

Ship it!


Now has design approval. ship it.

- Nyx


On Dec. 13, 2010, 7:08 a.m., Vadim ProductEngine wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/14/
> ---
> 
> (Updated Dec. 13, 2010, 7:08 a.m.)
> 
> 
> 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
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

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

2010-12-22 Thread 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 i

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

2010-12-20 Thread Nyx Linden

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


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.

- Nyx


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

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

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

2010-12-13 Thread Vadim ProductEngine

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

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

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges