funman300
eab522a1eb
Driven by a retail FIFA 17 capture: the client sends the WHOLE squad on
every save (~2KB, every slot/item/kit number), and a user swapping two
players produced nine changed slots across two saves. Slot deltas
therefore do not describe intent, so the only honest semantic operation
is "this is the squad now".
replace_squad, and why save_squad no longer has its own write path
------------------------------------------------------------------
save_squad UPDATEd the squad, DELETEd every squad_players row, then
INSERTed the new ones one at a time -- all outside a transaction. A
failure part-way through left a squad with some old players deleted and
only some new ones written: a state nobody asked for and no client can
detect. It also never checked that the cards being placed belonged to the
club, and accepted the same card in two slots.
replace_squad validates BEFORE any write (so a rejection leaves the
stored squad untouched) and performs every write in one transaction:
- card must exist AND belong to this club
- a card may occupy at most one slot
- a slot may hold at most one card
- slot indices must not be negative
- the squad being replaced must belong to this club
save_squad is now a thin wrapper over it. That deliberately tightens the
existing Core REST route -- it now validates ownership and rejects
duplicates. Those were bugs, and two write paths with different
guarantees is how the stricter one gets bypassed.
A cross-club card is reported as NotFound, not Forbidden: whether a card
exists in someone else's club is not the caller's business.
Game-rules boundary
-------------------
Core contained calculate_chemistry -- a full FUT-style link-scoring
formula. Chemistry is game-specific and changed between FIFA
generations, so a formula compiled into generic Core quietly makes Core a
FIFA-something server.
It now sits behind SquadRules, with the existing implementation preserved
byte-for-byte in behaviour as DefaultSquadRules ("openfut-default-v2").
Rules take a resolved SquadSnapshot of pure data rather than a pool and a
card database, so they are synchronous, testable without fixtures, and
cannot reach Core's storage. Fifa17SquadRules is deliberately NOT
written: the algorithm is unproven and inventing one is worse than having
none.
Client-reported values
----------------------
FIFA sends its own chemistry/rating/starRating. ClientReportedEvaluation
is a DIFFERENT TYPE from SquadEvaluation, so assigning one where the
other belongs does not compile. Disagreement is reported through
EvaluationComparison and never reconciled in either direction -- the
server's value stands and the mismatch is surfaced for investigation
against the exact squad that produced it.
Evidence
--------
17 unit tests, 113 in the crate, 7/7 mutations killed including
"ownership check removed", "replacement becomes a merge" and
"client-reported chemistry becomes the server value".
Scope note: `cargo fmt` without -p reformatted ~27 unrelated files; those
were reverted so this commit touches only the squad path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
100 lines
2.9 KiB
Rust
100 lines
2.9 KiB
Rust
use serde::{Deserialize, Serialize};
|
|
use uuid::Uuid;
|
|
|
|
#[derive(Debug, Clone, Serialize, Deserialize, sqlx::FromRow)]
|
|
pub struct Squad {
|
|
pub id: String,
|
|
pub club_id: String,
|
|
pub name: String,
|
|
pub formation: String,
|
|
pub created_at: String,
|
|
pub updated_at: String,
|
|
}
|
|
|
|
impl Squad {
|
|
pub fn new(
|
|
club_id: impl Into<String>,
|
|
name: impl Into<String>,
|
|
formation: impl Into<String>,
|
|
) -> Self {
|
|
let now = chrono::Utc::now().to_rfc3339();
|
|
Self {
|
|
id: Uuid::new_v4().to_string(),
|
|
club_id: club_id.into(),
|
|
name: name.into(),
|
|
formation: formation.into(),
|
|
created_at: now.clone(),
|
|
updated_at: now,
|
|
}
|
|
}
|
|
}
|
|
|
|
#[derive(Debug, Clone, Serialize, Deserialize, sqlx::FromRow)]
|
|
pub struct SquadPlayer {
|
|
pub id: String,
|
|
pub squad_id: String,
|
|
pub owned_card_id: String,
|
|
pub position_index: i64,
|
|
pub is_captain: bool,
|
|
pub is_on_bench: bool,
|
|
}
|
|
|
|
#[derive(Debug, Deserialize)]
|
|
pub struct SaveSquadRequest {
|
|
pub squad_id: Option<String>,
|
|
pub name: Option<String>,
|
|
pub formation: Option<String>,
|
|
pub players: Vec<SquadPlayerInput>,
|
|
}
|
|
|
|
#[derive(Debug, Deserialize)]
|
|
pub struct SquadPlayerInput {
|
|
pub owned_card_id: String,
|
|
pub position_index: i64,
|
|
pub is_captain: bool,
|
|
pub is_on_bench: bool,
|
|
}
|
|
|
|
/// A complete squad, as a game client sends it.
|
|
///
|
|
/// # Why replacement rather than edits
|
|
///
|
|
/// Retail FIFA 17 sends the WHOLE squad on every save — roughly 2 KB carrying
|
|
/// every slot, item id and kit number — and a user swapping two players
|
|
/// produced nine changed slots across two saves. Slot deltas therefore do not
|
|
/// describe what the user did, and any attempt to derive `swap_players` or
|
|
/// `move_player` from them would be inventing intent the wire never carried.
|
|
///
|
|
/// So the only honest semantic operation is: *this is the squad now*.
|
|
///
|
|
/// Empty slots are simply absent from `slots`; a client that models an empty
|
|
/// slot as a zero item id must drop it at the adapter boundary rather than
|
|
/// sending a player Core would have to special-case.
|
|
#[derive(Debug, Clone, Default)]
|
|
pub struct SquadReplacement {
|
|
pub name: Option<String>,
|
|
pub formation: Option<String>,
|
|
pub slots: Vec<SlotAssignment>,
|
|
}
|
|
|
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
|
pub struct SlotAssignment {
|
|
pub owned_card_id: String,
|
|
/// Core's slot numbering. The adapter maps the game's numbering onto it.
|
|
pub slot: i64,
|
|
pub is_captain: bool,
|
|
pub is_on_bench: bool,
|
|
}
|
|
|
|
/// Outcome of a replacement.
|
|
///
|
|
/// Carries the server's own evaluation and, separately, any disagreement with
|
|
/// what the client claimed — never a merged value.
|
|
#[derive(Debug, Clone)]
|
|
pub struct SquadReplaced {
|
|
pub squad: Squad,
|
|
pub slots_written: usize,
|
|
pub evaluation: crate::services::squad_rules::SquadEvaluation,
|
|
pub client_disagreements: Vec<crate::services::squad_rules::EvaluationComparison>,
|
|
}
|