diff --git a/openfut-adapter-fifa17/src/fut/squad.rs b/openfut-adapter-fifa17/src/fut/squad.rs index 350db85..ce0abae 100644 --- a/openfut-adapter-fifa17/src/fut/squad.rs +++ b/openfut-adapter-fifa17/src/fut/squad.rs @@ -144,13 +144,28 @@ pub struct ProposedSquad { /// The owned instance assigned as the squad's **manager**, reverse-resolved /// from the wire `manager` ref to a Core `owned_card_id` (so the assignment /// is ownership-backed, never a dangling wire id). `None` when the save - /// carries no manager. A manager wire id the resolver cannot map is reported - /// in `unresolved_wire_ids` — a save is refused rather than assigning a - /// manager the club does not own. + /// carries no manager, or when its manager ref does not resolve — see + /// [`Self::unresolved_manager_wire_id`]. pub manager_owned_card_id: Option, - /// Occupied wire item ids the resolver could not map. A caller MUST refuse the - /// replacement if this is non-empty — a save must never silently drop an - /// owned player it failed to identify. + /// A non-zero manager ref the resolver could not map, if any. + /// + /// This does NOT refuse the save. FIFA 17 sends a manager ref that is not an + /// owned club item: production's own squad points at instance 100000427, + /// which is absent from production's `/club/staff` listing (1975 items, + /// 100000001..100004826), and the client accepts that squad back unchanged — + /// so the client does not validate the manager against the club, and refusing + /// the save would break EVERY real squad save for a field that was not even + /// ownership-backed before migration 0023. + /// + /// An unresolvable ref therefore means "no ownership-backed manager": the + /// assignment is cleared, exactly as a full replacement should, and the id is + /// reported so the host can log what it could not map. An occupied PLAYER + /// slot is different and still refuses the save — dropping one would silently + /// lose an owned card from the club. + pub unresolved_manager_wire_id: Option, + /// Occupied PLAYER wire item ids the resolver could not map. A caller MUST + /// refuse the replacement if this is non-empty — a save must never silently + /// drop an owned player it failed to identify. pub unresolved_wire_ids: Vec, } @@ -215,13 +230,15 @@ pub fn to_proposed(put: &Fifa17SquadPut, resolver: &dyn SquadWireResolver) -> Pr } } // Manager: the first non-zero manager ref, reverse-resolved to an owned - // instance. An unresolvable manager is an unresolved wire id (refused), not a - // silently dropped assignment — you cannot manage with a card you don't own. + // instance. Unresolvable is NOT fatal (see `unresolved_manager_wire_id`) -- + // the real client always sends a dangling ref, so refusing would break every + // squad save. let mut manager_owned_card_id = None; + let mut unresolved_manager_wire_id = None; if let Some(wire) = put.manager.iter().map(|m| m.id).find(|&id| id != 0) { match resolver.owned_id_for_wire(wire) { Some(owned) => manager_owned_card_id = Some(owned), - None => unresolved.push(wire), + None => unresolved_manager_wire_id = Some(wire), } } ProposedSquad { @@ -230,6 +247,7 @@ pub fn to_proposed(put: &Fifa17SquadPut, resolver: &dyn SquadWireResolver) -> Pr formation: put.formation.clone(), slots, manager_owned_card_id, + unresolved_manager_wire_id, unresolved_wire_ids: unresolved, } } diff --git a/openfut-adapter-fifa17/src/fut/squad_ext.rs b/openfut-adapter-fifa17/src/fut/squad_ext.rs index 2224806..d15f402 100644 --- a/openfut-adapter-fifa17/src/fut/squad_ext.rs +++ b/openfut-adapter-fifa17/src/fut/squad_ext.rs @@ -316,15 +316,24 @@ mod tests { assert_eq!(ext.kicktakers[0].index, 0); } + /// A manager ref that does not resolve must NOT refuse the save: FIFA always + /// sends one, and on a real profile it is dangling (production points at + /// 100000427, absent from its own /club/staff). The save commits with no + /// ownership-backed manager and reports the id it could not map. #[test] - fn build_refuses_an_unowned_manager() { - // Manager wire id present but NOT resolvable → refused, never assigned a - // manager the club does not own. + fn an_unresolvable_manager_ref_clears_the_assignment_without_refusing() { let put = parse_squad_put(PUT_F442.as_bytes()).unwrap(); let mut ids = full_resolver().0; ids.remove(&100000427); - let err = build_squad_write(&put, &MapResolver(ids)).unwrap_err(); - assert_eq!(err, SquadBuildError::UnresolvedWireIds(vec![100000427])); + let build = build_squad_write(&put, &MapResolver(ids)).expect("save must still commit"); + assert_eq!(build.canonical.manager_owned_card_id, None); + assert_eq!( + build.canonical.unresolved_manager_wire_id, + Some(100000427), + "the ref we could not map is reported, not swallowed" + ); + // The starting XI is untouched -- only the manager assignment is dropped. + assert_eq!(build.canonical.slots.len(), 11); } #[test] diff --git a/openfut-launcher b/openfut-launcher index 44ebc4b..7edf682 160000 --- a/openfut-launcher +++ b/openfut-launcher @@ -1 +1 @@ -Subproject commit 44ebc4b23cc8ac0a46801bf1b75ceee9d08833c1 +Subproject commit 7edf682291096583b5b7b4e0d04fec078b739faf diff --git a/openfut-utas-host/src/lib.rs b/openfut-utas-host/src/lib.rs index 0e7d8e6..38f6d7d 100644 --- a/openfut-utas-host/src/lib.rs +++ b/openfut-utas-host/src/lib.rs @@ -2131,6 +2131,17 @@ pub fn handle_put_squad(body: &[u8], deps: &SquadDeps<'_>) -> (WireResponse, Squ ); } } + // FIFA always sends a manager ref, and on a real profile it does not resolve + // to an owned instance (production's own save points at 100000427, absent + // from its /club/staff). That is not an error: the save commits with NO + // ownership-backed manager. Logged so a ref we cannot map stays visible + // instead of vanishing. + if let Some(wire) = build.canonical.unresolved_manager_wire_id { + eprintln!( + "utas-host owner=RUST route=squad-replace manager_ref_unresolved={wire} \ + (saved with no manager assignment)" + ); + } // Commit canonical + extension atomically. No Python fallback on failure. let req = CoreReplaceRequest { name: build.canonical.name.clone(), diff --git a/openfut-utas-host/tests/host_test.rs b/openfut-utas-host/tests/host_test.rs index 506bd64..86d0b78 100644 --- a/openfut-utas-host/tests/host_test.rs +++ b/openfut-utas-host/tests/host_test.rs @@ -1216,11 +1216,13 @@ fn put_assigns_the_owned_manager_and_a_later_save_clears_it() { ); } -/// A manager ref the resolver cannot map is an unresolved wire id: the WHOLE -/// save is refused and nothing is committed. You cannot manage with a card you -/// do not own, and a save must never silently drop the assignment instead. +/// The REAL client always sends a manager ref, and on a real profile it does not +/// resolve: production's own save points at 100000427, which is absent from +/// production's `/club/staff`, and the client accepts that squad back unchanged. +/// Refusing the save would therefore break EVERY squad save, so an unresolvable +/// manager ref commits the squad with NO manager assignment. #[test] -fn put_refuses_a_manager_ref_that_is_not_an_owned_instance() { +fn put_saves_the_squad_when_the_manager_ref_does_not_resolve() { let items = vec![gk(), st()]; let (resolver, w) = resolver_with_wires(&items, ASSETS); let core = FakeCore::new(items.clone(), 2); @@ -1239,10 +1241,28 @@ fn put_refuses_a_manager_ref_that_is_not_an_owned_instance() { Some(100_000_427), ); let (resp, log) = handle_put_squad(&body, &deps); + assert_eq!(resp.status, 200, "{log:?}"); + assert_eq!(resp.body, br#"{"id":0}"#); + assert_eq!(core.replace_calls(), 1, "the squad itself is committed"); + assert_eq!( + core.manager(), + None, + "no ownership-backed manager is invented from a ref we cannot map" + ); + + // An occupied PLAYER slot that does not resolve is the opposite case: it + // would silently lose an owned card, so it still refuses the whole save. + let bad_player = put_body( + "f442", + w["oc-a"], + &[(0, w["oc-a"], 1), (1, 999_999_999, 9)], + "[]", + None, + ); + let (resp, log) = handle_put_squad(&bad_player, &deps); assert_eq!(resp.status, 400); assert_eq!(log.outcome, "unresolved_wire_ids"); - assert_eq!(core.replace_calls(), 0, "nothing committed"); - assert_eq!(core.manager(), None, "no manager assigned"); + assert_eq!(core.replace_calls(), 1, "nothing further committed"); } #[test]