#4739 Wearables sort order reliability

4 files changed, 66 insertions, 25 deletions

diff --git a/indra/newview/llagentwearables.cpp b/indra/newview/llagentwearables.cpp
index 25f5cbd78f..a075b6f004 100644
--- a/indra/newview/llagentwearables.cpp
+++ b/indra/newview/llagentwearables.cpp
@@ -538,6 +538,27 @@ LLInventoryItem* LLAgentWearables::getWearableInventoryItem(LLWearableType::ETyp
     return item;
 }
 
+const S32 LLAgentWearables::getWearableIdxFromItem(const LLViewerInventoryItem* item) const
+{
+    if (!item) return -1;
+    if (!item->isWearableType()) return -1;
+
+    LLWearableType::EType type = item->getWearableType();
+    U32 wearable_count = getWearableCount(type);
+    if (0 == wearable_count) return -1;
+
+    const LLUUID& asset_id = item->getAssetUUID();
+
+    for (U32 i = 0; i < wearable_count; ++i)
+    {
+        const LLViewerWearable* wearable = getViewerWearable(type, i);
+        if (!wearable) continue;
+        if (wearable->getAssetID() != asset_id) continue;
+        return i;
+    }
+
+    return -1;
+}
 const LLViewerWearable* LLAgentWearables::getWearableFromItemID(const LLUUID& item_id) const
 {
     const LLUUID& base_item_id = gInventory.getLinkedItemID(item_id);
@@ -1471,7 +1492,7 @@ bool LLAgentWearables::moveWearable(const LLViewerInventoryItem* item, bool clos
 
     LLWearableType::EType type = item->getWearableType();
     U32 wearable_count = getWearableCount(type);
-    if (0 == wearable_count) return false;
+    if (wearable_count < 2) return false;
 
     const LLUUID& asset_id = item->getAssetUUID();
 
diff --git a/indra/newview/llagentwearables.h b/indra/newview/llagentwearables.h
index 3b8ff93c76..1e118ffa98 100644
--- a/indra/newview/llagentwearables.h
+++ b/indra/newview/llagentwearables.h
@@ -87,6 +87,7 @@ public:
 public:
     const LLUUID        getWearableItemID(LLWearableType::EType type, U32 index /*= 0*/) const;
     const LLUUID        getWearableAssetID(LLWearableType::EType type, U32 index /*= 0*/) const;
+    const S32 getWearableIdxFromItem(const LLViewerInventoryItem* item) const;
     const LLViewerWearable* getWearableFromItemID(const LLUUID& item_id) const;
     LLViewerWearable*   getWearableFromItemID(const LLUUID& item_id);
     LLViewerWearable*   getWearableFromAssetID(const LLUUID& asset_id);
diff --git a/indra/newview/llappearancemgr.cpp b/indra/newview/llappearancemgr.cpp
index 6fa23727b1..caf3990801 100644
--- a/indra/newview/llappearancemgr.cpp
+++ b/indra/newview/llappearancemgr.cpp
@@ -4218,37 +4218,54 @@ bool LLAppearanceMgr::moveWearable(LLViewerInventoryItem* item, bool closer_to_b
     if (item->getType() != LLAssetType::AT_CLOTHING) return false;
     if (!gInventory.isObjectDescendentOf(item->getUUID(), getCOF())) return false;
 
+    S32 pos = gAgentWearables.getWearableIdxFromItem(item);
+    if (pos < 0) return false; // Not found
+
+    U32 count = gAgentWearables.getWearableCount(item->getWearableType());
+    if (count < 2) return false; // Nothing to swap with
+    if (closer_to_body)
+    {
+        if (pos == 0) return false; // already first
+    }
+    else
+    {
+        if (pos == count - 1)  return false; // already last
+    }
+
+    U32 old_pos = (U32)pos;
+    U32 swap_with = closer_to_body ? old_pos - 1 : old_pos + 1;
+    LLUUID swap_item_id = gAgentWearables.getWearableItemID(item->getWearableType(), swap_with);
+
+    // Find link item from item id.
     LLInventoryModel::cat_array_t cats;
     LLInventoryModel::item_array_t items;
     LLFindWearablesOfType filter_wearables_of_type(item->getWearableType());
     gInventory.collectDescendentsIf(getCOF(), cats, items, true, filter_wearables_of_type);
     if (items.empty()) return false;
 
-    // We assume that the items have valid descriptions.
-    std::sort(items.begin(), items.end(), WearablesOrderComparator(item->getWearableType()));
-
-    if (closer_to_body && items.front() == item) return false;
-    if (!closer_to_body && items.back() == item) return false;
-
-    LLInventoryModel::item_array_t::iterator it = std::find(items.begin(), items.end(), item);
-    if (items.end() == it) return false;
-
-
-    //swapping descriptions
-    closer_to_body ? --it : ++it;
-    LLViewerInventoryItem* swap_item = *it;
-    if (!swap_item) return false;
-    std::string tmp = swap_item->getActualDescription();
-    swap_item->setDescription(item->getActualDescription());
-    item->setDescription(tmp);
+    LLViewerInventoryItem* swap_item = nullptr;
+    for (auto iter : items)
+    {
+        if (iter->getLinkedUUID() == swap_item_id)
+        {
+            swap_item = iter.get();
+            break;
+        }
+    }
+    if (!swap_item)
+    {
+        return false;
+    }
 
-    // LL_DEBUGS("Inventory") << "swap, item "
-    //                     << ll_pretty_print_sd(item->asLLSD())
-    //                     << " swap_item "
-    //                     << ll_pretty_print_sd(swap_item->asLLSD()) << LL_ENDL;
+    // Description is supposed to hold sort index, but user could have changed
+    // order rapidly and there might be a state mismatch between description
+    // and gAgentWearables, trust gAgentWearables over description.
+    // Generate new description.
+    std::string new_desc = build_order_string(item->getWearableType(), old_pos);
+    swap_item->setDescription(new_desc);
+    new_desc = build_order_string(item->getWearableType(), swap_with);
+    item->setDescription(new_desc);
 
-    // FIXME switch to use AISv3 where supported.
-    //items need to be updated on a dataserver
     item->setComplete(true);
     item->updateServer(false);
     gInventory.updateItem(item);
diff --git a/indra/newview/llviewerinventory.cpp b/indra/newview/llviewerinventory.cpp
index 6155058f14..ec5381ddfc 100644
--- a/indra/newview/llviewerinventory.cpp
+++ b/indra/newview/llviewerinventory.cpp
@@ -419,7 +419,9 @@ void LLViewerInventoryItem::updateServer(bool is_new) const
                          << LL_ENDL;
         return;
     }
-    if(gAgent.getID() != mPermissions.getOwner())
+    LLUUID owner = mPermissions.getOwner();
+    if(gAgent.getID() != owner
+        && owner.notNull()) // incomplete?
     {
         // *FIX: deal with this better.
         LL_WARNS(LOG_INV) << "LLViewerInventoryItem::updateServer() - for unowned item "