diff --git a/migrations/0030_owned_card_training_single.sql b/migrations/0030_owned_card_training_single.sql new file mode 100644 index 0000000..3b0209c --- /dev/null +++ b/migrations/0030_owned_card_training_single.sql @@ -0,0 +1,66 @@ +-- Reshape attribute training to AT MOST ONE effect per instance, replaceable. +-- +-- WHY THIS SUPERSEDES 0029'S SHAPE. 0029 keyed on (owned_card_id, +-- attribute_index) and recorded that same-slot behaviour was UNKNOWN, enforcing +-- the unknown as a refusal. That was the honest shape while the semantics were +-- unrecovered. They are now recovered, and BOTH halves of 0029's shape are +-- wrong: +-- +-- * "You can only boost one attribute or all six. You can not do it with 2, 3, +-- 4 or 5 attributes." -- so two effects must never coexist on one instance, +-- which the old composite key permitted (and which staging demonstrated by +-- holding a slot-4 and a slot-1 effect at once). +-- * "When you apply a new training card to a player, he loses the improved +-- attributes of previous training cards. It does not accumulate, it +-- replaces." -- so a second apply REPLACES, it does not refuse. +-- +-- Both quotes are from the contemporaneous FIFA 17-specific training guide +-- (fifauteam, published 2016-09-08), corroborated by the shipped table: each +-- family has exactly 21 rows = 7 card types x 3 levels, and the 7th type in each +-- family (subtypes 57 and 67) is the only one flagged `weightrare = 2` with +-- amounts 3/6/10, matching the documented RARE "ALL" card at +3/+6/+10. +-- DOCUMENTED, corroborated TABLE_PROVEN. It is NOT LIVE_PROVEN against EA. +-- +-- 0029 is left intact rather than rewritten: it is already applied to the +-- supervised staging environment, so migration history matters there. +-- +-- NEW SHAPE. One row per instance, so "one attribute or all six" is a +-- representable invariant instead of a convention: +-- attribute_index INTEGER NULL -- a slot in Core's six-attribute model, or +-- NULL meaning ALL SIX slots (the rare card). +-- The PRIMARY KEY on owned_card_id alone is what makes a second application a +-- REPLACE (delete-then-insert inside the one apply transaction) rather than an +-- accumulation. +-- +-- The 1..=15 amount bound is NOT tightened here: 15 is the single-attribute +-- ceiling while the all-six card authors at most 10, and which ceiling applies +-- depends on the card family -- a per-game rule that belongs at apply time where +-- the game's table is in scope, not in the schema. +-- +-- DATA CARRIED FORWARD: where an instance somehow holds several effects (only +-- reachable on staging under 0029's shape), the MOST RECENT survives, which is +-- exactly the "replaces" rule applied retroactively. + +CREATE TABLE owned_card_training_new ( + owned_card_id TEXT NOT NULL PRIMARY KEY REFERENCES owned_cards(id) ON DELETE CASCADE, + attribute_index INTEGER CHECK (attribute_index IS NULL OR attribute_index BETWEEN 0 AND 5), + amount INTEGER NOT NULL CHECK (amount >= 1 AND amount <= 99), + source_card_id TEXT NOT NULL, + applied_at TEXT NOT NULL +); + +INSERT INTO owned_card_training_new + (owned_card_id, attribute_index, amount, source_card_id, applied_at) +SELECT t.owned_card_id, t.attribute_index, t.amount, t.source_card_id, t.applied_at + FROM owned_card_training t + JOIN ( + SELECT owned_card_id, MAX(applied_at) AS newest + FROM owned_card_training + GROUP BY owned_card_id + ) pick + ON pick.owned_card_id = t.owned_card_id + AND pick.newest = t.applied_at + GROUP BY t.owned_card_id; + +DROP TABLE owned_card_training; +ALTER TABLE owned_card_training_new RENAME TO owned_card_training; diff --git a/src/routes/cards.rs b/src/routes/cards.rs index 7afc8fe..66ede99 100644 --- a/src/routes/cards.rs +++ b/src/routes/cards.rs @@ -147,15 +147,12 @@ pub async fn get_collection( let effective_overall = def.overall as i64 + o.training_bonus; let effective_position = o.position_override.as_deref().unwrap_or(&def.position); // Attribute training is per-instance state, so the finished attributes - // belong in the envelope beside the finished rating. The raw effects go + // belong in the envelope beside the finished rating. The raw effect goes // out too: a caller that needs to show WHICH attribute was trained // cannot recover that by differencing against a definition it may not - // have. - const NO_TRAINING: &[training_svc::TrainingEffect] = &[]; - let effects = training - .get(&o.id) - .map(Vec::as_slice) - .unwrap_or(NO_TRAINING); + // have. At most ONE effect per instance -- FIFA 17 replaces rather than + // accumulates, so this is an Option, not a list. + let effect = training.get(&o.id); let body = json!({ "owned_card_id": o.id, "content_kind": o.content_kind, @@ -172,8 +169,8 @@ pub async fn get_collection( "contract_matches": o.contract_matches, "effective_overall": effective_overall, "effective_position": effective_position, - "effective_attributes": training_svc::effective_attributes_json(def, effects), - "training": effects, + "effective_attributes": training_svc::effective_attributes_json(def, effect), + "training": effect, "card": def, }); views.push(OwnedItemView { diff --git a/src/services/instance_effect.rs b/src/services/instance_effect.rs index 7bfc46a..0b09df9 100644 --- a/src/services/instance_effect.rs +++ b/src/services/instance_effect.rs @@ -44,20 +44,29 @@ pub enum InstanceEffect { cap: i64, default_when_unset: i64, }, - /// Attach an attribute training effect to the target. + /// Attach an attribute training effect to the target, REPLACING any the + /// instance already carries. /// /// `attribute_index` is a slot in Core's own six-attribute card model, in - /// `CardDefinition` declaration order (0 pace .. 5 physical). Naming the - /// slot rather than the game's attribute is what keeps this game-neutral: - /// that FIFA 17's "GK speed" is slot 4 is the adapter's reversed knowledge, - /// and it stays there. + /// `CardDefinition` declaration order (0 pace .. 5 physical), or `None` for + /// an effect that boosts ALL SIX slots. Naming the slot rather than the + /// game's attribute is what keeps this game-neutral: that FIFA 17's "GK + /// speed" is slot 4 is the adapter's reversed knowledge, and it stays there. /// - /// `max_amount` is the caller's authored ceiling for its own family (FIFA 17 - /// authors 5/10/15, so 15). Core cannot know it, but it can refuse anything - /// above the number the caller itself declares, which is what stops a host - /// from describing a "+99 pace" that no card could grant. + /// REPLACEMENT, NOT ACCUMULATION, and at most one effect per instance. Both + /// halves are the caller's game rule, but they are enforced here because the + /// storage shape is Core's: FIFA 17's own documentation states "you can only + /// boost one attribute or all six" and "when you apply a new training card + /// to a player, he loses the improved attributes of previous training cards. + /// It does not accumulate, it replaces." + /// + /// `max_amount` is the caller's authored ceiling for the specific family + /// (FIFA 17: 15 single-attribute, 10 for the rare all-six card). Core cannot + /// know it, but it can refuse anything above the number the caller itself + /// declares, which is what stops a host describing a "+99 pace" no card + /// could grant. ApplyTraining { - attribute_index: i64, + attribute_index: Option, amount: i64, max_amount: i64, }, @@ -192,16 +201,18 @@ const ATTRIBUTE_SLOTS: i64 = 6; async fn apply_training( tx: &mut SqliteConnection, ctx: &ConsumeContext, - attribute_index: i64, + attribute_index: Option, amount: i64, max_amount: i64, ) -> AppResult { let target = require_target(ctx, "apply_training")?; - if !(0..ATTRIBUTE_SLOTS).contains(&attribute_index) { - return Err(AppError::BadRequest(format!( - "apply_training attribute_index must be 0..{ATTRIBUTE_SLOTS}, got {attribute_index}" - ))); + if let Some(slot) = attribute_index { + if !(0..ATTRIBUTE_SLOTS).contains(&slot) { + return Err(AppError::BadRequest(format!( + "apply_training attribute_index must be 0..{ATTRIBUTE_SLOTS} or absent, got {slot}" + ))); + } } if amount < 1 { return Err(AppError::BadRequest(format!( @@ -223,7 +234,7 @@ async fn apply_training( } // Same reasoning as contracts: a loan is borrowed for a fixed run of // matches, so durably improving it would outlive the thing it is attached - // to. Conservative and consistent rather than reversed — no FIFA 17 source + // to. Conservative and consistent rather than reversed -- no FIFA 17 source // speaks to training a loan item. if target.is_loan { return Err(AppError::BadRequest(format!( @@ -232,18 +243,28 @@ async fn apply_training( ))); } - // The INSERT is the check. `(owned_card_id, attribute_index)` is the primary - // key, so a second training on a slot that already carries one collides here - // and the whole transaction rolls back — the source is NOT consumed. That is - // deliberate: replacement-vs-stacking is UNKNOWN (see migration 0029), and a - // refusal is the only answer that neither invents a rule nor silently eats a - // card. Detected as a constraint violation rather than a SELECT-then-INSERT - // so it holds against a concurrent writer instead of racing it. - let inserted = sqlx::query( + // REPLACE. One instance carries at most one training effect, and a new card + // supersedes whatever was there -- including an effect on a DIFFERENT slot, + // because FIFA 17 allows "one attribute or all six" and never a mixture. + // Delete-then-insert inside the caller's transaction, so the old effect can + // never survive a failed insert and the two can never coexist. + let previous = sqlx::query_as::<_, (Option, i64, String)>( + "SELECT attribute_index, amount, source_card_id FROM owned_card_training \ + WHERE owned_card_id = ?", + ) + .bind(&target.id) + .fetch_optional(&mut *tx) + .await?; + + sqlx::query("DELETE FROM owned_card_training WHERE owned_card_id = ?") + .bind(&target.id) + .execute(&mut *tx) + .await?; + + sqlx::query( "INSERT INTO owned_card_training \ (owned_card_id, attribute_index, amount, source_card_id, applied_at) \ - VALUES (?, ?, ?, ?, ?) \ - ON CONFLICT(owned_card_id, attribute_index) DO NOTHING", + VALUES (?, ?, ?, ?, ?)", ) .bind(&target.id) .bind(attribute_index) @@ -251,17 +272,7 @@ async fn apply_training( .bind(&ctx.source.card_id) .bind(Utc::now().to_rfc3339()) .execute(&mut *tx) - .await? - .rows_affected(); - - if inserted != 1 { - return Err(AppError::Conflict(format!( - "target item '{}' already carries training on attribute slot {attribute_index}; \ - FIFA 17 replacement/stacking behaviour is unproven, so this is refused rather \ - than guessed", - target.id - ))); - } + .await?; Ok(json!({ "kind": "apply_training", @@ -270,12 +281,19 @@ async fn apply_training( // Named `granted` as well as `amount` so every effect's recorded outcome // answers "what did this card award" under one key, whatever the family. "granted": amount, - // `before`/`after` describe the TRAINING held on this slot, not the + // `before`/`after` describe the training this instance holds, not the // attribute's value: Core stores effects, and the attribute total is a // projection over a definition Core does not consult here. `before` is - // always 0 because a slot that already carried training refused above. - "before": 0, + // the magnitude of the effect this one replaced, 0 when there was none. + "before": previous.as_ref().map(|(_, a, _)| *a).unwrap_or(0), "after": amount, + // What was displaced, so the outcome records the replacement rather than + // silently overwriting history. + "replaced": previous.map(|(slot, amt, src)| json!({ + "attribute_index": slot, + "amount": amt, + "source_card_id": src, + })), })) } @@ -369,16 +387,25 @@ mod tests { fn train(attribute_index: i64, amount: i64) -> InstanceEffect { InstanceEffect::ApplyTraining { - attribute_index, + attribute_index: Some(attribute_index), amount, max_amount: 15, } } - async fn training_of(pool: &db::Pool, id: &str) -> Vec<(i64, i64, String)> { - sqlx::query_as::<_, (i64, i64, String)>( + /// The rare card: every slot, ceiling 10. + fn train_all(amount: i64) -> InstanceEffect { + InstanceEffect::ApplyTraining { + attribute_index: None, + amount, + max_amount: 10, + } + } + + async fn training_of(pool: &db::Pool, id: &str) -> Vec<(Option, i64, String)> { + sqlx::query_as::<_, (Option, i64, String)>( "SELECT attribute_index, amount, source_card_id FROM owned_card_training \ - WHERE owned_card_id = ? ORDER BY attribute_index", + WHERE owned_card_id = ?", ) .bind(id) .fetch_all(pool) @@ -421,20 +448,20 @@ mod tests { "granted": 10, "before": 0, "after": 10, + "replaced": null, }) ); assert_eq!( training_of(&pool, "fresh").await, - vec![(4, 10, "def-card-1".to_string())] + vec![(Some(4), 10, "def-card-1".to_string())] ); assert!(!source_exists(&pool, "card-1").await); } - /// Replacement-vs-stacking is UNKNOWN, so a second card on the SAME slot is - /// refused — and, critically, the refusal rolls back the whole transaction, - /// so the player keeps the card rather than paying for nothing. + /// A second card on the SAME slot REPLACES the first and does not + /// accumulate: 10 then 15 leaves 15, never 25. #[tokio::test] - async fn a_second_training_on_the_same_slot_is_refused_and_the_card_survives() { + async fn a_second_training_on_the_same_slot_replaces_rather_than_accumulating() { let (_dir, pool) = fixture().await; consume_item( &pool, @@ -446,7 +473,7 @@ mod tests { .await .expect("first apply"); - let err = consume_item( + let out = consume_item( &pool, "prof", "club", @@ -454,22 +481,24 @@ mod tests { &train(4, 15), ) .await - .expect_err("second apply on the same slot must be refused"); - assert!(matches!(err, AppError::Conflict(_)), "got {err:?}"); + .expect("second apply replaces"); - // The original effect is untouched: not replaced, not stacked. assert_eq!( training_of(&pool, "fresh").await, - vec![(4, 10, "def-card-1".to_string())] + vec![(Some(4), 15, "def-card-2".to_string())], + "the newer effect must stand alone, not sum to 25" ); - // And the second card was NOT spent. - assert!(source_exists(&pool, "card-2").await); + assert_eq!(out.effect["before"], json!(10)); + assert_eq!(out.effect["after"], json!(15)); + assert_eq!(out.effect["replaced"]["amount"], json!(10)); + // Both cards were legitimately spent. + assert!(!source_exists(&pool, "card-2").await); } - /// Distinct slots are independent: each training card names exactly one - /// attribute, and nothing in the shipped data couples them. + /// A card on a DIFFERENT slot also replaces: FIFA 17 permits "one attribute + /// or all six", never a mixture, so two slots must never be boosted at once. #[tokio::test] - async fn distinct_slots_coexist_on_one_instance() { + async fn a_training_on_a_different_slot_still_replaces_the_previous_one() { let (_dir, pool) = fixture().await; for (identity, source, slot, amount) in [("act-1", "card-1", 4, 10), ("act-2", "card-2", 1, 5)] @@ -486,13 +515,66 @@ mod tests { } assert_eq!( training_of(&pool, "fresh").await, - vec![ - (1, 5, "def-card-2".to_string()), - (4, 10, "def-card-1".to_string()) - ] + vec![(Some(1), 5, "def-card-2".to_string())], + "only the newest effect may remain" ); } + /// The rare card boosts every slot, and replaces a single-attribute effect + /// exactly like any other new card. + #[tokio::test] + async fn the_all_six_card_stores_no_slot_and_replaces_a_single_attribute_effect() { + let (_dir, pool) = fixture().await; + consume_item( + &pool, + "prof", + "club", + &apply_request("act-1", "card-1", "fresh"), + &train(4, 15), + ) + .await + .expect("single-attribute apply"); + + consume_item( + &pool, + "prof", + "club", + &apply_request("act-2", "card-2", "fresh"), + &train_all(10), + ) + .await + .expect("all-six apply"); + + assert_eq!( + training_of(&pool, "fresh").await, + vec![(None, 10, "def-card-2".to_string())], + "the all-six effect stores a NULL slot and stands alone" + ); + } + + /// The all-six card authors at most +10; a caller declaring that ceiling + /// cannot then push +15 through it. + #[tokio::test] + async fn the_all_six_ceiling_is_enforced_independently() { + let (_dir, pool) = fixture().await; + let err = consume_item( + &pool, + "prof", + "club", + &apply_request("act-1", "card-1", "fresh"), + &InstanceEffect::ApplyTraining { + attribute_index: None, + amount: 15, + max_amount: 10, + }, + ) + .await + .expect_err("an over-ceiling all-six boost must be refused"); + assert!(matches!(err, AppError::BadRequest(_)), "got {err:?}"); + assert!(training_of(&pool, "fresh").await.is_empty()); + assert!(source_exists(&pool, "card-1").await); + } + /// The caller declares its own family's ceiling and is held to it. This is /// what stops a host describing a boost no card could grant. #[tokio::test] @@ -516,7 +598,7 @@ mod tests { #[tokio::test] async fn a_slot_outside_the_card_model_is_refused() { let (_dir, pool) = fixture().await; - for slot in [-1, 6, 99] { + for slot in [-1i64, 6, 99] { let err = consume_item( &pool, "prof", @@ -582,7 +664,7 @@ mod tests { assert_eq!(replay.effect, first.effect); assert_eq!( training_of(&pool, "fresh").await, - vec![(2, 15, "def-card-1".to_string())] + vec![(Some(2), 15, "def-card-1".to_string())] ); } diff --git a/src/services/training.rs b/src/services/training.rs index 7995dea..605842b 100644 --- a/src/services/training.rs +++ b/src/services/training.rs @@ -27,11 +27,15 @@ use crate::{db::Pool, error::AppResult, models::card::CardDefinition}; /// that keeps the projected card inside the model it is drawn from. pub const ATTRIBUTE_MAX: i64 = 99; -/// One training effect attached to one instance. +/// The one training effect an instance may carry. +/// +/// At most one per instance: FIFA 17 allows "one attribute or all six" and a new +/// card replaces the old, so a second concurrent effect is not representable. #[derive(Debug, Clone, Serialize, FromRow)] pub struct TrainingEffect { - /// Slot in Core's six-attribute model, `CardDefinition` declaration order. - pub attribute_index: i64, + /// Slot in Core's six-attribute model, `CardDefinition` declaration order, + /// or `None` for an effect that boosts ALL SIX slots. + pub attribute_index: Option, pub amount: i64, pub source_card_id: String, } @@ -44,30 +48,30 @@ pub struct TrainingEffect { pub async fn load_for_club( pool: &Pool, club_id: &str, -) -> AppResult>> { - let rows = sqlx::query_as::<_, (String, i64, i64, String)>( +) -> AppResult> { + let rows = sqlx::query_as::<_, (String, Option, i64, String)>( "SELECT t.owned_card_id, t.attribute_index, t.amount, t.source_card_id \ FROM owned_card_training t \ JOIN owned_cards o ON o.id = t.owned_card_id \ - WHERE o.club_id = ? \ - ORDER BY t.owned_card_id, t.attribute_index", + WHERE o.club_id = ?", ) .bind(club_id) .fetch_all(pool) .await?; - let mut by_instance: HashMap> = HashMap::new(); - for (owned_card_id, attribute_index, amount, source_card_id) in rows { - by_instance - .entry(owned_card_id) - .or_default() - .push(TrainingEffect { - attribute_index, - amount, - source_card_id, - }); - } - Ok(by_instance) + Ok(rows + .into_iter() + .map(|(owned_card_id, attribute_index, amount, source_card_id)| { + ( + owned_card_id, + TrainingEffect { + attribute_index, + amount, + source_card_id, + }, + ) + }) + .collect()) } /// The definition's six attributes in canonical slot order. @@ -88,14 +92,23 @@ pub fn base_attributes(def: &CardDefinition) -> [i64; 6] { /// Base attributes with any training folded in, clamped to the model's domain. /// +/// A `None` slot boosts ALL SIX attributes — FIFA 17's rare "all" training card. /// An out-of-range slot is ignored rather than panicking: the schema already /// refuses one, so reaching this would mean the row was written around Core, and /// dropping it degrades one attribute instead of failing every projection. -pub fn effective_attributes(def: &CardDefinition, effects: &[TrainingEffect]) -> [i64; 6] { +pub fn effective_attributes(def: &CardDefinition, effect: Option<&TrainingEffect>) -> [i64; 6] { let mut out = base_attributes(def); - for e in effects { - if let Some(slot) = out.get_mut(e.attribute_index as usize) { - *slot = (*slot + e.amount).clamp(0, ATTRIBUTE_MAX); + let Some(e) = effect else { return out }; + match e.attribute_index { + Some(slot) => { + if let Some(v) = out.get_mut(slot as usize) { + *v = (*v + e.amount).clamp(0, ATTRIBUTE_MAX); + } + } + None => { + for v in out.iter_mut() { + *v = (*v + e.amount).clamp(0, ATTRIBUTE_MAX); + } } } out @@ -104,9 +117,9 @@ pub fn effective_attributes(def: &CardDefinition, effects: &[TrainingEffect]) -> /// The same six values as a named object, for the projection envelope. pub fn effective_attributes_json( def: &CardDefinition, - effects: &[TrainingEffect], + effect: Option<&TrainingEffect>, ) -> serde_json::Value { - let a = effective_attributes(def, effects); + let a = effective_attributes(def, effect); serde_json::json!({ "pace": a[0], "shooting": a[1],