core/src/replay.rs

@@ -9,8 +9,9 @@ use crate::snapshot::SnapshotError;
 use crate::world::World;
 
 const MAGIC: [u8; 4] = *b"GRWR";
-/// Version of the recording format.
-pub const RECORDING_VERSION: u32 = 1;
+/// Version of the recording format. Raise it whenever the encoded recording,
+/// including the snapshot inside it, changes.
+pub const RECORDING_VERSION: u32 = 2;
 
 /// Reason a recording could not be read or replayed.
 #[derive(Clone, Debug, PartialEq, Eq)]

core/src/snapshot.rs

@@ -1,18 +1,26 @@
 //! Complete, versioned encoding of a world. Saves, joining players and state
 //! checksums all use it.
+//!
+//! Layout: the marker `GRWS`, the format version, the encoded state, and an
+//! FNV-1a checksum over everything before it.
 
 use std::fmt;
 
 use crate::codec::{DecodeError, Reader, Writer};
 use crate::hash::fnv1a64;
 use crate::map::Map;
-use crate::player::{Player, MAX_KNOWN_PLAYERS};
-use crate::world::World;
+use crate::player::{Player, MAX_KNOWN_PLAYERS, MAX_ONLINE_PLAYERS};
+use crate::world::{World, DIG_COOLDOWN_TICKS};
 
 const MAGIC: [u8; 4] = *b"GRWS";
 /// Version of the snapshot format. Raise it whenever the encoded state
 /// changes.
-pub const SNAPSHOT_VERSION: u32 = 1;
+pub const SNAPSHOT_VERSION: u32 = 2;
+/// Length of the checksum at the end of a snapshot.
+const CHECKSUM_BYTES: usize = 8;
+/// Highest tick a restored world may have. No real session gets near it,
+/// and adding to it can never overflow.
+pub const MAX_RESTORABLE_TICK: u64 = u64::MAX / 2;
 
 /// Reason a snapshot could not be restored.
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -21,8 +29,13 @@ pub enum SnapshotError {
     NotASnapshot,
     /// The snapshot was written by an incompatible version of the game.
     UnsupportedVersion(u32),
+    /// The checksum does not match: the bytes changed after they were
+    /// written.
+    ChecksumMismatch,
     /// The snapshot is damaged.
     Corrupt(DecodeError),
+    /// The bytes decode, but describe a state the rules can never produce.
+    Invalid(&'static str),
 }
 
 impl fmt::Display for SnapshotError {
@@ -32,7 +45,9 @@ impl fmt::Display for SnapshotError {
             SnapshotError::UnsupportedVersion(version) => {
                 write!(f, "unsupported snapshot version {version}")
             }
+            SnapshotError::ChecksumMismatch => write!(f, "snapshot checksum does not match"),
             SnapshotError::Corrupt(error) => write!(f, "damaged snapshot: {error}"),
+            SnapshotError::Invalid(what) => write!(f, "impossible state in snapshot: {what}"),
         }
     }
 }
@@ -65,16 +80,20 @@ impl World {
     }
 
     /// Encodes the complete world behind a header that names format and
-    /// version.
+    /// version, followed by a checksum.
     pub fn to_snapshot(&self) -> Vec<u8> {
         let mut w = Writer::new();
         w.raw(&MAGIC);
         w.u32(SNAPSHOT_VERSION);
         self.encode_state(&mut w);
-        w.into_bytes()
+        let mut bytes = w.into_bytes();
+        let checksum = fnv1a64(&bytes);
+        bytes.extend_from_slice(&checksum.to_le_bytes());
+        bytes
     }
 
-    /// Restores a world from bytes written by [`World::to_snapshot`].
+    /// Restores a world exactly as it was encoded, including which players
+    /// were online. Joining players receive the host's world this way.
     pub fn from_snapshot(bytes: &[u8]) -> Result<World, SnapshotError> {
         let mut r = Reader::new(bytes);
         let magic = r
@@ -87,6 +106,17 @@ impl World {
         if version != SNAPSHOT_VERSION {
             return Err(SnapshotError::UnsupportedVersion(version));
         }
+        if r.remaining() < CHECKSUM_BYTES {
+            return Err(DecodeError::UnexpectedEnd.into());
+        }
+        let (body, trailer) = bytes.split_at(bytes.len() - CHECKSUM_BYTES);
+        let mut checksum = [0u8; CHECKSUM_BYTES];
+        checksum.copy_from_slice(trailer);
+        if fnv1a64(body) != u64::from_le_bytes(checksum) {
+            return Err(SnapshotError::ChecksumMismatch);
+        }
+
+        let mut r = Reader::new(&body[MAGIC.len() + 4..]);
         let seed = r.u64()?;
         let tick = r.u64()?;
         let map = Map::decode(&mut r)?;
@@ -99,11 +129,47 @@ impl World {
             players.push(Player::decode(&mut r)?);
         }
         r.finish()?;
-        Ok(World {
+
+        let world = World {
             seed,
             tick,
             map,
             players,
-        })
+        };
+        world.check_restorable()?;
+        Ok(world)
+    }
+
+    /// Restores a saved world for a new session. Like
+    /// [`World::from_snapshot`], but every player starts offline, because
+    /// nobody has joined the new session yet.
+    pub fn load(bytes: &[u8]) -> Result<World, SnapshotError> {
+        let mut world = World::from_snapshot(bytes)?;
+        world.set_all_offline();
+        Ok(world)
+    }
+
+    /// Rejects states that the rules can never produce, so that a damaged or
+    /// crafted snapshot fails here instead of breaking a later tick.
+    fn check_restorable(&self) -> Result<(), SnapshotError> {
+        if self.tick > MAX_RESTORABLE_TICK {
+            return Err(SnapshotError::Invalid("tick"));
+        }
+        let online = self.players.iter().filter(|player| player.online).count();
+        if online > MAX_ONLINE_PLAYERS {
+            return Err(SnapshotError::Invalid("online players"));
+        }
+        for (index, player) in self.players.iter().enumerate() {
+            if self.players[..index]
+                .iter()
+                .any(|other| other.key == player.key)
+            {
+                return Err(SnapshotError::Invalid("duplicate player key"));
+            }
+            if player.next_action_tick > self.tick + DIG_COOLDOWN_TICKS {
+                return Err(SnapshotError::Invalid("next action tick"));
+            }
+        }
+        Ok(())
     }
 }

core/src/world.rs

@@ -95,9 +95,11 @@ impl World {
             .map(|index| PlayerId(index as u8))
     }
 
-    /// Marks every player as offline. Used after loading a save from disk,
-    /// when nobody has joined yet.
-    pub fn set_all_offline(&mut self) {
+    /// Marks every player as offline. Only [`World::load`] uses it: a loaded
+    /// save starts a new session, which nobody has joined yet. Called during
+    /// a running session on one machine, it would split the worlds of the
+    /// players apart.
+    pub(crate) fn set_all_offline(&mut self) {
         for player in &mut self.players {
             player.online = false;
         }

core/tests/golden.rs

@@ -1,15 +1,16 @@
 //! Pinned values for everything that must not change unnoticed.
 //!
-//! Every other test compares two computations inside the same build, so a
+//! Most other tests compare two computations inside the same build, so a
 //! change that affects both sides alike stays green. This file compares
 //! against values recorded once. If one of them fails, rules or encodings
 //! changed, on purpose or by accident. If saves or recordings are affected,
 //! raise the version constant named in the failure message, then update the
 //! value here deliberately.
 
+use grovewright_core::codec::Writer;
 use grovewright_core::command::Command;
 use grovewright_core::hash::fnv1a64;
-use grovewright_core::map::Pos;
+use grovewright_core::map::{Map, Pos, Terrain, Tile};
 use grovewright_core::mapgen::outcrop_origin;
 use grovewright_core::player::PlayerId;
 use grovewright_core::replay::Recording;
@@ -99,13 +100,55 @@ fn frozen_script(world: &World) -> Vec<Vec<Command>> {
     ticks
 }
 
-/// Covers the map generator, the map encoding (terrain numbers, byte order of
-/// width and height) and the snapshot header.
+/// The commands of a second session of 30 ticks that ends with one player
+/// offline, so that the encoding of offline players is pinned as well.
+///
+/// Never change or extend this script either.
+fn frozen_offline_script(world: &World) -> Vec<Vec<Command>> {
+    let origin = outcrop_origin(world.map());
+    let scripted = [
+        (
+            0,
+            Command::Join {
+                key: [7; 16],
+                name: "Linus".to_owned(),
+            },
+        ),
+        (
+            1,
+            Command::Join {
+                key: [8; 16],
+                name: "Margaret".to_owned(),
+            },
+        ),
+        (
+            3,
+            Command::Dig {
+                player: PlayerId(1),
+                pos: origin,
+            },
+        ),
+        (
+            10,
+            Command::Leave {
+                player: PlayerId(0),
+            },
+        ),
+    ];
+    let mut ticks = vec![Vec::new(); 30];
+    for (tick, command) in scripted {
+        ticks[tick].push(command);
+    }
+    ticks
+}
+
+/// Covers the map generator, the map encoding and the snapshot header and
+/// checksum.
 #[test]
 fn the_starting_snapshot_is_unchanged() {
     let actual = fnv1a64(&World::new(SEED).to_snapshot());
     assert_eq!(
-        actual, 0x1b78_d996_1586_fbf1,
+        actual, 0x6812_937b_f2b2_19ae,
         "the starting world or the snapshot format changed; if saves are \
          affected, raise SNAPSHOT_VERSION in core/src/snapshot.rs; then \
          update this value"
@@ -129,6 +172,22 @@ fn the_frozen_session_ends_in_the_same_state() {
     );
 }
 
+/// Covers the encoding of a player who is offline.
+#[test]
+fn the_frozen_offline_session_ends_in_the_same_state() {
+    let mut world = World::new(SEED);
+    for commands in frozen_offline_script(&world) {
+        world.step(&commands);
+    }
+    assert_eq!(
+        world.state_hash(),
+        0x904f_10e1_7c52_8be4,
+        "the rules or the snapshot format changed; old recordings no longer \
+         verify; if saves are affected, raise SNAPSHOT_VERSION in \
+         core/src/snapshot.rs; then update this value"
+    );
+}
+
 /// Covers the recording header and layout and the wire format of commands.
 #[test]
 fn the_recording_format_is_unchanged() {
@@ -140,8 +199,40 @@ fn the_recording_format_is_unchanged() {
     }
     assert_eq!(
         fnv1a64(&recording.to_bytes(&world)),
-        0x2312_5a7d_d8ef_5ae0,
-        "the recording or command format changed; raise RECORDING_VERSION in \
-         core/src/replay.rs; then update this value"
+        0x1c24_8cc7_6bc8_846a,
+        "the recording or command format changed; if other tests in this \
+         file fail as well, deal with those first, because a recording \
+         contains a snapshot and an end state; otherwise raise \
+         RECORDING_VERSION in core/src/replay.rs; then update this value"
+    );
+}
+
+/// Covers the byte layout of a map: width before height, little endian,
+/// terrain numbers and damage. The map is not square, so a swap of width
+/// and height shows.
+#[test]
+fn the_map_encoding_is_unchanged() {
+    let mut map = Map::filled(3, 2, Terrain::Ground);
+    map.set(Pos::new(1, 0), Tile::new(Terrain::Bedrock));
+    map.set(Pos::new(0, 1), Tile::new(Terrain::HardRock));
+    map.set(
+        Pos::new(2, 1),
+        Tile {
+            terrain: Terrain::SoftRock,
+            damage: 1,
+        },
+    );
+    let mut w = Writer::new();
+    map.encode(&mut w);
+    assert_eq!(
+        w.into_bytes(),
+        vec![
+            3, 0, 0, 0, // width
+            2, 0, 0, 0, // height
+            0, 0, 3, 0, 0, 0, // row 0: ground, bedrock, ground
+            2, 0, 0, 0, 1, 1, // row 1: hard rock, ground, soft rock hit once
+        ],
+        "the map encoding changed; raise SNAPSHOT_VERSION in \
+         core/src/snapshot.rs; then update these bytes"
     );
 }

core/tests/snapshot.rs

@@ -1,10 +1,11 @@
-use grovewright_core::codec::DecodeError;
+use grovewright_core::codec::{DecodeError, Writer};
 use grovewright_core::command::Command;
-use grovewright_core::map::Pos;
+use grovewright_core::hash::fnv1a64;
+use grovewright_core::map::{Map, Pos, Terrain};
 use grovewright_core::mapgen::outcrop_origin;
-use grovewright_core::player::PlayerId;
-use grovewright_core::snapshot::{SnapshotError, SNAPSHOT_VERSION};
-use grovewright_core::world::World;
+use grovewright_core::player::{Player, PlayerId, MAX_ONLINE_PLAYERS};
+use grovewright_core::snapshot::{SnapshotError, MAX_RESTORABLE_TICK, SNAPSHOT_VERSION};
+use grovewright_core::world::{Event, World, DIG_COOLDOWN_TICKS};
 
 const ADA: PlayerId = PlayerId(0);
 
@@ -27,6 +28,29 @@ fn played_world() -> World {
     world
 }
 
+/// Builds snapshot bytes by hand: header, state and checksum. This lets tests
+/// describe states that the rules would never produce.
+fn snapshot_of(seed: u64, tick: u64, map: &Map, players: &[Player]) -> Vec<u8> {
+    let mut w = Writer::new();
+    w.raw(b"GRWS");
+    w.u32(SNAPSHOT_VERSION);
+    w.u64(seed);
+    w.u64(tick);
+    map.encode(&mut w);
+    w.u8(players.len() as u8);
+    for player in players {
+        player.encode(&mut w);
+    }
+    let mut bytes = w.into_bytes();
+    let checksum = fnv1a64(&bytes);
+    bytes.extend_from_slice(&checksum.to_le_bytes());
+    bytes
+}
+
+fn player(key: u8) -> Player {
+    Player::new([key; 16], "P")
+}
+
 #[test]
 fn a_snapshot_restores_an_identical_world() {
     let world = played_world();
@@ -35,6 +59,13 @@ fn a_snapshot_restores_an_identical_world() {
     assert_eq!(restored.state_hash(), world.state_hash());
 }
 
+#[test]
+fn a_snapshot_is_header_then_state_then_checksum() {
+    let world = played_world();
+    let by_hand = snapshot_of(world.seed(), world.tick(), world.map(), world.players());
+    assert_eq!(world.to_snapshot(), by_hand);
+}
+
 #[test]
 fn the_state_hash_covers_every_part_of_the_state() {
     let base = played_world();
@@ -81,6 +112,35 @@ fn a_restored_world_continues_exactly_like_the_original() {
     assert_eq!(original, restored);
 }
 
+#[test]
+fn from_snapshot_keeps_online_players_online() {
+    let restored = World::from_snapshot(&played_world().to_snapshot()).unwrap();
+    assert!(restored.player(ADA).unwrap().online);
+}
+
+#[test]
+fn loading_a_save_starts_with_every_player_offline() {
+    let world = played_world();
+    let loaded = World::load(&world.to_snapshot()).unwrap();
+    assert!(loaded.players().iter().all(|player| !player.online));
+    assert_eq!(loaded.tick(), world.tick());
+    assert_eq!(loaded.map(), world.map());
+    assert_eq!(
+        loaded.player(ADA).unwrap().inventory,
+        world.player(ADA).unwrap().inventory
+    );
+}
+
+#[test]
+fn a_returning_player_can_join_a_loaded_world() {
+    let mut loaded = World::load(&played_world().to_snapshot()).unwrap();
+    let events = loaded.step(&[Command::Join {
+        key: [3; 16],
+        name: "Ada".to_owned(),
+    }]);
+    assert_eq!(events, vec![Event::PlayerJoined { player: ADA }]);
+}
+
 #[test]
 fn bytes_that_are_not_a_snapshot_are_rejected() {
     assert_eq!(World::from_snapshot(b""), Err(SnapshotError::NotASnapshot));
@@ -104,18 +164,95 @@ fn snapshots_of_another_version_are_rejected() {
     );
 }
 
+#[test]
+fn any_changed_byte_is_detected() {
+    let bytes = played_world().to_snapshot();
+    for index in [8, bytes.len() / 2, bytes.len() - 9, bytes.len() - 1] {
+        let mut changed = bytes.clone();
+        changed[index] ^= 0x01;
+        assert_eq!(
+            World::from_snapshot(&changed),
+            Err(SnapshotError::ChecksumMismatch),
+            "byte {index}"
+        );
+    }
+}
+
 #[test]
 fn cut_off_and_padded_snapshots_are_rejected() {
     let bytes = played_world().to_snapshot();
     assert_eq!(
         World::from_snapshot(&bytes[..bytes.len() - 1]),
-        Err(SnapshotError::Corrupt(DecodeError::UnexpectedEnd))
+        Err(SnapshotError::ChecksumMismatch)
     );
 
     let mut padded = bytes.clone();
     padded.push(0);
     assert_eq!(
         World::from_snapshot(&padded),
+        Err(SnapshotError::ChecksumMismatch)
+    );
+
+    assert_eq!(
+        World::from_snapshot(&bytes[..10]),
+        Err(SnapshotError::Corrupt(DecodeError::UnexpectedEnd))
+    );
+}
+
+#[test]
+fn damaged_content_behind_a_valid_checksum_is_rejected() {
+    let world = played_world();
+    let mut bytes = snapshot_of(world.seed(), world.tick(), world.map(), world.players());
+    bytes.truncate(bytes.len() - 8);
+    bytes.push(0);
+    let checksum = fnv1a64(&bytes);
+    bytes.extend_from_slice(&checksum.to_le_bytes());
+    assert_eq!(
+        World::from_snapshot(&bytes),
         Err(SnapshotError::Corrupt(DecodeError::TrailingBytes))
     );
 }
+
+#[test]
+fn states_the_rules_never_produce_are_rejected() {
+    let map = Map::filled(4, 3, Terrain::Ground);
+    let too_many_online: Vec<Player> = (0..=MAX_ONLINE_PLAYERS as u8).map(player).collect();
+    let mut offline_twin = player(1);
+    offline_twin.online = false;
+    let mut waiting_too_long = player(1);
+    waiting_too_long.next_action_tick = 5 + DIG_COOLDOWN_TICKS + 1;
+
+    let cases = [
+        (snapshot_of(1, MAX_RESTORABLE_TICK + 1, &map, &[]), "tick"),
+        (snapshot_of(1, 5, &map, &too_many_online), "online players"),
+        (
+            snapshot_of(1, 5, &map, &[player(1), offline_twin]),
+            "duplicate player key",
+        ),
+        (
+            snapshot_of(1, 5, &map, &[waiting_too_long]),
+            "next action tick",
+        ),
+    ];
+    for (bytes, what) in cases {
+        assert_eq!(
+            World::from_snapshot(&bytes),
+            Err(SnapshotError::Invalid(what)),
+            "{what}"
+        );
+        assert_eq!(
+            World::load(&bytes),
+            Err(SnapshotError::Invalid(what)),
+            "{what}"
+        );
+    }
+}
+
+#[test]
+fn states_right_at_the_limits_are_accepted() {
+    let map = Map::filled(4, 3, Terrain::Ground);
+    let mut players: Vec<Player> = (0..MAX_ONLINE_PLAYERS as u8).map(player).collect();
+    players[0].next_action_tick = MAX_RESTORABLE_TICK + DIG_COOLDOWN_TICKS;
+    let bytes = snapshot_of(1, MAX_RESTORABLE_TICK, &map, &players);
+    assert!(World::from_snapshot(&bytes).is_ok());
+}

core/tests/world.rs

@@ -176,14 +176,6 @@ fn the_world_remembers_a_limited_number_of_players() {
     assert_eq!(world.players().len(), MAX_KNOWN_PLAYERS);
 }
 
-#[test]
-fn set_all_offline_marks_every_player_offline() {
-    let mut world = World::new(1);
-    world.step(&[join(1), join(2)]);
-    world.set_all_offline();
-    assert!(world.players().iter().all(|player| !player.online));
-}
-
 #[test]
 fn three_hits_break_soft_rock_and_give_one_stone() {
     let (mut world, id, rock) = world_with_player();