Repository navigation
feat: add outfits, a skin library and REST equip broadcasts - #18
Conversation
Skins are stored as references to Mojang textures; applying an outfit swaps the equipped cosmetics and broadcasts the change.
Wyvest
left a comment
Wyvest
left a comment
There was a problem hiding this comment.
Reviewed the outfits/skins/broadcast changes. Main issue: apply ignores blank, so wearing a starter outfit wipes the player's look. Particle color also isn't kept in sync with the active outfit in a few places. Details inline.
| } | ||
|
|
||
| #[tracing::instrument(level = "debug", skip(state))] | ||
| async fn apply( |
There was a problem hiding this comment.
apply never looks at outfit.blank. Starter outfits #2–#8 are seeded with blank = true, no cosmetics and particle_color = None, so applying one deletes every player_equipped_cosmetic row, clears user.particle_color, and broadcasts an unequip for every slot. The PR description says starter outfits should pick up the current look the first time they're worn.
Suggest: when outfit.blank, copy the current equipment and particle color into the outfit, set blank = false, make it active, and leave equipment untouched instead of swapping.
| Ok(Some(trimmed.to_string())) | ||
| } | ||
|
|
||
| pub(crate) async fn mirror_active_outfit_slot<C: ConnectionTrait>( |
There was a problem hiding this comment.
Only slot changes get mirrored into the active outfit. In-game SetParticleColor goes through persist_particle_color_queue, which updates user.particle_color but not the active outfit's particle_color. So: active outfit A is red, player switches to blue in game, applies B, re-applies A, and the color goes back to red. Equivalent mirroring in the particle color queue would fix this.
| warn!("Unable to persist websocket equipment update: {error}"); | ||
| match result { | ||
| Ok(Some(player)) => { | ||
| if let Err(error) = mirror_active_outfit_slot( |
There was a problem hiding this comment.
Ordering race: this re-reads the user and mirrors into whatever outfit is active when the queued update is processed. If a player equips in game (runtime updated, update queued) and then immediately applies outfit B over REST, the queued update can land after apply commits. It then upserts the old slot into player_equipped_cosmetic and mirrors it into outfit B. DB and runtime disagree, and B is silently modified. The window is small, but it's the exact flow this PR enables (OneClient + running game).
| .await?; | ||
| } | ||
|
|
||
| User::update_many() |
There was a problem hiding this comment.
create makes the new outfit active but doesn't apply its particle_color. user.particle_color and runtime keep the old value, and nothing is broadcast. Also, when the body omits particle_color it's stored as NULL rather than the player's current color, so an outfit "created from the current look" clears particles the next time it's applied. Consider defaulting to player.particle_color (or the runtime value).
| if let Some(mojang_cape_id) = mojang_cape_id { | ||
| active.mojang_cape_id = Set(mojang_cape_id); | ||
| } | ||
| if let Some(particle_color) = body.particle_color { |
There was a problem hiding this comment.
PATCHing particle_color on the active outfit only changes the outfit row. user.particle_color and watchers keep the old color until the outfit is re-applied, so active outfit and live state diverge. If the outfit is active, write user.particle_color and call publish_particle_color_change.
| } | ||
|
|
||
| for (slot, value) in body.equipment.equipped.clone() { | ||
| mirror_active_outfit_slot(&txn, &player, slot, value).await?; |
There was a problem hiding this comment.
Mirroring now runs inside the equip transaction with ?, using the player row loaded at auth time. If the active outfit is deleted concurrently, the insert into player_outfit_cosmetic violates the FK and the whole equip rolls back with a 500. The websocket path only warns on mirror failure. Either re-read active_outfit_id inside the txn or treat mirror failure as non-fatal. (Minor: this also issues one updated_at UPDATE per slot. Could fold into the first loop and bump once.)
| .filter(player_outfit::Column::PlayerId.eq(player.id)) | ||
| .all(&txn) | ||
| .await?; | ||
| if existing.len() as u64 >= OUTFIT_LIMIT { |
There was a problem hiding this comment.
Count-then-insert under READ COMMITTED with no lock: two concurrent POST /outfits at 127 both pass and you end up at 129. Same pattern in skin add. Low impact, but SELECT ... FOR UPDATE on the user row (or a lock on user at the start of the txn) would make the limit real.
|
|
||
| let inserted = player_skin::ActiveModel { | ||
| player_id: Set(player.id), | ||
| name: Set(name.unwrap_or_else(|| format!("Skin #{}", count + 1))), |
| if let Some(favorite) = body.favorite { | ||
| active.favorite = Set(favorite); | ||
| } | ||
| if body.skin_id.is_some() || mojang_cape_id.is_some() { |
There was a problem hiding this comment.
body.skin_id.is_some() is also true for an explicit null (Some(None)), same for mojang_cape_id (and normalize_cape_id maps "" to None). Clearing a field on a blank outfit marks it non-blank even though nothing was set, contrary to the doc ("Setting skin_id or mojang_cape_id marks..."). Use matches!(body.skin_id, Some(Some(_))) etc.
| } | ||
| } | ||
|
|
||
| pub(crate) async fn publish_equipment_change( |
There was a problem hiding this comment.
This duplicates the runtime update + broadcast already inline in the SetEquippedCosmetic handler. Having the handler call this helper (with the owner echo made optional) keeps the two paths from drifting.
3 participants
Adds saved outfits and a skin library for the wardrobe in PolyPlus and OneClient, and broadcasts equipment changes made over REST so a running game picks them up.
Outfits
GETandPOST /outfits,PATCHandDELETE /outfits/{id}, andPOST /outfits/{id}/applySkins
GETandPOST /skins,PATCHandDELETE /skins/{id}Equipment broadcast
PUT /cosmetics/playernow sendsPlayerCosmeticEquippedover the websocket, the same as an equip made in game. Without it, changes made in OneClient don't reach a game that's already running.Database
One migration (
m20261015_000000_create_outfits_and_skins) adds theplayer_skin,player_outfitandplayer_outfit_cosmetictables and an active outfit column onuser.The clients are in Polyfrost/PolyPlus#205 and Polyfrost/OneLauncher#1003.