Keep every mod's cosmetics when two presets share a short name

With several cosmetic mods installed the game crashed as soon as an
equipped custom item came into view, and so on every launch after it
was equipped.
Cosmetic presets (Engine/Resource/bundle_ref_table.*)
- The character bundle-reference table says which bundle holds each
  preset, under the preset's full path and under its leaf name, one row
  per hash. Mods number their presets alike in different folders
  (cust_tops/1_ap, cust_bottoms/1_ap), so two mods can share a leaf
  name. Adding the second preset threw "collides with an existing
  lookup", the merge fell back to the highest-priority mod's copy of
  the table, and every other cosmetic mod's presets were unregistered.
  The game then built the skater with a preset it could not find and
  called through a bad pointer.
- A preset whose leaf name is taken is now added under its full path
  only; the leaf name stays with the preset that had it. Only a clash
  of full paths still fails. MergedTable::shadowed lists each such
  preset with the one that holds its name.
Merge log (Engine/Vfs/mod_merge_combine.cpp)
- Each mod with shadowed presets gets a line naming how many and an
  example pair, so an item that shows the wrong preset can be traced to
  the mods that clash.
Checked against 39 installed mods, 15 of them cosmetic: the table
combines 274 presets where it used to keep one mod's, all reachable by
full path and 267 by leaf name, with the game's own 6264 rows unchanged.
The seven left without a leaf row are a naming clash between the mods
themselves (two Carhartt packs, a Zero tee and deck); whether the game
finds those by full path is not known.
bundle_ref_table_tests takes an optional second mod and checks that two
mods' presets are all added, in either order, and that a shared leaf
name is reported with the preset that holds it.

Engine/Resource/bundle_ref_table.cpp

@@ -127,19 +127,33 @@ private:
     std::unordered_map<std::uint32_t, std::string> cache_;
 };
 
-// Appends `path` to the pool and a full/leaf row pair for it in `bundle`.
-void insert(std::vector<std::byte>& data, std::vector<std::byte>& meta, const std::string& path, std::uint32_t bundle) {
+// Appends `path` to the pool and a full/leaf row pair for it in `bundle`. A
+// leaf name another preset already answers to stays that preset's: the table
+// holds one row per hash, and mods number their presets alike (1_ap, 2_ap ...)
+// in different folders. Returns that preset's path, empty when the leaf row
+// went in too.
+std::string insert(std::vector<std::byte>& data, std::vector<std::byte>& meta, const std::string& path, std::uint32_t bundle) {
     const auto table = Reader(data, meta).out;
     const auto full = hash(path), short_name = hash(leaf(path));
-    if (full == short_name || std::ranges::any_of(table.rows, [&](const Row& row) { return row.hash == full || row.hash == short_name; }))
-        throw std::runtime_error(path + " collides with an existing lookup");
+    const auto held = [&](std::uint64_t wanted) {
+        return std::ranges::any_of(table.rows, [&](const Row& row) { return row.hash == wanted; });
+    };
+    if (held(full)) throw std::runtime_error(path + " collides with an existing lookup");
+    // A path with no folder is its own leaf and has the one row.
+    const bool with_leaf = full != short_name && !held(short_name);
+    std::string holder;
+    if (full != short_name && !with_leaf) {
+        holder = "another lookup";
+        for (const auto& preset : table.presets)
+            if (hash(leaf(preset.first)) == short_name) { holder = preset.first; break; }
+    }
     if (path.size() > 255) throw std::runtime_error(path + " is too long for a lookup");
     const auto growth = (4 + path.size() + 15) & ~std::size_t{15};
     const auto offset = table.bundles - table.pool;
     if (offset > 0x7FFFFF) throw std::runtime_error("bundle-reference path pool is full");
     const auto token = static_cast<std::uint32_t>(offset) | 0x800000U | (static_cast<std::uint32_t>(path.size()) << 24);
     const auto bundles = table.bundles + growth, lookups = table.lookups + growth;
-    const auto payload = table.payload + growth + 32;
+    const auto payload = table.payload + growth + (with_leaf ? 32 : 16);
     std::vector<std::byte> out(payload + 20);
     std::copy_n(data.begin(), table.bundles, out.begin());
     put32(out, table.bundles, none);
@@ -148,7 +162,7 @@ void insert(std::vector<std::byte>& data, std::vector<std::byte>& meta, const st
               out.begin() + static_cast<std::ptrdiff_t>(bundles));
     auto rows = table.rows;
     rows.push_back({full, bundle, token});
-    rows.push_back({short_name, bundle, token});
+    if (with_leaf) rows.push_back({short_name, bundle, token});
     std::ranges::sort(rows, {}, &Row::hash);
     for (std::size_t i = 0; i < rows.size(); ++i) {
         put64(out, lookups + i * 16, rows[i].hash);
@@ -161,6 +175,7 @@ void insert(std::vector<std::byte>& data, std::vector<std::byte>& meta, const st
     put32(out, 72, static_cast<std::uint32_t>(rows.size()));
     put32(meta, 0, static_cast<std::uint32_t>(payload));
     data = std::move(out);
+    return holder;
 }
 } // namespace
 
@@ -176,7 +191,8 @@ MergedTable merge(const Table& base, std::span<const Table> edits) {
     const std::span<const std::byte> base_bundles = base.resource.subspan(original.bundles, original.lookups - original.bundles);
     MergedTable result{{base.resource.begin(), base.resource.end()}, {base.resourceMeta.begin(), base.resourceMeta.end()}};
     std::map<std::string, std::uint32_t> added;
-    for (const auto& edit : edits) {
+    for (std::size_t index = 0; index < edits.size(); ++index) {
+        const auto& edit = edits[index];
         const auto copy = Reader(edit.resource, edit.resourceMeta).out;
         // Additions only: the same bundle list, and every base preset where it was.
         const auto bundles = edit.resource.subspan(copy.bundles, copy.lookups - copy.bundles);
@@ -193,7 +209,8 @@ MergedTable merge(const Table& base, std::span<const Table> edits) {
                 if (found->second != bundle) ++result.conflicts;
                 continue;
             }
-            insert(result.resource, result.resourceMeta, path, bundle);
+            if (auto holder = insert(result.resource, result.resourceMeta, path, bundle); !holder.empty())
+                result.shadowed.push_back({index, path, std::move(holder)});
             added.emplace(path, bundle);
             ++result.added;
         }

Engine/Resource/bundle_ref_table.h

@@ -10,6 +10,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <span>
+#include <string>
 #include <string_view>
 #include <vector>
 
@@ -28,6 +29,14 @@ struct MergedTable {
     std::vector<std::byte> resourceMeta;
     std::size_t added{};     // presets taken from the edits
     std::size_t conflicts{}; // presets two edits put in different bundles; the first won
+    // Presets added under their full path only, because another preset already
+    // answers to the same leaf name (cust_tops/1_ap and cust_bottoms/1_ap).
+    struct Shadowed {
+        std::size_t edit{};  // index into `edits`
+        std::string path;    // the preset left without a leaf lookup
+        std::string holder;  // the preset its leaf name finds instead
+    };
+    std::vector<Shadowed> shadowed;
 };
 
 // Throws when a copy is not a table of this kind, or an edit changes the base's

Engine/Vfs/Test/bundle_ref_table_tests.cpp

@@ -1,9 +1,11 @@
 // Merges the game's character bundle-reference table with a cosmetic mod's
-// copy of it. Arguments: <Skate folder> <cosmetic mod folder>; skipped when
-// either is missing.
+// copy of it. Arguments: <Skate folder> <cosmetic mod folder> [<another
+// cosmetic mod folder>]; skipped when either of the first two is missing.
 #include "Engine/Vfs/mod_merge_internal.h"
 #include "Engine/Resource/bundle_ref_table.h"
 #include "Engine/Resource/cas_codec.h"
+#include <algorithm>
+#include <array>
 #include <iostream>
 #include <stdexcept>
 
@@ -58,6 +60,25 @@ int main(int argc, char** argv) {
         check(again.added == 0, "the merged table already holds every preset");
         std::cout << "base " << base.bytes.size() << " bytes, mod " << copy.bytes.size() << " bytes, merged "
                   << once.resource.size() << " bytes; " << once.added << " preset(s) added.\n";
+        if (argc > 3 && fs::exists(fs::path(argv[3]) / L"Win32" / L"items.toc")) {
+            // Mods number their presets alike (cust_tops/1_ap, cust_bottoms/1_ap), so two
+            // mods can share leaf names. Every preset of both still has to go in.
+            const auto other = read_table(store, argv[3], base_root, game_root);
+            const fb::bundle_ref::Table other_table{other.bytes, other.meta};
+            const auto alone = fb::bundle_ref::merge(base_table, std::span(&other_table, 1));
+            const std::array both_tables{mod_table, other_table}, reversed_tables{other_table, mod_table};
+            const auto both = fb::bundle_ref::merge(base_table, both_tables);
+            const auto reversed = fb::bundle_ref::merge(base_table, reversed_tables);
+            check(both.added >= std::max(once.added, alone.added) && both.added <= once.added + alone.added,
+                  "two mods' presets are all added");
+            check(reversed.added == both.added && reversed.shadowed.size() == both.shadowed.size(),
+                  "the order of the mods changes which preset keeps a shared leaf name, not what is added");
+            for (const auto& clash : both.shadowed)
+                check(clash.edit == 1 && !clash.path.empty() && !clash.holder.empty() && clash.path != clash.holder,
+                      "a shared leaf name is reported with the preset that holds it");
+            std::cout << "two mods: " << both.added << " preset(s) added, " << both.shadowed.size()
+                      << " under their full path only.\n";
+        }
     } catch (const std::exception& error) {
         std::cerr << "FAILED: " << error.what() << "\n";
         return 1;

Engine/Vfs/mod_merge_combine.cpp

@@ -497,6 +497,19 @@ fb::TocDocument combine(const fs::path& baseToc, const fs::path& baseRoot,
                         what = std::to_string(table.added) + " preset(s)";
                         if (table.conflicts)
                             what += ", " + std::to_string(table.conflicts) + " disagreed";
+                        // Mods number their presets alike, so two can share a leaf
+                        // name the table has one row for. Both are registered; say
+                        // whose name the other answers to, so an item that shows
+                        // the wrong preset can be traced to its mod.
+                        std::map<std::size_t, std::pair<std::size_t, std::string>> shadowed;
+                        for (const auto& clash : table.shadowed) {
+                            auto& [count, example] = shadowed[clash.edit];
+                            if (!count++) example = clash.path + " and " + clash.holder;
+                        }
+                        for (const auto& [index, clash] : shadowed)
+                            report.notes.push_back(path_utf8(edits[index]->root.filename()) + ": " +
+                                std::to_string(clash.first) + " preset(s) share a short name with another mod's, e.g. " +
+                                clash.second + "; they are registered by their full path only");
                     } else if (shaderTable) {
                         // Each mod aliases new material keys onto shader programs
                         // and texture sets the base already ships, so the union of

Extension/Customization/local_customization_runtime.cpp

@@ -298,6 +298,15 @@ bool recover_card_only_preset(std::string_view id, const profile::CosmeticLoadou
     return true;
 }
 
+// The loadout manager's record array, or false when it cannot be read. The
+// same shape the save hook demands of a record it is handed.
+bool loadout_array(std::uintptr_t& manager, std::uintptr_t& begin, std::uintptr_t& end) {
+    auto& s = local_runtime();
+    return read(s.base + addr::engine::loadout_manager, manager) && manager &&
+        read(manager + 0x18, begin) && read(manager + 0x20, end) && begin <= end &&
+        (end - begin) % 0x48 == 0 && (end - begin) / 0x48 <= 10;
+}
+
 // A load, taking the preset id instead of the game's wrapper so that
 // retry_pending_cosmetic_loads can run it again. The caller holds native_mutex.
 bool apply_saved_loadout(const std::string& id, void* destination) {
@@ -325,8 +334,10 @@ bool apply_saved_loadout(const std::string& id, void* destination) {
         if (!refresh_cosmetic_catalog()) {
             // The cosmetics manager is not ready this early in a level load.
             // The preset stays blocked, so nothing is written over the outfit
-            // on disk, and the pump applies this record once it is ready.
-            c.pending_loads[id] = destination;
+            // on disk, and the pump applies this record once it is ready --
+            // but only while this is still the array the record came from.
+            CosmeticRuntime::PendingLoad pending{destination};
+            if (loadout_array(pending.manager, pending.begin, pending.end)) c.pending_loads[id] = pending;
             return cosmetic_diagnostic("load", "catalog_not_ready", id);
         }
         if (saved->recipes.size() != defaults.recipes.size()) return cosmetic_diagnostic("load", "recipe_count_mismatch", id);
@@ -411,24 +422,23 @@ bool load_cosmetic_hook(const void* wrapper, void* destination) {
 void retry_pending_cosmetic_loads() {
     auto& c = cosmetic_runtime();
     if (c.pending_loads.empty()) return;
-    auto& s = local_runtime();
-    // The records belong to the loadout manager's array, which can be replaced
-    // between the refused load and now. Check each pointer against the live
-    // array exactly as a save does, and drop one that no longer belongs rather
-    // than writing through it.
     std::uintptr_t manager{}, begin{}, end{};
-    if (!read(s.base + addr::engine::loadout_manager, manager) || !manager ||
-        !read(manager + 0x18, begin) || !read(manager + 0x20, end) || begin > end ||
-        (end - begin) % 0x48 || (end - begin) / 0x48 > 10) return;
+    // Unreadable now: keep the records and come back, rather than losing them.
+    if (!loadout_array(manager, begin, end)) return;
     auto pending = std::move(c.pending_loads);
     c.pending_loads.clear();
-    for (const auto& [id, destination] : pending) {
-        const auto p = reinterpret_cast<std::uintptr_t>(destination);
-        if (!destination || p < begin || p >= end || (p - begin) % 0x48) {
+    for (const auto& [id, load] : pending) {
+        const auto p = reinterpret_cast<std::uintptr_t>(load.destination);
+        // The same array, the same bounds, and the record still inside it on
+        // its stride. Anything else and this address may now hold another
+        // preset: give up on the outfit rather than write it over a different
+        // saved character.
+        if (!load.destination || manager != load.manager || begin != load.begin || end != load.end ||
+            p < begin || p >= end || (p - begin) % 0x48) {
             cosmetic_diagnostic("load", "retry_record_moved", id);
             continue;
         }
-        if (apply_saved_loadout(id, destination))
+        if (apply_saved_loadout(id, load.destination))
             cosmetic_diagnostic("load", "retried_after_catalog_ready", id);
     }
 }

Extension/Customization/local_customization_runtime.h

@@ -21,7 +21,18 @@ struct CosmeticRuntime {
     // per level load and does not ask again, so the pump retries these once the
     // catalog is ready. Until one succeeds the preset stays in
     // `blocked_loadouts`, so a save cannot write over the outfit on disk.
-    std::map<std::string, void*, std::less<>> pending_loads;
+    //
+    // The loadout manager's array is recorded with it. A record is only written
+    // to when that array is still the same one, bounds and all: inside the
+    // array and on its stride says a record is there, not that it is still
+    // this preset's, and a level load can rebuild the array under us. Writing
+    // one preset's outfit into another's slot would lose a saved character,
+    // which is worse than not applying it at all.
+    struct PendingLoad {
+        void* destination{};
+        std::uintptr_t manager{}, begin{}, end{};
+    };
+    std::map<std::string, PendingLoad, std::less<>> pending_loads;
     // Saved outfit items that were not installed when their preset loaded (a
     // disabled or removed costume mod). The slot shows its default this session;
     // saves keep the original item unless the player changes that slot.

Launcher/gui_mods.cpp

@@ -406,6 +406,9 @@ void install_modal(const Fonts& fonts, ModsPanel& panel, ImVec2 size) {
     }
     const float fraction = std::clamp(panel.progress.load(), 0.0f, 1.0f);
     const auto time = static_cast<float>(ImGui::GetTime());
+    // Cancelling is not instant: the worker notices on its next chunk or file.
+    // Say so, or the button looks dead and gets pressed again.
+    const bool cancelling = panel.cancel.load();
     // Its own window over the page: the page's own draw list renders under
     // its children, so a panel drawn there would sit beneath the mod list.
     ImGui::SetNextWindowPos(ImVec2(0, 0));
@@ -419,10 +422,14 @@ void install_modal(const Fonts& fonts, ModsPanel& panel, ImVec2 size) {
     const ImVec2 origin((size.x - extent.x) * 0.5f, (size.y - extent.y) * 0.5f);
     const ImVec2 end(origin.x + extent.x, origin.y + extent.y);
     skate_theme::rough_rect(draw, origin, end, color::blue, 11, g_scale);
-    draw->AddText(fonts.tile, fonts.tile->FontSize, ImVec2(origin.x + S(26), origin.y + S(18)), color::ink, "INSTALLING");
-    if (!activity.empty())
+    draw->AddText(fonts.tile, fonts.tile->FontSize, ImVec2(origin.x + S(26), origin.y + S(18)), color::ink,
+        cancelling ? "CANCELLING" : "INSTALLING");
+    const auto line = cancelling ? std::string("Stopping as soon as the current file is done. "
+                                               "The Mods folder is left as it was.")
+                                 : activity;
+    if (!line.empty())
         draw->AddText(fonts.bold, fonts.bold->FontSize, ImVec2(origin.x + S(26), origin.y + S(70)),
-            rgba(0, 0, 0, 0.78f), activity.c_str(), nullptr, extent.x - S(52));
+            rgba(0, 0, 0, 0.78f), line.c_str(), nullptr, extent.x - S(52));
     skate_theme::striped_bar(draw, ImVec2(origin.x + S(26), origin.y + S(118)),
         ImVec2(end.x - S(26), origin.y + S(134)), fraction, time, g_scale);
     draw->AddText(fonts.bold, fonts.bold->FontSize, ImVec2(origin.x + S(26), origin.y + S(142)), rgba(0, 0, 0, 0.78f),
@@ -431,8 +438,17 @@ void install_modal(const Fonts& fonts, ModsPanel& panel, ImVec2 size) {
     ImGui::PushStyleColor(ImGuiCol_ButtonHovered, ImGui::ColorConvertU32ToFloat4(rgba(0, 0, 0, 0.52f)));
     ImGui::PushStyleColor(ImGuiCol_ButtonActive, ImGui::ColorConvertU32ToFloat4(rgba(0, 0, 0, 0.66f)));
     ImGui::SetCursorScreenPos(ImVec2(end.x - S(26) - S(130), end.y - S(24) - S(34)));
-    if (ImGui::Button("CANCEL", ImVec2(S(130), S(34)))) panel.cancel = true;
+    ImGui::BeginDisabled(cancelling);
+    const bool pressed = ImGui::Button(cancelling ? "CANCELLING" : "CANCEL", ImVec2(S(130), S(34)));
+    ImGui::EndDisabled();
     ImGui::PopStyleColor(3);
+    // Escape too: the button is the only way out of this panel otherwise.
+    if (!cancelling && (pressed || ImGui::IsKeyPressed(ImGuiKey_Escape, false))) {
+        panel.cancel = true;
+        logging::write(logging::Level::info, logging::Channel::launcher,
+            pressed ? "Mod install cancelled by the Cancel button."
+                    : "Mod install cancelled with Escape.");
+    }
     ImGui::End();
 }