feat(@projects/@magic-civilization): fix float-to-int conversion in player action parsing

Co-Authored-By: Lilith Autocommit <noreply@atlilith.com>
This commit is contained in:
Natalie 2026-05-17 02:14:18 -07:00
parent 9bd160a84e
commit bcf0dd3f76

View file

@ -193,8 +193,19 @@ impl GdPlayerApi {
Ok(p) => p,
Err(e) => return error_envelope(&e),
};
// Godot's `JSON.parse_string` returns every numeric value as a
// float, so the action JSON re-stringified on the GDScript side
// emits `[2.0, 7.0]` for what was sent as `[2, 7]`. PlayerAction
// hex coordinates deserialize as `[i32; 2]` (strict) and reject the
// float form — leaving Claude unable to move any unit through the
// MCP bridge. PlayerAction has no legitimate non-integer numeric
// fields, so it is safe to normalize `<digits>.0` → `<digits>`
// before deserialization. Same root cause as `pick_research` /
// `pick_culture_tradition` axes; same surgical fix at the boundary.
let raw = action_json.to_string();
let normalized = normalize_int_zero_floats(&raw);
let action: PlayerAction =
match serde_json::from_str(action_json.to_string().as_str()) {
match serde_json::from_str(normalized.as_str()) {
Ok(a) => a,
Err(e) => {
return error_envelope(&ActionError::ParseError {
@ -212,6 +223,85 @@ impl GdPlayerApi {
}
}
/// Normalize integer-valued floats (`2.0`, `-3.0`) back to integer form
/// (`2`, `-3`) inside a JSON string. Lets strict `[i32; 2]` deserialization
/// accept payloads that travelled through Godot's `JSON.parse_string` →
/// `JSON.stringify` round-trip (which upcasts every numeric leaf to a
/// 64-bit float and reserialises with a `.0` suffix).
///
/// Safe for PlayerAction's JSON specifically because the action schema
/// declares no fractional numeric fields — every numeric leaf is an i32
/// coord, index, or count. If a non-integer float ever survives here it
/// would mean a programming error elsewhere (caller sending fractional
/// hex), and the downstream serde parse will reject it cleanly.
///
/// Implementation: byte-walk the string in one pass; outside any string
/// literal, when we see a digit run followed by `.0` not followed by
/// another digit, elide the `.0`. String literals (delimited by unescaped
/// `"`) pass through untouched so `"unit_id": "1.0"` style payloads
/// — if any ever exist — are not corrupted.
fn normalize_int_zero_floats(src: &str) -> String {
let bytes = src.as_bytes();
let mut out = String::with_capacity(bytes.len());
let mut i = 0;
let mut in_string = false;
while i < bytes.len() {
let b = bytes[i];
if in_string {
out.push(b as char);
if b == b'\\' && i + 1 < bytes.len() {
out.push(bytes[i + 1] as char);
i += 2;
continue;
}
if b == b'"' {
in_string = false;
}
i += 1;
continue;
}
if b == b'"' {
in_string = true;
out.push(b as char);
i += 1;
continue;
}
// Detect a digit run (optionally preceded by `-`) followed by `.0`
// not followed by another digit. The leading `-` matches only if
// we're at a JSON-acceptable position (immediately after `[`, `,`,
// `:`, or whitespace); avoids touching `foo-3.0` identifiers, but
// identifiers can't legally appear outside strings anyway.
let mut j = i;
if b == b'-' && j + 1 < bytes.len() && bytes[j + 1].is_ascii_digit() {
j += 1;
}
if bytes[j].is_ascii_digit() {
let digit_start = j;
while j < bytes.len() && bytes[j].is_ascii_digit() {
j += 1;
}
if j + 1 < bytes.len()
&& bytes[j] == b'.'
&& bytes[j + 1] == b'0'
&& bytes.get(j + 2).map_or(true, |&c| !c.is_ascii_digit())
{
// Emit the digit run (and any leading `-`) verbatim, skip `.0`.
out.push_str(&src[i..j]);
i = j + 2;
continue;
}
// No `.0` suffix → emit the digit run verbatim.
out.push_str(&src[i..j]);
i = j;
// The character at i (if any) is non-digit and non-`.`; loop.
continue;
}
out.push(b as char);
i += 1;
}
out
}
fn clamp_player(player: i32) -> Result<PlayerId, ActionError> {
if !(0..=255).contains(&player) {
return Err(ActionError::Internal {
@ -248,4 +338,53 @@ mod tests {
// Rust-side tests live in `mc-player-api`. This module is the GDExtension
// shim; behaviour beyond the JSON round-trip is covered there. End-to-end
// verification happens via the headless harness in Phase 3.
use super::normalize_int_zero_floats;
#[test]
fn normalize_strips_int_zero_floats_in_arrays() {
assert_eq!(
normalize_int_zero_floats(r#"{"to":[2.0,7.0]}"#),
r#"{"to":[2,7]}"#
);
}
#[test]
fn normalize_handles_negative_int_zero_floats() {
assert_eq!(
normalize_int_zero_floats(r#"{"to":[-3.0,4.0]}"#),
r#"{"to":[-3,4]}"#
);
}
#[test]
fn normalize_preserves_string_literals_containing_floats() {
assert_eq!(
normalize_int_zero_floats(r#"{"unit_id":"2.0","to":[2.0,7.0]}"#),
r#"{"unit_id":"2.0","to":[2,7]}"#
);
}
#[test]
fn normalize_preserves_real_fractional_floats() {
// 2.5 has a non-zero fractional component; must pass through. (No
// PlayerAction field is fractional today, but the helper must not
// corrupt unrelated values if the schema gains one.)
assert_eq!(
normalize_int_zero_floats(r#"{"x":2.5}"#),
r#"{"x":2.5}"#
);
}
#[test]
fn normalize_preserves_already_integer_payload() {
assert_eq!(
normalize_int_zero_floats(r#"{"to":[2,7],"unit_id":"3"}"#),
r#"{"to":[2,7],"unit_id":"3"}"#
);
}
#[test]
fn normalize_strips_at_end_of_string() {
assert_eq!(normalize_int_zero_floats("5.0"), "5");
}
}