fix: skins QA problems + flow change (#6216)

* fix: skins backend bugs + apply flow

* fix: caching structure

* feat: collapse already duplicated skins + fix moj api spam

* fix: doc

* fix: flatten migrations

* feat: remove default cape/cape override concept

* fix: fmt + lint

* feat: remove SelectCapeModal for inline cape list

* feat: qa

* feat: virtualisation of skins sections + fix texture/model cache

* fix: lint

* fix: virt bugs + renderer fixes

* fix: qa bugs

* fix: doc

* fix: re-add click impulse anim from prototypes + re-add interact anim length cap

* fix: regressions

* devex: split up SkinPreviewrenderer

* fix: lint

* fix: introduce dynamic mode in virtual-scroll.ts

* feat: qa

* fix: nametag bug + remove minecon skin pack suffix

* feat: pain (literally)

* feat: user agent on moj reqs

* feat: impl per account flush queue for operations

* fix: breadcrumb

* chore: i18n pass

* fix: lint + prep + check

* fix: misalignments

packages/app-lib/src/state/minecraft_skins/mod.rs

@@ -5,81 +5,31 @@ use super::MinecraftSkinVariant;
 
 pub mod mojang_api;
 
-/// Represents the default cape for a Minecraft player.
-#[derive(Debug, Clone)]
-pub struct DefaultMinecraftCape {
-    /// The UUID of a cape for a Minecraft player, which comes from its profile.
-    ///
-    /// This UUID may or may not be different for every player, even if they refer to the same cape.
-    pub id: Uuid,
-}
-
-impl DefaultMinecraftCape {
-    pub async fn set(
-        minecraft_user_id: Uuid,
-        cape_id: Uuid,
-        db: impl sqlx::Acquire<'_, Database = sqlx::Sqlite>,
-    ) -> crate::Result<()> {
-        let minecraft_user_id = minecraft_user_id.as_hyphenated();
-        let cape_id = cape_id.as_hyphenated();
-
-        sqlx::query!(
-            "INSERT OR REPLACE INTO default_minecraft_capes (minecraft_user_uuid, id) VALUES (?, ?)",
-            minecraft_user_id, cape_id
-        )
-        .execute(&mut *db.acquire().await?)
-        .await?;
-
-        Ok(())
-    }
-
-    pub async fn get(
-        minecraft_user_id: Uuid,
-        db: impl sqlx::Acquire<'_, Database = sqlx::Sqlite>,
-    ) -> crate::Result<Option<Self>> {
-        let minecraft_user_id = minecraft_user_id.as_hyphenated();
-
-        Ok(sqlx::query_as!(
-            Self,
-            "SELECT id AS 'id: Hyphenated' FROM default_minecraft_capes WHERE minecraft_user_uuid = ?",
-            minecraft_user_id
-        )
-        .fetch_optional(&mut *db.acquire().await?)
-        .await?)
-    }
-
-    pub async fn remove(
-        minecraft_user_id: Uuid,
-        db: impl sqlx::Acquire<'_, Database = sqlx::Sqlite>,
-    ) -> crate::Result<()> {
-        let minecraft_user_id = minecraft_user_id.as_hyphenated();
-
-        sqlx::query!(
-            "DELETE FROM default_minecraft_capes WHERE minecraft_user_uuid = ?",
-            minecraft_user_id
-        )
-        .execute(&mut *db.acquire().await?)
-        .await?;
-
-        Ok(())
-    }
-}
-
-/// Represents a custom skin for a Minecraft player.
+/// Represents a saved skin row for a Minecraft player.
+///
+/// The same player and `texture_key` always point to the same saved skin.
+/// Changing the model variant or cape updates that saved skin instead of
+/// creating a second copy. Bundled default skins with a cape are also stored
+/// here so the cape can stay associated with the default skin card.
 #[derive(Debug, Clone)]
 pub struct CustomMinecraftSkin {
-    /// The key for the texture skin, which is akin to a hash that identifies it.
+    /// The key for the skin texture, which is akin to a hash that identifies it.
     pub texture_key: String,
     /// The variant of the skin model.
     pub variant: MinecraftSkinVariant,
     /// The UUID of the cape that this skin uses, which should match one of the
     /// cape UUIDs the player has in its profile.
     ///
-    /// If `None`, the skin does not have an explicit cape set, and the default
-    /// cape for this player, if any, should be used.
+    /// If `None`, the skin is saved without a cape.
     pub cape_id: Option<Uuid>,
 }
 
+struct CustomMinecraftSkinRow {
+    texture_key: String,
+    variant: MinecraftSkinVariant,
+    cape_id: Option<Hyphenated>,
+}
+
 impl CustomMinecraftSkin {
     pub async fn add(
         minecraft_user_id: Uuid,
@@ -95,24 +45,59 @@ impl CustomMinecraftSkin {
         let mut transaction = db.begin().await?;
 
         sqlx::query!(
-            "INSERT OR REPLACE INTO custom_minecraft_skin_textures (texture_key, texture) VALUES (?, ?)",
-            texture_key, texture
+            "DELETE FROM custom_minecraft_skins WHERE minecraft_user_uuid = ? AND texture_key = ?",
+            minecraft_user_id,
+            texture_key
         )
         .execute(&mut *transaction)
         .await?;
 
         sqlx::query!(
-            "INSERT OR REPLACE INTO custom_minecraft_skins (minecraft_user_uuid, texture_key, variant, cape_id) VALUES (?, ?, ?, ?)",
-            minecraft_user_id, texture_key, variant, cape_id
-        )
-        .execute(&mut *transaction)
-        .await?;
+			"INSERT OR REPLACE INTO custom_minecraft_skin_textures (texture_key, texture) VALUES (?, ?)",
+			texture_key, texture
+		)
+		.execute(&mut *transaction)
+		.await?;
+
+        sqlx::query!(
+			"INSERT OR REPLACE INTO custom_minecraft_skins (minecraft_user_uuid, texture_key, variant, cape_id) VALUES (?, ?, ?, ?)",
+			minecraft_user_id, texture_key, variant, cape_id
+		)
+		.execute(&mut *transaction)
+		.await?;
 
         transaction.commit().await?;
 
         Ok(())
     }
 
+    pub async fn get_by_texture(
+        minecraft_user_id: Uuid,
+        texture_key: &str,
+        db: impl sqlx::Acquire<'_, Database = sqlx::Sqlite>,
+    ) -> crate::Result<Option<Self>> {
+        let minecraft_user_id = minecraft_user_id.as_hyphenated();
+
+        sqlx::query_as!(
+            CustomMinecraftSkinRow,
+            "SELECT texture_key, variant AS 'variant: MinecraftSkinVariant', cape_id AS 'cape_id: Hyphenated' \
+            FROM custom_minecraft_skins \
+            WHERE minecraft_user_uuid = ? AND texture_key = ?",
+            minecraft_user_id,
+            texture_key
+        )
+        .fetch_optional(&mut *db.acquire().await?)
+        .await?
+        .map(|row| {
+            Ok(Self {
+                texture_key: row.texture_key,
+                variant: row.variant,
+                cape_id: row.cape_id.map(Uuid::from),
+            })
+        })
+        .transpose()
+    }
+
     pub async fn get_many(
         minecraft_user_id: Uuid,
         offset: u32,
@@ -165,12 +150,11 @@ impl CustomMinecraftSkin {
         db: impl sqlx::Acquire<'_, Database = sqlx::Sqlite>,
     ) -> crate::Result<()> {
         let minecraft_user_id = minecraft_user_id.as_hyphenated();
-        let cape_id = self.cape_id.map(|id| id.hyphenated());
 
         sqlx::query!(
-            "DELETE FROM custom_minecraft_skins \
-            WHERE minecraft_user_uuid = ? AND texture_key = ? AND variant = ? AND cape_id IS ?",
-            minecraft_user_id, self.texture_key, self.variant, cape_id
+            "DELETE FROM custom_minecraft_skins WHERE minecraft_user_uuid = ? AND texture_key = ?",
+            minecraft_user_id,
+            self.texture_key
         )
         .execute(&mut *db.acquire().await?)
         .await?;

packages/app-lib/src/state/minecraft_skins/mojang_api.rs

@@ -10,7 +10,10 @@ use super::MinecraftSkinVariant;
 use crate::{
     ErrorKind,
     data::Credentials,
-    state::{MinecraftProfile, PROFILE_CACHE, ProfileCacheEntry},
+    state::{
+        MINECRAFT_SERVICES_USER_AGENT, MinecraftProfile, PROFILE_CACHE,
+        ProfileCacheEntry,
+    },
     util::fetch::INSECURE_REQWEST_CLIENT,
 };
 
@@ -24,12 +27,13 @@ impl MinecraftCapeOperation {
     ) -> crate::Result<()> {
         update_profile_cache_from_response(
             INSECURE_REQWEST_CLIENT
-                .put("https://api.minecraftservices.com/minecraft/profile/capes/active")
-                .header("Content-Type", "application/json; charset=utf-8")
-                .header("Accept", "application/json")
-                .bearer_auth(&credentials.access_token)
-                .json(&json!({
-                    "capeId": cape_id.hyphenated(),
+				.put("https://api.minecraftservices.com/minecraft/profile/capes/active")
+				.header("Content-Type", "application/json; charset=utf-8")
+				.header("Accept", "application/json")
+				.header("User-Agent", MINECRAFT_SERVICES_USER_AGENT)
+				.bearer_auth(&credentials.access_token)
+				.json(&json!({
+					"capeId": cape_id.hyphenated(),
                 }))
                 .send()
                 .await
@@ -42,12 +46,13 @@ impl MinecraftCapeOperation {
 
     pub async fn unequip_any(credentials: &Credentials) -> crate::Result<()> {
         update_profile_cache_from_response(
-            INSECURE_REQWEST_CLIENT
-                .delete("https://api.minecraftservices.com/minecraft/profile/capes/active")
-                .header("Accept", "application/json")
-                .bearer_auth(&credentials.access_token)
-                .send()
-                .await
+			INSECURE_REQWEST_CLIENT
+				.delete("https://api.minecraftservices.com/minecraft/profile/capes/active")
+				.header("Accept", "application/json")
+				.header("User-Agent", MINECRAFT_SERVICES_USER_AGENT)
+				.bearer_auth(&credentials.access_token)
+				.send()
+				.await
                 .and_then(|response| response.error_for_status())?
         )
         .await;
@@ -64,7 +69,7 @@ impl MinecraftSkinOperation {
         credentials: &Credentials,
         texture: TextureStream,
         variant: MinecraftSkinVariant,
-    ) -> crate::Result<()>
+    ) -> crate::Result<Option<Arc<MinecraftProfile>>>
     where
         TextureStream: TryStream + Send + 'static,
         TextureStream::Error: Into<Box<dyn Error + Send + Sync>>,
@@ -91,12 +96,13 @@ impl MinecraftSkinOperation {
                     .file_name("skin.png"),
             );
 
-        update_profile_cache_from_response(
+        let profile = update_profile_cache_from_response(
             INSECURE_REQWEST_CLIENT
                 .post(
                     "https://api.minecraftservices.com/minecraft/profile/skins",
                 )
                 .header("Accept", "application/json")
+                .header("User-Agent", MINECRAFT_SERVICES_USER_AGENT)
                 .bearer_auth(&credentials.access_token)
                 .multipart(form)
                 .send()
@@ -105,17 +111,18 @@ impl MinecraftSkinOperation {
         )
         .await;
 
-        Ok(())
+        Ok(profile)
     }
 
     pub async fn unequip_any(credentials: &Credentials) -> crate::Result<()> {
         update_profile_cache_from_response(
-            INSECURE_REQWEST_CLIENT
-                .delete("https://api.minecraftservices.com/minecraft/profile/skins/active")
-                .header("Accept", "application/json")
-                .bearer_auth(&credentials.access_token)
-                .send()
-                .await
+			INSECURE_REQWEST_CLIENT
+				.delete("https://api.minecraftservices.com/minecraft/profile/skins/active")
+				.header("Accept", "application/json")
+				.header("User-Agent", MINECRAFT_SERVICES_USER_AGENT)
+				.bearer_auth(&credentials.access_token)
+				.send()
+				.await
                 .and_then(|response| response.error_for_status())?
         )
         .await;
@@ -124,19 +131,24 @@ impl MinecraftSkinOperation {
     }
 }
 
-async fn update_profile_cache_from_response(response: reqwest::Response) {
+async fn update_profile_cache_from_response(
+    response: reqwest::Response,
+) -> Option<Arc<MinecraftProfile>> {
     let Some(mut profile) = response.json::<MinecraftProfile>().await.ok()
     else {
         tracing::warn!(
             "Failed to parse player profile from skin or cape operation response, not updating profile cache"
         );
-        return;
+        return None;
     };
 
     profile.fetch_time = Some(Instant::now());
+    let profile = Arc::new(profile);
 
     PROFILE_CACHE
         .lock()
         .await
-        .insert(profile.id, ProfileCacheEntry::Hit(Arc::new(profile)));
+        .insert(profile.id, ProfileCacheEntry::Hit(Arc::clone(&profile)));
+
+    Some(profile)
 }