aecbff0de8
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>
381 lines
14 KiB
Rust
381 lines
14 KiB
Rust
//! Squad evaluation, behind a game-rules boundary.
|
|
//!
|
|
//! # Why this is a trait and not a function in `squad.rs`
|
|
//!
|
|
//! Chemistry, rating and star rating are **game-specific**. FUT chemistry
|
|
//! changed substantially between FIFA generations, so a single formula
|
|
//! compiled into generic Core would quietly make Core a FIFA-something server.
|
|
//! Core is allowed to understand that a squad *has* an evaluation; it is not
|
|
//! allowed to know how any particular game computes one.
|
|
//!
|
|
//! ```text
|
|
//! Core owns the squad, slots, items, persistence
|
|
//! | and the SEMANTIC concept of an evaluation
|
|
//! v
|
|
//! SquadRules how a specific game computes it
|
|
//! |
|
|
//! +-- DefaultSquadRules OpenFUT's own rules (the implementation that
|
|
//! | already existed in Core)
|
|
//! +-- Fifa17SquadRules NOT YET WRITTEN. The FIFA 17 algorithm is not
|
|
//! proven, and inventing one would be worse than
|
|
//! having none.
|
|
//! ```
|
|
//!
|
|
//! # Pure data in, evaluation out
|
|
//!
|
|
//! Rules take a [`SquadSnapshot`] — already resolved by Core from the database
|
|
//! — rather than a pool and a card database. That keeps every rules
|
|
//! implementation synchronous, dependency-free and testable without fixtures,
|
|
//! and it stops a game's rules from reaching into Core's storage.
|
|
//!
|
|
//! # Client-reported values are not evaluations
|
|
//!
|
|
//! FIFA 17 sends its own `chemistry`, `rating` and `starRating` on every squad
|
|
//! save. Those are observations about what the client believes, captured for
|
|
//! shadow validation, and they are deliberately a *different type* from
|
|
//! [`SquadEvaluation`] so no later code can pass one where the other belongs.
|
|
|
|
use serde::{Deserialize, Serialize};
|
|
|
|
/// One player in a squad, reduced to the attributes rules are allowed to see.
|
|
///
|
|
/// Deliberately not `OwnedCard` + `CardDefinition`: rules should not be able to
|
|
/// reach storage identifiers, loan state or acquisition history.
|
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
|
pub struct SquadPlayerCard {
|
|
/// Core's owned-card id. Present so an evaluation can attribute per-player
|
|
/// results; rules must not interpret its contents.
|
|
pub owned_card_id: String,
|
|
pub card_id: String,
|
|
pub name: String,
|
|
pub overall: u8,
|
|
pub position: String,
|
|
pub nation: String,
|
|
pub league: String,
|
|
pub club: String,
|
|
/// Slot this player occupies, in Core's numbering.
|
|
pub slot: i64,
|
|
pub on_bench: bool,
|
|
}
|
|
|
|
/// Everything a rules implementation may consider.
|
|
#[derive(Debug, Clone, Default)]
|
|
pub struct SquadSnapshot {
|
|
pub formation: String,
|
|
pub players: Vec<SquadPlayerCard>,
|
|
}
|
|
|
|
impl SquadSnapshot {
|
|
pub fn starters(&self) -> impl Iterator<Item = &SquadPlayerCard> {
|
|
self.players.iter().filter(|p| !p.on_bench)
|
|
}
|
|
}
|
|
|
|
/// The semantic result Core understands.
|
|
///
|
|
/// `chemistry` has no fixed scale here on purpose — `chemistry_max` travels
|
|
/// with it, because a later game may not use 100.
|
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
|
pub struct SquadEvaluation {
|
|
pub chemistry: i64,
|
|
pub chemistry_max: i64,
|
|
pub rating: i64,
|
|
pub star_rating: i64,
|
|
/// Per-player detail, for UIs and for diagnosing a rules mismatch.
|
|
pub players: Vec<PlayerEvaluation>,
|
|
/// Which rules produced this, so a stored or logged evaluation is never
|
|
/// ambiguous about its own provenance.
|
|
pub rules: String,
|
|
}
|
|
|
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
|
pub struct PlayerEvaluation {
|
|
pub owned_card_id: String,
|
|
pub chemistry: i64,
|
|
pub detail: Vec<(String, i64)>,
|
|
}
|
|
|
|
/// What a game client claimed about a squad it sent.
|
|
///
|
|
/// **Never canonical.** A separate type from [`SquadEvaluation`] specifically so
|
|
/// that assigning one to the other does not compile. A modified client can put
|
|
/// anything here; OpenFUT has no independent knowledge of what it means until
|
|
/// its own rules run.
|
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
|
|
pub struct ClientReportedEvaluation {
|
|
pub client_reported_chemistry: Option<i64>,
|
|
pub client_reported_rating: Option<i64>,
|
|
pub client_reported_star_rating: Option<i64>,
|
|
}
|
|
|
|
/// Result of comparing what the client claimed against what the server derived.
|
|
///
|
|
/// A mismatch is **not** silently reconciled in either direction: the server's
|
|
/// value stands as canonical and the disagreement is reported so the rules
|
|
/// model can be investigated against the exact squad that produced it.
|
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
|
pub struct EvaluationComparison {
|
|
pub field: String,
|
|
pub client: i64,
|
|
pub server: i64,
|
|
}
|
|
|
|
impl ClientReportedEvaluation {
|
|
/// Fields where the client and the server disagree. Empty means agreement
|
|
/// on every field the client actually sent.
|
|
pub fn compare(&self, server: &SquadEvaluation) -> Vec<EvaluationComparison> {
|
|
let mut out = Vec::new();
|
|
let mut check = |field: &str, client: Option<i64>, srv: i64| {
|
|
if let Some(c) = client {
|
|
if c != srv {
|
|
out.push(EvaluationComparison {
|
|
field: field.to_string(),
|
|
client: c,
|
|
server: srv,
|
|
});
|
|
}
|
|
}
|
|
};
|
|
check(
|
|
"chemistry",
|
|
self.client_reported_chemistry,
|
|
server.chemistry,
|
|
);
|
|
check("rating", self.client_reported_rating, server.rating);
|
|
check(
|
|
"star_rating",
|
|
self.client_reported_star_rating,
|
|
server.star_rating,
|
|
);
|
|
out
|
|
}
|
|
}
|
|
|
|
/// How a specific game evaluates a squad.
|
|
pub trait SquadRules: Send + Sync {
|
|
/// Stable identifier recorded in [`SquadEvaluation::rules`].
|
|
fn name(&self) -> &'static str;
|
|
fn evaluate(&self, snapshot: &SquadSnapshot) -> SquadEvaluation;
|
|
}
|
|
|
|
/// OpenFUT's own rules — the implementation that already lived in Core.
|
|
///
|
|
/// Moved here unchanged in behaviour rather than rewritten: it is the default
|
|
/// for clients that have no game-specific rules, and changing its numbers while
|
|
/// relocating it would have made the move unreviewable.
|
|
///
|
|
/// Link scoring: club +3 each (max 6), league +1 each (max 4), nation +1 each
|
|
/// (max 3), per player capped at 10, team total capped at 100.
|
|
pub struct DefaultSquadRules;
|
|
|
|
impl SquadRules for DefaultSquadRules {
|
|
fn name(&self) -> &'static str {
|
|
"openfut-default-v2"
|
|
}
|
|
|
|
fn evaluate(&self, snapshot: &SquadSnapshot) -> SquadEvaluation {
|
|
let starters: Vec<&SquadPlayerCard> = snapshot.starters().collect();
|
|
|
|
let mut players = Vec::with_capacity(starters.len());
|
|
let mut total: i64 = 0;
|
|
|
|
for (i, p) in starters.iter().enumerate() {
|
|
let count = |f: fn(&SquadPlayerCard) -> &String, v: &String| {
|
|
starters
|
|
.iter()
|
|
.enumerate()
|
|
.filter(|(j, o)| *j != i && f(o) == v)
|
|
.count() as i64
|
|
};
|
|
let club_links = count(|c| &c.club, &p.club);
|
|
let league_links = count(|c| &c.league, &p.league);
|
|
let nation_links = count(|c| &c.nation, &p.nation);
|
|
|
|
let club_pts = (club_links * 3).min(6);
|
|
let league_pts = league_links.min(4);
|
|
let nation_pts = nation_links.min(3);
|
|
let chem = (club_pts + league_pts + nation_pts).min(10);
|
|
total += chem;
|
|
|
|
players.push(PlayerEvaluation {
|
|
owned_card_id: p.owned_card_id.clone(),
|
|
chemistry: chem,
|
|
detail: vec![
|
|
("club_links".into(), club_links),
|
|
("league_links".into(), league_links),
|
|
("nation_links".into(), nation_links),
|
|
("club_pts".into(), club_pts),
|
|
("league_pts".into(), league_pts),
|
|
("nation_pts".into(), nation_pts),
|
|
],
|
|
});
|
|
}
|
|
|
|
// Mean overall of the starters, rounded down. Empty squad rates 0
|
|
// rather than dividing by zero.
|
|
let rating = if starters.is_empty() {
|
|
0
|
|
} else {
|
|
starters.iter().map(|p| p.overall as i64).sum::<i64>() / starters.len() as i64
|
|
};
|
|
|
|
SquadEvaluation {
|
|
chemistry: total.min(100),
|
|
chemistry_max: 100,
|
|
rating,
|
|
// 0-5 from the rating band. Coarse on purpose: this is OpenFUT's
|
|
// own presentation value, not a reconstruction of any game's.
|
|
star_rating: match rating {
|
|
0 => 0,
|
|
1..=64 => 1,
|
|
65..=74 => 2,
|
|
75..=81 => 3,
|
|
82..=87 => 4,
|
|
_ => 5,
|
|
},
|
|
players,
|
|
rules: self.name().to_string(),
|
|
}
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
fn p(slot: i64, club: &str, league: &str, nation: &str, overall: u8) -> SquadPlayerCard {
|
|
SquadPlayerCard {
|
|
owned_card_id: format!("owned-{slot}"),
|
|
card_id: format!("card-{slot}"),
|
|
name: format!("P{slot}"),
|
|
overall,
|
|
position: "ST".into(),
|
|
nation: nation.into(),
|
|
league: league.into(),
|
|
club: club.into(),
|
|
slot,
|
|
on_bench: false,
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn an_empty_squad_evaluates_without_dividing_by_zero() {
|
|
let e = DefaultSquadRules.evaluate(&SquadSnapshot::default());
|
|
assert_eq!(e.chemistry, 0);
|
|
assert_eq!(e.rating, 0);
|
|
assert_eq!(e.star_rating, 0);
|
|
assert!(e.players.is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn bench_players_do_not_contribute() {
|
|
let mut snap = SquadSnapshot {
|
|
formation: "4-4-2".into(),
|
|
players: vec![p(0, "A", "L", "N", 80), p(1, "A", "L", "N", 80)],
|
|
};
|
|
let with_both = DefaultSquadRules.evaluate(&snap);
|
|
snap.players[1].on_bench = true;
|
|
let with_bench = DefaultSquadRules.evaluate(&snap);
|
|
assert!(
|
|
with_bench.chemistry < with_both.chemistry,
|
|
"a benched team-mate must not create links: {with_bench:?}"
|
|
);
|
|
assert_eq!(with_bench.players.len(), 1);
|
|
}
|
|
|
|
#[test]
|
|
fn links_are_capped_per_category_and_per_player() {
|
|
// Eleven identical players: club links alone would be 30 pts uncapped.
|
|
let players: Vec<_> = (0..11).map(|i| p(i, "A", "L", "N", 90)).collect();
|
|
let e = DefaultSquadRules.evaluate(&SquadSnapshot {
|
|
formation: "4-4-2".into(),
|
|
players,
|
|
});
|
|
for pe in &e.players {
|
|
assert_eq!(pe.chemistry, 10, "per-player cap is 10: {pe:?}");
|
|
}
|
|
assert_eq!(e.chemistry, 100);
|
|
assert_eq!(e.chemistry_max, 100);
|
|
}
|
|
|
|
#[test]
|
|
fn team_chemistry_is_capped_at_the_maximum() {
|
|
// 15 starters would total 150 uncapped.
|
|
let players: Vec<_> = (0..15).map(|i| p(i, "A", "L", "N", 90)).collect();
|
|
let e = DefaultSquadRules.evaluate(&SquadSnapshot {
|
|
formation: "x".into(),
|
|
players,
|
|
});
|
|
assert_eq!(e.chemistry, 100);
|
|
}
|
|
|
|
#[test]
|
|
fn unrelated_players_earn_no_chemistry() {
|
|
let players = vec![
|
|
p(0, "A", "L1", "N1", 80),
|
|
p(1, "B", "L2", "N2", 80),
|
|
p(2, "C", "L3", "N3", 80),
|
|
];
|
|
let e = DefaultSquadRules.evaluate(&SquadSnapshot {
|
|
formation: "x".into(),
|
|
players,
|
|
});
|
|
assert_eq!(e.chemistry, 0);
|
|
assert_eq!(e.rating, 80);
|
|
}
|
|
|
|
#[test]
|
|
fn the_evaluation_names_the_rules_that_produced_it() {
|
|
let e = DefaultSquadRules.evaluate(&SquadSnapshot::default());
|
|
assert_eq!(e.rules, "openfut-default-v2");
|
|
assert_eq!(DefaultSquadRules.name(), "openfut-default-v2");
|
|
}
|
|
|
|
/// The comparison must report disagreement rather than reconcile it.
|
|
#[test]
|
|
fn a_client_that_disagrees_is_reported_not_reconciled() {
|
|
let server = DefaultSquadRules.evaluate(&SquadSnapshot {
|
|
formation: "x".into(),
|
|
players: vec![p(0, "A", "L", "N", 80)],
|
|
});
|
|
let claimed = ClientReportedEvaluation {
|
|
client_reported_chemistry: Some(52),
|
|
client_reported_rating: Some(server.rating),
|
|
client_reported_star_rating: None,
|
|
};
|
|
let diff = claimed.compare(&server);
|
|
assert_eq!(diff.len(), 1, "{diff:?}");
|
|
assert_eq!(diff[0].field, "chemistry");
|
|
assert_eq!(diff[0].client, 52);
|
|
assert_eq!(diff[0].server, server.chemistry);
|
|
// And the server's own value is untouched by the comparison.
|
|
assert_eq!(server.chemistry, 0);
|
|
}
|
|
|
|
/// A field the client did not send cannot disagree.
|
|
#[test]
|
|
fn absent_client_fields_are_not_treated_as_zero() {
|
|
let server = DefaultSquadRules.evaluate(&SquadSnapshot {
|
|
formation: "x".into(),
|
|
players: vec![p(0, "A", "L", "N", 80)],
|
|
});
|
|
assert!(ClientReportedEvaluation::default()
|
|
.compare(&server)
|
|
.is_empty());
|
|
}
|
|
|
|
#[test]
|
|
fn agreement_reports_nothing() {
|
|
let server = DefaultSquadRules.evaluate(&SquadSnapshot {
|
|
formation: "x".into(),
|
|
players: vec![p(0, "A", "L", "N", 80)],
|
|
});
|
|
let claimed = ClientReportedEvaluation {
|
|
client_reported_chemistry: Some(server.chemistry),
|
|
client_reported_rating: Some(server.rating),
|
|
client_reported_star_rating: Some(server.star_rating),
|
|
};
|
|
assert!(claimed.compare(&server).is_empty());
|
|
}
|
|
}
|