replacing saved outfit does not take off all clothing layers if old outfit uses more than one of any one type

[Lance Corrimal]

when you're wearing more than one layer of the same type, e.g. two or more tattoo layers, "replace outfit" on a saved outfit takes off only one of them.

steps to reproduce:

• "detach all" + "take off all", then add two tattoo layers
• Save this as "outfit 1"
• take off the two tattoo layers
• wear one different tattoo layer
• Save as "outfit 2"

now, switch between the two outfits by doing "replace outfit" on the saved outfits tab

Observed behavior in RLV 2.4 (RestrainedLove viewer v2.04.00.00) and any linden lab viewer up to viewer-development: the tattoos get replaced properly.

Observed behavior in RLV 2.7 (RestrainedLove viewer v2.07.03.03 (3.0.3.20205): one of the tattoos from the outfit with more than one stays behind when you change away from that outfit.

Further details: * the position in the stack of layers has no influence on which tattoo doesn't get removed * RLV enabled or disabled makes no difference * Self -> Take off -> Detach all has to be done more than once in this case, and sometimes does not take off the extra one at all, only "take off" in inventory helps. * the link in COF does get removed properly though

[Marine Kelley]

I'm sorry, I can't reproduce this bug, it seems to be working as expected, following your repro steps. However I have worked a little on the appearance manager lately, to fix other oddities, and it is very likely that I fixed this bug in the process without knowing, before you even reported it.

I'm setting it to "Resolved" since I believe it was indeed a bug before whatever I did on that piece of code. If it rears its head in the future, don't hesitate to reopen the issue (or to create a new one), stating that it had already been reported in the past.

Thank you !

[Lance Corrimal]

nope, still present in 3.1.0 :/

I can still reproduce by switching between two outfits that use two different tattoos each, only one tattoo gets taken off.

[Lance Corrimal]

further detail: the tattoo that is still worn is marked as "not worn" in the outfit tab, but still has "take off" as contect menu option.

[Marine Kelley]

I see you are using "detach all" and "take off all"... Don't use them (I didn't when trying to reproduce the bug). The SL codebase on these two features is bugged big time. They make you believe you have removed all clothes, while you are still wearing some of them. Totally confusing.

Seems related to https://jira.secondlife.com/browse/SVC-6769

[Lance Corrimal]

i'm using "detach all" and "take off all" in the workaround... on reproducing the bug:

make two outfits that contain two tattoo layers each, and replace outfits on the outfits tab, and you'll see that one tattoo stays behind.

and I'm not so sure about it being related to SVC-6769, as I cannot repro with original LL viewers, only with RLV...

[Marine Kelley]

Oh I was specifically talking about "Detach All" and "Take Off All", not about this bug here.

[Lance Corrimal]

checked in the latest release (2.07.04.02) and the bug is still there. have one outfit with two t-shirt layers and one with only one, and replacing the two shirt outfit with the one shirt outfit does not take off both shirts from the former.

[Lance Corrimal]

this bug still exists, and still repro's the same way.

make two outfits: same shape, skin, hairbase, and eyes, and four different tattoo layers, and four different alpha layers, two and two in each outfit.

and btw it reproduces with any saved outfit that contains more than one of the same texture wearable: pants, shirt, jacket, whatever. "jump" between those two outfits by doing "replace outfit" in your saved outfits window, and you will observe that in the saved outfit the alphas and tattoos appear to be properly "unworn" but when you look at your avie and your normal inventory they are still worn.

[Lance Corrimal]

another detail:

it does not make a difference if you do the "replace outfit" in your saved outfits list, or by doing it on the actual folder under "My Outfits" in your normal inventory.

and it does not reproduce in the official LL viewer.

[Lance Corrimal]

I think the problem might be in LLAgentWearables::setWearableOutfit() or LLAgentWearables::userUpdateAttachments()

[Lance Corrimal]

the attached patch makes "replace outfit" work again. It should give you enough pointers as to what needs to be done to fix this, and keep your optimizations intact... :)

[Lance Corrimal]

there seems to be more fishyness in that code.

if I add only the attached patch to your code, the viewer crashes on login...

[Marine Kelley]

I'm confused... You said your patch was probably fixing the problem, while keeping my optimisations (which certainly leave to be desired, but fix the pet peeve I had with the avatar "bouncing" everytime we changed clothes), but it actually removes all my code. Does your avatar bounce when you change outfits or even one cloth at a time while wearing a shoe base (one with a height) ?

Thanks for helping me fix this issue, btw. I just don't want to drop the fix for the "bouncing", to me the behaviour of the official viewer is unacceptable.

[Lance Corrimal]

ok just a bit of clarification,

the patch that i just attached here does nothing but add some debug output to your otherwise unchanged code. I'm trying to figure out what goes wrong where. The weird part is, that added line of llinfos blabla makes the viewer crash on login.

the pull request that I sent you yesterday does indeed remove your modifications, which makes changing outfits work as intended. I have to try if it brings the bouncing back as well. Hard to notice on a male avie, with no shoes with any kind of heel height. guess i'm in for some cowboy boots or sumthin.

[Lance Corrimal]

i just checked... without your code the bouncing is there even when both outfits involved contain the same shoe base :/

[Marine Kelley]

Probably cur_wearable is NULL on startup... That's the only way it could make the viewer crash, I think.

Thank you for checking... yes you need some kind of platform shoes, some that change the height of your avatar, to notice the bouncing.

[Lance Corrimal]

I think I'm onto a new algorithm for your bouncing dislike :)

how about this:

the new version would take off everything, except items where LLWearableType::getAllowMultiWear(type) returns false... which should be the case for shoes.

That way replace outfit would / should still replace everything, but without taking off the shoes, which would remove the bounce.

...on the other hand, with the right body physics, bounce is a good thing for everyone else :P

[Marine Kelley]

Sounds good, as long as there is no bouncing (physics or not :p) ! However, it seems that the action of taking or removing one piece of clothing is time-consuming for the viewer. I may be wrong, but maybe removing all clothes except shoes, then wearing clothes again (including some that were removed without really needing to), would take more time than removing only the clothes that were meant to be removed in the first place.

[Lance Corrimal]

That might be true, but the current "take off only what is needed" code obviously misses stuff as soon as there are layers with more than one item... thats why i tried to introduce that debug code, I wanted to check if the list actually contained everything I'm wearing.

[Marine Kelley]

I remember having written that code at the very beginning of v2, when you could only have one item per layer, that's why. So yes your solution is certainly better now (and easier to write and maintain).

[Lance Corrimal]

darn. rob me blind. removing my old workaround for this issue fixes the stuff with the trap we've been discussing in email... and brings this issue back to life.

[Lance Corrimal]

Changing the algorithm to use LLWearableType::getAllowMultiWear(type) to find out which things need to be taken off, and which will be replaced by new ones, would basically leave the avatar naked whenever a "change outfit" trap issued @addoutfit=n before taking off clothing. 0.o

[Lance Corrimal]

I can still reproduce this... even with the latest RLV source based on SSA.