From 770029f2070e0a7bcad5c51a502c8e72555a3a3e Mon Sep 17 00:00:00 2001 From: funman300 Date: Fri, 21 Aug 2026 19:50:13 +0000 Subject: [PATCH] fix(import-fifa17): carry the fields a consumable needs to exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects that together made the club's 17 owned consumables invisible while club/stats still counted them — the count gate promised 17, the item route served 0. 1. The catalog omitted `card_asset_id`, `amount`, `contract` and `rating` for non-player definitions. Without an art id the adapter refuses to emit the card (it would draw the notfound box), and the families that read `amount`/`contract` would render "-1" or grant nothing. All four values are present in the source wire and were simply dropped on the way out. 2. Owned rows were imported without a `content_kind`, and Core defaults an unstated row to `player` — durably recording a fitness coach and a contract card as players in the ownership authority, even though the catalog-driven wire looked right. These are definition-level fields, so every owned copy must agree; a group that disagrees is deferred rather than resolved by taking the first copy's value. Measured on the real profile: observed `amount` equals the `fcc_*` table row for every consumable that carries one (1, 2, 4, 5, 10, 15), and a wire omission corresponds to a table amount of 0. It is therefore NOT a stack count — two copies of 5003068 are two instances — so the import states no `quantity` at all. --- openfut-import-fifa17/src/apply.rs | 7 +++ openfut-import-fifa17/src/lib.rs | 44 +++++++++++++++-- openfut-import-fifa17/src/model.rs | 4 +- openfut-import-fifa17/src/tests.rs | 76 ++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 5 deletions(-) diff --git a/openfut-import-fifa17/src/apply.rs b/openfut-import-fifa17/src/apply.rs index ce224fb..dad4f5a 100644 --- a/openfut-import-fifa17/src/apply.rs +++ b/openfut-import-fifa17/src/apply.rs @@ -70,6 +70,11 @@ pub struct GenericClub { pub struct GenericOwned { pub owned_item_id: String, pub card_id: String, + /// Generic classification for Core's ownership row. FIFA17 is the only side + /// that can map `cardsubtypeid` onto this, and Core defaults to `player`, so + /// leaving it off would durably record a coach or a contract card as a + /// player — wrong in the ownership authority even when the wire looks right. + pub content_kind: &'static str, } #[derive(Debug, Serialize)] @@ -174,6 +179,7 @@ pub fn plan_apply( owned.push(GenericOwned { owned_item_id: core_id.clone(), card_id: def.card_id.clone(), + content_kind: "player", }); mappings.push(IdentityMapping { core_id, @@ -191,6 +197,7 @@ pub fn plan_apply( owned.push(GenericOwned { owned_item_id: core_id.clone(), card_id: def.card_id.clone(), + content_kind: def.kind.as_str(), }); mappings.push(IdentityMapping { core_id, diff --git a/openfut-import-fifa17/src/lib.rs b/openfut-import-fifa17/src/lib.rs index 2218def..d674146 100644 --- a/openfut-import-fifa17/src/lib.rs +++ b/openfut-import-fifa17/src/lib.rs @@ -546,6 +546,21 @@ pub struct NonPlayerDefinition { pub card_asset_id: Option, /// Source team id for a kit definition, when present. pub team_id: Option, + /// Consumable effect magnitude (`amount`), when the source carries one. + /// + /// Definition-level, and measured to be so: across every owned consumable in + /// the real profile the observed `amount` equals its `fcc_*` table row + /// (1, 2, 4, 5, 10, 15 — no disagreements), and a wire omission corresponds + /// to a table `amount` of 0. It is NOT a stack count: two copies of 5003068 + /// arrive as two separate instances, each with the same amount. + pub amount: Option, + /// Contract-card payload (`contract`), when the source carries one. Present + /// on exactly the contract families (subtypes 201/202) and absent from every + /// other owned consumable, so it is the card's own field rather than the + /// generic per-item contract atom that players and staff carry. + pub contract: Option, + /// Card rating, when the source carries one (matches the `fcc_*` row). + pub rating: Option, /// Honest functional label (e.g. "Player Contract", "GK Coach", "Kit"). pub name: String, /// Wire ids of every owned copy of this resourceId (preserved). @@ -635,10 +650,20 @@ pub fn plan_non_player_definitions(profile: &Profile) -> NonPlayerPlan { let card_asset_id = items[0].cardassetid; let team_id = items[0].teamid; - if items - .iter() - .any(|item| item.cardassetid != card_asset_id || item.teamid != team_id) - { + // These are definition-level, so every owned copy must agree. Two copies + // of one consumable that disagreed would mean the field is really + // per-instance, and silently taking the first copy's value would bake a + // guess into the catalog — so defer the whole group instead. + let amount = items[0].amount; + let contract = items[0].contract; + let rating = items[0].rating; + if items.iter().any(|item| { + item.cardassetid != card_asset_id + || item.teamid != team_id + || item.amount != amount + || item.contract != contract + || item.rating != rating + }) { plan.deferred.push(DeferredNonPlayer { resource_id, subtype: Some(subtype), @@ -700,6 +725,9 @@ pub fn plan_non_player_definitions(profile: &Profile) -> NonPlayerPlan { subtype, card_asset_id, team_id, + amount, + contract, + rating, name: label.to_string(), wire_ids, }); @@ -1184,6 +1212,14 @@ pub fn emit_content( serde_json::json!({ "card_asset_id": d.card_asset_id, "team_id": d.team_id, + // A consumable is unrenderable without these: the adapter refuses + // to emit a card whose art id it does not know, and the families + // that read `amount`/`contract` draw "-1" or grant nothing when + // the field is missing. Omitting them here is what kept every + // owned consumable off the wire. + "amount": d.amount, + "contract": d.contract, + "rating": d.rating, "asset_id": d.asset_id.unwrap_or(d.resource_id), "version": 0, "rareflag": 0, diff --git a/openfut-import-fifa17/src/model.rs b/openfut-import-fifa17/src/model.rs index f6a3c36..0254cf6 100644 --- a/openfut-import-fifa17/src/model.rs +++ b/openfut-import-fifa17/src/model.rs @@ -70,7 +70,9 @@ pub struct Item { /// Consumable ART id (small id), distinct from `resourceId`. Permissive. #[serde(default)] pub cardassetid: Option, - /// Consumable stack size (`amount`). Permissive. + /// Consumable effect magnitude (`amount`) — NOT a stack size: every owned + /// copy is its own instance and carries its definition's value, which is + /// exactly the `fcc_*` table's `amount` column. Permissive. #[serde(default)] pub amount: Option, /// Staff/contract `contract` count. Permissive. diff --git a/openfut-import-fifa17/src/tests.rs b/openfut-import-fifa17/src/tests.rs index 4a7d16d..c45493d 100644 --- a/openfut-import-fifa17/src/tests.rs +++ b/openfut-import-fifa17/src/tests.rs @@ -714,6 +714,58 @@ fn plan_non_player_supports_seventeen_consumables_and_three_staff() { assert_eq!(by_id("fifa17_3000003").name, "GK Coach"); } +/// A consumable the adapter cannot render is a consumable the club cannot use: +/// the shaper drops any card whose art id it does not know, and the families +/// that read `amount`/`contract` would draw "-1" or grant nothing. Emitting the +/// definition without these fields is exactly what kept all 17 owned +/// consumables off the wire while club/stats still counted them. +#[test] +fn consumable_definitions_carry_the_fields_the_client_renders() { + let items = vec![ + // training +15: carries `amount`, no `contract` + r#"{"id":100000201,"resourceId":5003012,"assetId":5003012,"itemType":"player", + "cardsubtypeid":54,"cardassetid":3,"amount":15,"rating":85,"rareflag":0}"# + .to_string(), + // player contract: carries `contract`, no `amount` + r#"{"id":100000202,"resourceId":5001004,"assetId":5001004,"itemType":"player", + "cardsubtypeid":201,"cardassetid":7,"contract":7,"rating":60,"rareflag":0}"# + .to_string(), + ]; + let plan = plan_non_player_definitions(&profile(&items, "[]", 100000500)); + assert!(plan.deferred.is_empty(), "{:?}", plan.deferred); + let by_id = |cid: &str| plan.supported.iter().find(|d| d.card_id == cid).unwrap(); + + let training = by_id("fifa17_5003012"); + assert_eq!(training.card_asset_id, Some(3), "card art id"); + assert_eq!(training.amount, Some(15), "effect magnitude"); + assert_eq!(training.contract, None); + assert_eq!(training.rating, Some(85)); + + let contract = by_id("fifa17_5001004"); + assert_eq!(contract.card_asset_id, Some(7)); + assert_eq!(contract.contract, Some(7)); + assert_eq!(contract.amount, None, "contract families ignore amount"); +} + +/// `amount` is definition-level (every owned copy of a card carries the same +/// value), so two copies that DISAGREE mean the field is really per-instance. +/// Taking the first copy's value would silently bake a guess into the catalog. +#[test] +fn disagreeing_render_metadata_defers_rather_than_guessing() { + let items = vec![ + r#"{"id":100000201,"resourceId":5003012,"assetId":5003012,"itemType":"player", + "cardsubtypeid":54,"cardassetid":3,"amount":15,"rareflag":0}"# + .to_string(), + r#"{"id":100000202,"resourceId":5003012,"assetId":5003012,"itemType":"player", + "cardsubtypeid":54,"cardassetid":3,"amount":10,"rareflag":0}"# + .to_string(), + ]; + let plan = plan_non_player_definitions(&profile(&items, "[]", 100000500)); + assert!(plan.supported.is_empty(), "must not pick a winner"); + assert_eq!(plan.deferred.len(), 1); + assert_eq!(plan.deferred[0].reason, "render_metadata_conflict"); +} + #[test] fn kit_missing_render_metadata_defers() { let item = r#"{"id":100000501,"resourceId":6300006,"assetId":6300006, @@ -893,6 +945,30 @@ fn plan_apply_mints_non_player_owned_instances() { assert!(cards.contains("fifa17_5003012"), "consumable minted"); assert!(cards.contains("fifa17_3000083"), "staff minted"); assert!(cards.contains("fifa17_6300006"), "kit minted"); + // Core is the ownership authority, and it defaults an unstated row to + // `player`. A coach or a contract card durably recorded as a player is wrong + // in the authority even while the catalog-driven wire still looks right. + let kind_of = |card: &str| { + plan.request + .owned + .iter() + .find(|o| o.card_id == card) + .unwrap() + .content_kind + }; + assert_eq!(kind_of("fifa17_20801"), "player"); + assert_eq!(kind_of("fifa17_5003012"), "consumable"); + assert_eq!(kind_of("fifa17_3000083"), "staff"); + assert_eq!(kind_of("fifa17_6300006"), "kit"); + // `amount` is an effect magnitude, not a stack count: two copies of one + // consumable are two rows, never one row of quantity 2. The importer states + // no quantity at all, and Core's default for an absent quantity is "not a + // stack" — so the guarantee is that the key never appears in the request. + let wire = serde_json::to_string(&plan.request).unwrap(); + assert!( + !wire.contains("quantity"), + "no instance may claim to be a stack: {wire}" + ); // Deterministic OwnedItemId per (persona, wire) — same rule as players. let m = plan .mappings