From d6da85ec23aa43113ed6c8e18e1d02eef76d41db Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:02:02 -0700 Subject: [PATCH 01/15] fix(particle): cap server particle count per spawn S2A carries an i32 count; a huge value pushed millions of particles in one call before tick trimmed them to MAX_PARTICLES, stalling the main thread or exhausting memory. Clamp the spawn count to MAX_PARTICLES. --- crates/fpsmaster_app/src/particle.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/fpsmaster_app/src/particle.rs b/crates/fpsmaster_app/src/particle.rs index 7449beb..8602bde 100644 --- a/crates/fpsmaster_app/src/particle.rs +++ b/crates/fpsmaster_app/src/particle.rs @@ -110,6 +110,10 @@ pub struct BlockDebris { pub uv_off: [f32; 2], } +/// Cap on live particles (per kind). `tick` trims the oldest beyond it, and +/// `spawn` never creates more than this from one packet. +const MAX_PARTICLES: usize = 4000; + /// All live particles, ticked once per game tick. pub struct ParticleSystem { particles: Vec, @@ -173,7 +177,6 @@ impl ParticleSystem { } self.block_particles.retain(|p| p.age < p.max_age); // Bound the working set so a particle storm can't grow without limit. - const MAX_PARTICLES: usize = 4000; if self.particles.len() > MAX_PARTICLES { let drop = self.particles.len() - MAX_PARTICLES; self.particles.drain(0..drop); @@ -202,7 +205,11 @@ impl ParticleSystem { // handled inside `make_particle` (one particle, no positional jitter). let colour_carrying = matches!(type_id, 23 | 30 | 15 | 16 | 17); let base = if colour_carrying { 1 } else { count.max(1) }; - let n = ((base as f32) * self.density).round() as usize; + // `count` is a server-supplied i32: a hostile or buggy S2A with a huge + // count would otherwise push millions of particles in one call (stall / + // OOM) before `tick` trims to MAX_PARTICLES. Anything past the cap would + // be trimmed anyway, so clamp here. + let n = (((base as f32) * self.density).round() as usize).min(MAX_PARTICLES); for _ in 0..n { let p_pos = if colour_carrying { pos @@ -644,6 +651,16 @@ impl Rng { mod tests { use super::*; + /// A server particle packet with an absurd count is clamped at spawn time + /// instead of pushing millions of particles before `tick` trims them. + #[test] + fn huge_server_count_is_capped_at_spawn() { + let mut sys = ParticleSystem::new(); + sys.set_density(4.0); + sys.spawn(0, Vec3::ZERO, Vec3::ONE, 0.1, i32::MAX, &[]); + assert_eq!(sys.particles.len(), MAX_PARTICLES); + } + /// A flame's render scale shrinks as it ages (vanilla `1 - f*f*0.5`). #[test] fn flame_loses_scale_over_life() { From a87ce50c539e74c226e2bdb812b6b30e8ff13e95 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:02:03 -0700 Subject: [PATCH 02/15] fix(world): discard world on dimension-changing Respawn Vanilla replaces the WorldClient when the dimension changes. Proxies switch servers with a Respawn into another dimension followed by the target one and the old server never sends ChunkUnload, so its chunks and entities stayed in memory and lingered as ghost terrain/entities. Reset the world and all per-world side tables (except the local player) and have the renderer drop every chunk mesh and in-flight mesh job. --- crates/fpsmaster_app/src/game.rs | 106 ++++++++++++++++++++ crates/fpsmaster_app/src/main.rs | 5 + crates/fpsmaster_app/src/particle.rs | 7 ++ crates/fpsmaster_render/src/renderer/mod.rs | 14 +++ 4 files changed, 132 insertions(+) diff --git a/crates/fpsmaster_app/src/game.rs b/crates/fpsmaster_app/src/game.rs index d89f29d..5011256 100644 --- a/crates/fpsmaster_app/src/game.rs +++ b/crates/fpsmaster_app/src/game.rs @@ -799,6 +799,9 @@ pub struct GameState { /// Player chunk at the last `enforce_render_distance` pass, so the scan runs /// only when crossing a chunk boundary. last_residency_chunk: Option, + /// Set when a dimension-changing Respawn discarded the whole world; the host + /// must then drop every GPU chunk mesh (see `take_world_reset`). + world_reset: bool, /// Set for a built-in single-player world: the terrain generator that /// streams chunks around the player (`stream_local_world`). `None` for demo /// worlds and server sessions, which get their chunks elsewhere. @@ -1332,6 +1335,7 @@ impl GameState { urgent_remesh: HashSet::new(), evicted_columns: HashSet::new(), last_residency_chunk: None, + world_reset: false, local_worldgen: None, pending_block_changes: std::collections::HashMap::new(), chest_lid_angles: std::collections::HashMap::new(), @@ -5928,6 +5932,15 @@ impl GameState { } fn handle_respawn(&mut self, dimension: i32, game_mode: u8) -> bool { + // Vanilla builds a fresh WorldClient when the dimension changes. Bungee + // style proxies switch servers by sending a Respawn into another + // dimension and then the target one, and the old server never sends + // ChunkUnload — so without this the previous server's terrain and + // entities stay in memory and linger as ghosts in the new world. + // A same-dimension Respawn (death) keeps the world, as in vanilla. + if dimension as i8 != self.dimension { + self.reset_world(); + } // The server resends chunks plus a fresh PlayerPositionLook at the // respawn point; wait for that position before reporting movement // again so we don't send a stale (death-location) position. @@ -5948,6 +5961,47 @@ impl GameState { false } + /// Discard every chunk, entity (except the local player) and per-world side + /// table, as vanilla does by replacing the WorldClient on a dimension + /// change. Flags `world_reset` so the host also drops all GPU meshes. + fn reset_world(&mut self) { + let mut world = World::new(); + world.set_has_sky_light(self.world.has_sky_light()); + world.upsert_entity(self.player.clone()); + self.world = world; + self.world_reset = true; + self.dirty_chunks.clear(); + self.urgent_remesh.clear(); + self.evicted_columns.clear(); + self.last_residency_chunk = None; + self.pending_block_changes.clear(); + self.entity_uuids.clear(); + self.entity_items.clear(); + self.entity_xp.clear(); + self.falling_blocks.clear(); + self.entity_equipment.clear(); + self.vehicles.clear(); + self.entity_prev_render_pos.clear(); + for s in self.entity_sounds.drain(..) { + self.moving_sound_commands + .push(MovingSoundCommand::Stop { id: s.sound_id }); + } + self.chest_lid_angles.clear(); + self.chest_open_targets.clear(); + self.signs.clear(); + self.skulls.clear(); + self.lightning_bolts.clear(); + self.weather = Weather::default(); + self.particles.clear(); + } + + /// Whether the world was discarded (dimension change) since the last call; + /// the host then drops every GPU chunk mesh in one go instead of trickling + /// thousands of removals through the per-frame dirty budget. + pub fn take_world_reset(&mut self) -> bool { + std::mem::take(&mut self.world_reset) + } + fn handle_player_position_look( &mut self, x: f64, @@ -9258,6 +9312,58 @@ mod interaction_tests { assert_eq!(gs.hud_vitals().absorption, 0.0); } + /// A proxy server switch (Respawn into another dimension, then the target + /// one) discards the previous server's chunks and entities; a same-dimension + /// Respawn (death) keeps them. + #[test] + fn dimension_change_respawn_discards_world() { + let mut gs = GameState::empty_for_server(1.0); + gs.apply_play_packet(ClientboundPlayPacket::JoinGame { + entity_id: 7, + game_mode: 0, + dimension: 0, + difficulty: 0, + max_players: 20, + level_type: "default".to_owned(), + reduced_debug_info: false, + }); + gs.world.set_block(0, 64, 0, BlockState::new(1, 0)); + gs.world.set_block(40, 64, 40, BlockState::new(1, 0)); + gs.apply_play_packet(ClientboundPlayPacket::SpawnPlayer { + entity_id: 8, + uuid: [1u8; 16], + x: 0.5, + y: 65.0, + z: 0.5, + yaw: 0.0, + pitch: 0.0, + }); + gs.pending_block_changes + .insert(ChunkPos::new(9, 9), vec![(144, 64, 144, BlockState::new(1, 0))]); + let respawn = |dimension| ClientboundPlayPacket::Respawn { + dimension, + difficulty: 0, + game_mode: 0, + level_type: "default".to_owned(), + }; + + // Death respawn in the same dimension keeps the world. + gs.apply_play_packet(respawn(0)); + assert_eq!(gs.loaded_chunk_count(), 2); + assert!(!gs.take_world_reset()); + + gs.apply_play_packet(respawn(-1)); + gs.apply_play_packet(respawn(0)); + assert_eq!(gs.loaded_chunk_count(), 0); + assert!(gs.world.entity(EntityId(8)).is_none()); + assert!(gs.world.entity(EntityId(7)).is_some(), "local player survives"); + assert!(gs.entity_uuids.is_empty()); + assert!(gs.pending_block_changes.is_empty()); + assert!(gs.dirty_chunks.is_empty()); + assert!(gs.take_world_reset()); + assert!(!gs.take_world_reset()); + } + #[test] fn respawn_clears_effects_and_resets_max_health() { use fpsmaster_protocol::v1_8_9::packets::EntityProperty; diff --git a/crates/fpsmaster_app/src/main.rs b/crates/fpsmaster_app/src/main.rs index ddc7550..6220392 100644 --- a/crates/fpsmaster_app/src/main.rs +++ b/crates/fpsmaster_app/src/main.rs @@ -2420,6 +2420,11 @@ fn render_frame( // Single-player world streaming: generate the columns newly in range and // drop far ones (a no-op for demo/server sessions). New columns are marked // dirty and meshed through the budget below; dropped ones free their meshes. + // A dimension change discarded the whole world (server switch behind a + // proxy); drop every GPU mesh at once so the old terrain never draws. + if app.game.take_world_reset() { + renderer.clear_chunk_meshes(); + } let stream_removed = app.game.stream_local_world(app.settings.render_distance); if !stream_removed.is_empty() { renderer.drop_chunk_sections(&stream_removed); diff --git a/crates/fpsmaster_app/src/particle.rs b/crates/fpsmaster_app/src/particle.rs index 8602bde..2614a82 100644 --- a/crates/fpsmaster_app/src/particle.rs +++ b/crates/fpsmaster_app/src/particle.rs @@ -136,6 +136,13 @@ impl ParticleSystem { } } + /// Drop every live particle (world change: the old world's effects must + /// not carry over, like vanilla `EffectRenderer.clearEffects`). + pub fn clear(&mut self) { + self.particles.clear(); + self.block_particles.clear(); + } + /// Set the spawn-count multiplier (extension `particleDensity` preset). pub fn set_density(&mut self, density: f32) { self.density = density.clamp(0.0, 4.0); diff --git a/crates/fpsmaster_render/src/renderer/mod.rs b/crates/fpsmaster_render/src/renderer/mod.rs index 089df91..110ca39 100644 --- a/crates/fpsmaster_render/src/renderer/mod.rs +++ b/crates/fpsmaster_render/src/renderer/mod.rs @@ -6967,6 +6967,20 @@ impl Renderer { } } + /// Drop every chunk mesh and forget all in-flight mesh jobs (their results + /// are discarded on arrival) — the world they belonged to was replaced. + pub fn clear_chunk_meshes(&mut self) { + let sections: Vec = self.chunk_sections.iter().copied().collect(); + for pos in sections { + self.remove_section(pos); + } + self.chunk_mesh_generations.clear(); + self.chunk_solid.clear(); + self.chunk_cutout.clear(); + self.chunk_transparent.clear(); + self.chunk_water.clear(); + } + /// Free the GPU meshes of the given sections without touching their block /// data. Drives the client-side render-distance safety net /// ([`GameState::enforce_render_distance`]): sections beyond the view are From 382055dc0c0473f2bcc86fe23c5348d490a7e830 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:03:01 -0700 Subject: [PATCH 03/15] fix(sound): stream background music and bound the effect cache Music went through the static-sound path: each new track was fully decoded to PCM on the main thread (multi-second stall for a 4-minute ogg) and the ~90 MB result was cached forever. Stream music via kira's StreamingSoundData (decoded on its own thread, never cached), skip caching clips longer than ~10 s and flush the effect cache once it exceeds ~64 MB of PCM. --- crates/fpsmaster_app/src/sound.rs | 52 ++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/crates/fpsmaster_app/src/sound.rs b/crates/fpsmaster_app/src/sound.rs index 3d2c4ec..896872e 100644 --- a/crates/fpsmaster_app/src/sound.rs +++ b/crates/fpsmaster_app/src/sound.rs @@ -16,12 +16,21 @@ use std::path::{Path, PathBuf}; use glam::Vec3; use kira::sound::static_sound::StaticSoundData; +use kira::sound::streaming::{StreamingSoundData, StreamingSoundHandle}; +use kira::sound::FromFileError; use kira::{AudioManager, AudioManagerSettings, DefaultBackend, Decibels, Panning, PlaybackRate}; /// Vanilla rolloff: a positioned sound at volume `1.0` is inaudible past this /// many blocks; volume `> 1.0` extends the range proportionally. const ATTENUATION_RANGE: f32 = 16.0; +/// Decoded clips longer than this many frames (~10 s at 44.1 kHz, ~3.5 MB of +/// PCM) are played but not kept in the cache. +const MAX_CACHED_CLIP_FRAMES: usize = 441_000; +/// Total decoded frames the cache may hold (~64 MB of stereo f32 PCM). Past it +/// the cache is flushed and refills on demand, so memory stays bounded. +const MAX_CACHED_TOTAL_FRAMES: usize = 8_000_000; + /// A sound request produced by `game.rs`, drained and played by `main.rs`. /// Carries no audio types so the game layer stays decoupled from the backend. #[derive(Debug, Clone)] @@ -193,8 +202,12 @@ pub struct SoundManager { manager: Option>, /// Event key -> its category and weighted sound variants. events: HashMap, - /// Decoded ogg cache, keyed by sound asset name (e.g. `"dig/stone1"`). + /// Decoded ogg cache for short effects, keyed by sound asset name (e.g. + /// `"dig/stone1"`). Bounded: see `MAX_CACHED_*`. Music never goes here — it + /// is streamed. cache: HashMap>, + /// Total decoded frames currently held in `cache`. + cache_frames: usize, /// Resolved asset roots to read ogg files from (directories or jars). asset_roots: Vec, listener: Listener, @@ -205,7 +218,7 @@ pub struct SoundManager { /// The single background music track, if one is playing (MUSIC category). /// Driven by the music methods, which `main.rs` calls from the `MusicTicker` /// commands each frame. - music: Option, + music: Option>, /// Entity-attached positioned sounds, keyed by a game-assigned id. moving: HashMap, } @@ -228,6 +241,7 @@ impl SoundManager { manager, events, cache: HashMap::new(), + cache_frames: 0, asset_roots, listener: Listener { position: Vec3::ZERO, @@ -330,6 +344,15 @@ impl SoundManager { if data.is_none() { log::debug!("audio: missing sound asset {name}"); } + let frames = data.as_ref().map_or(0, |d| d.frames.len()); + if frames > MAX_CACHED_CLIP_FRAMES { + return data; + } + if self.cache_frames + frames > MAX_CACHED_TOTAL_FRAMES { + self.cache.clear(); + self.cache_frames = 0; + } + self.cache_frames += frames; self.cache.insert(name.to_string(), data.clone()); data } @@ -337,7 +360,9 @@ impl SoundManager { // ─── Music channel ────────────────────────────────────────────────────── /// Start a background music track by its `sounds.json` asset name (e.g. - /// `"music/menu/menu"`), replacing any track already playing. Played + /// `"music/menu/menu"`), replacing any track already playing. Streamed: + /// kira decodes it incrementally off the main thread (a multi-minute track + /// decoded up front stalled the frame and held ~90 MB of PCM). Played /// non-positionally (centered) at the MUSIC category × master volume. The /// handle is kept so [`is_music_playing`](Self::is_music_playing) can poll it /// and [`stop_music`](Self::stop_music) can end it. No-op with no audio @@ -349,7 +374,8 @@ impl SoundManager { if self.manager.is_none() { return; } - let Some(data) = self.sound_data(asset_name) else { + let Some(data) = open_ogg_stream(&self.asset_roots, asset_name) else { + log::debug!("audio: missing music asset {asset_name}"); return; }; let gain = (self.volumes.music * self.volumes.master).clamp(0.0, 1.0); @@ -647,6 +673,24 @@ fn load_ogg(roots: &[PathBuf], name: &str) -> Option { None } +/// Open an ogg for streaming playback by its `sounds.json` asset name. Only the +/// container header is probed here; decoding happens on kira's stream thread. +fn open_ogg_stream(roots: &[PathBuf], name: &str) -> Option> { + let asset = format!("assets/minecraft/sounds/{name}.ogg"); + let stripped = asset.strip_prefix("assets/").unwrap_or(&asset); + for root in roots { + for path in [root.join(&asset), root.join(stripped)] { + if path.is_file() { + match StreamingSoundData::from_file(&path) { + Ok(data) => return Some(data), + Err(err) => log::debug!("audio: cannot stream {}: {err}", path.display()), + } + } + } + } + None +} + /// Read an asset file from a directory root, also tolerating the `assets/` /// prefix being stripped (the texture loader does the same). The 1.8.9 ogg /// sounds ship as loose files (they are never inside the client jar), so a From d6ea858c315707364152f431af804af34fb8c455 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:06:31 -0700 Subject: [PATCH 04/15] fix(net): split reader/writer threads, bound frame sizes, report thread death - The network thread alternated a 10 ms timed read with draining the send queue, so every outbound packet (attacks, flying, transaction acks) waited up to 10 ms on a quiet connection. Split the socket into read and write halves: the writer blocks on the queue and writes immediately, the reader blocks on the socket. Dropping the handle shuts the socket down so both threads exit. - Reject frame lengths and zlib uncompressed lengths outside vanilla's 2097152 limit (negative lengths panicked with_capacity, huge ones grew the buffer or inflated unbounded) and validate list counts in 0x13/0x22/0x26 and team packets against the remaining packet size. - A panic on the reader thread is caught and sent as Disconnected, and pump_network treats a disconnected event channel as a lost connection instead of leaving the player frozen in the world. --- crates/fpsmaster_app/src/main.rs | 10 +- crates/fpsmaster_app/src/network.rs | 143 +++++++++++++----- crates/fpsmaster_protocol/src/codec.rs | 34 ++++- crates/fpsmaster_protocol/src/net.rs | 123 ++++++++++++--- .../fpsmaster_protocol/src/v1_8_9/packets.rs | 48 +++++- .../src/lang/fpsmaster.en_us.lang | 1 + .../src/lang/fpsmaster.zh_cn.lang | 1 + 7 files changed, 285 insertions(+), 75 deletions(-) diff --git a/crates/fpsmaster_app/src/main.rs b/crates/fpsmaster_app/src/main.rs index 6220392..78e0123 100644 --- a/crates/fpsmaster_app/src/main.rs +++ b/crates/fpsmaster_app/src/main.rs @@ -2014,7 +2014,15 @@ fn pump_network(app: &mut App, window: &winit::window::Window, cursor_captured: disconnect = Some(message); break; } - Err(_) => break, + Err(std::sync::mpsc::TryRecvError::Empty) => break, + // Every sender is gone without a Disconnected event: the network + // threads died (a panic). Treat it as a lost connection instead of + // leaving the player frozen in a world that no longer updates. + Err(std::sync::mpsc::TryRecvError::Disconnected) => { + log::error!("network thread exited without a disconnect event"); + disconnect = Some(i18n::tr("fpsmaster.network.threadStopped")); + break; + } } } diff --git a/crates/fpsmaster_app/src/network.rs b/crates/fpsmaster_app/src/network.rs index 17c9e6f..aead64f 100644 --- a/crates/fpsmaster_app/src/network.rs +++ b/crates/fpsmaster_app/src/network.rs @@ -1,12 +1,10 @@ use std::{ sync::mpsc::{self, Receiver, Sender}, thread, - time::Duration, }; use fpsmaster_protocol::{ - io::ProtocolError, - net::BlockingClient, + net::{BlockingClient, ClientReader}, v1_8_9::{ chunk::{decode_chunk_column, ChunkColumnData}, packets::{ClientboundPlayPacket, EntityAction, ServerboundPacket}, @@ -218,35 +216,53 @@ fn network_thread( username: login.username, uuid: login.uuid, }); - let _ = client.set_read_timeout(Some(Duration::from_millis(10))); + // Reads and writes run on separate threads over the same socket: the + // writer blocks on the command queue and puts each packet on the wire the + // moment the main thread queues it, while this thread blocks on reads. (A + // single thread alternating a timed read with the queue delayed every + // outbound packet by up to the read timeout.) Acks/pongs (KeepAlive, + // ConfirmTransaction, teleport confirm) and the flying packet are produced on + // the MAIN thread at tick time, so their order matches vanilla's + // `NetHandlerPlayClient` — what transaction-based anti-cheats key off — and + // the single writer keeps that order on the wire. + let (mut reader, mut writer) = client.into_split(); + let writer_events = events.clone(); + thread::spawn(move || { + while let Ok(NetworkCommand::Send(packet)) = commands.recv() { + log::debug!("sending {}", packet_debug_name(&packet)); + if let Err(err) = writer.write_packet(packet.into_frame()) { + let message = format!("write failed: {err}"); + let _ = writer_events.send(NetworkEvent::Disconnected(message)); + break; + } + } + // The handle was dropped (left the server) or a write failed: close the + // socket so the reader's blocking read returns and that thread ends too. + writer.shutdown(); + }); + + // A panic while decoding a hostile packet must still reach the main thread + // as a disconnect; the writer thread keeps its own sender alive, so the + // channel alone would not show this thread is gone. + let read = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + read_loop(&mut reader, &events) + })); + let message = match read { + Ok(message) => message, + Err(_) => "network thread panicked".to_owned(), + }; + let _ = events.send(NetworkEvent::Disconnected(message)); +} + +/// Forward every incoming packet to the main thread (in receipt order) until the +/// connection fails; returns the failure message. +fn read_loop(reader: &mut ClientReader, events: &Sender) -> String { // Sky light presence is dimension-derived (overworld only); tracked here so // chunks can be decoded on this thread without the game state. let mut has_sky_light = true; - - // This thread is a pure reader/writer: it writes whatever serverbound packets - // the main thread queues (in order) and forwards every incoming packet to the - // main thread (in receipt order). Acks/pongs (KeepAlive, ConfirmTransaction, - // teleport confirm) and the flying packet are produced on the MAIN thread, at - // tick time, so their order matches vanilla's `NetHandlerPlayClient` (which - // runs on the game thread) — what transaction-based anti-cheats key off. loop { - loop { - match commands.try_recv() { - Ok(NetworkCommand::Send(packet)) => { - log::debug!("sending {}", packet_debug_name(&packet)); - if let Err(err) = client.write_packet(packet.into_frame()) { - let _ = - events.send(NetworkEvent::Disconnected(format!("write failed: {err}"))); - return; - } - } - Err(mpsc::TryRecvError::Empty) => break, - Err(mpsc::TryRecvError::Disconnected) => return, - } - } - - match client.read_play_packet_1_8_9() { + match reader.read_play_packet_1_8_9() { Ok(packet) => { // Track dimension-derived sky light (needed to decode chunks on // this thread). The join handshake + every ack now happen on the @@ -260,7 +276,6 @@ fn network_thread( } _ => {} } - // Intercept chunk packets and decode them here, off the render // thread, delivering ready-to-load columns; forward everything // else (in receipt order, so block-change ordering is preserved). @@ -321,11 +336,7 @@ fn network_thread( } } } - Err(ProtocolError::Io(message)) if is_timeout(&message) => {} - Err(err) => { - let _ = events.send(NetworkEvent::Disconnected(format!("read failed: {err}"))); - return; - } + Err(err) => return format!("read failed: {err}"), } } } @@ -351,14 +362,6 @@ pub fn initial_play_packets() -> Vec { ] } -fn is_timeout(message: &str) -> bool { - message.contains("timed out") - || message.contains("would block") - || message.contains("Resource temporarily unavailable") - || message.contains("os error 10060") - || message.contains("os error 10035") -} - fn packet_debug_name(packet: &ServerboundPacket) -> &'static str { match packet { ServerboundPacket::Player { .. } => "C03 Player", @@ -413,6 +416,64 @@ mod tests { } } + /// Read one uncompressed frame (length-prefixed) from a fake server socket. + fn read_raw_frame(stream: &mut std::net::TcpStream) -> fpsmaster_protocol::codec::PacketFrame { + use std::io::Read; + let mut len = 0usize; + for shift in (0..35).step_by(7) { + let mut b = [0u8; 1]; + stream.read_exact(&mut b).unwrap(); + len |= ((b[0] & 0x7f) as usize) << shift; + if b[0] & 0x80 == 0 { + break; + } + } + let mut body = vec![0u8; len]; + stream.read_exact(&mut body).unwrap(); + fpsmaster_protocol::codec::PacketFrame::decode_payload(&body).unwrap() + } + + /// Outbound packets reach the socket while the server is silent (no read + /// timeout gating the writer), and a closed connection surfaces as a + /// Disconnected event. + #[test] + fn writer_sends_immediately_and_close_reports_disconnect() { + use fpsmaster_protocol::codec::{encode_frame, Compression, PacketFrame}; + use fpsmaster_protocol::io::PacketWriter; + use std::io::Write; + use std::time::{Duration, Instant}; + + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + let handle = NetworkHandle::connect_offline_1_8_9("127.0.0.1".into(), port, "Tester".into()); + let (mut server, _) = listener.accept().unwrap(); + assert_eq!(read_raw_frame(&mut server).id, 0x00); // handshake + assert_eq!(read_raw_frame(&mut server).id, 0x00); // login start + let mut success = PacketWriter::new(); + success.write_string("00000000-0000-0000-0000-000000000000"); + success.write_string("Tester"); + let frame = PacketFrame::new(0x02, success.into_inner()); + server + .write_all(&encode_frame(&frame, Compression::Disabled).unwrap()) + .unwrap(); + assert!(matches!( + handle.events.recv_timeout(Duration::from_secs(5)).unwrap(), + NetworkEvent::Connected { .. } + )); + + let sent = Instant::now(); + handle.send_packet(ServerboundPacket::KeepAlive { id: 7 }); + server.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + assert_eq!(read_raw_frame(&mut server).id, 0x00); + assert!(sent.elapsed() < Duration::from_secs(1)); + + drop(server); + assert!(matches!( + handle.events.recv_timeout(Duration::from_secs(5)).unwrap(), + NetworkEvent::Disconnected(_) + )); + } + #[test] fn walking_packets_match_vanilla_position_rotation_decision() { let mut state = WalkingPacketState::default(); diff --git a/crates/fpsmaster_protocol/src/codec.rs b/crates/fpsmaster_protocol/src/codec.rs index f1694dd..d7ab8e5 100644 --- a/crates/fpsmaster_protocol/src/codec.rs +++ b/crates/fpsmaster_protocol/src/codec.rs @@ -74,7 +74,14 @@ pub fn decode_frame(packet_data: &[u8], compression: Compression) -> Result crate::net::MAX_PACKET_LEN { + return Err(ProtocolError::InvalidData("bad uncompressed packet length")); + } + // Read at most one byte past the declared size so an over-long + // stream is detected as a mismatch without inflating all of it. + let mut decoder = ZlibDecoder::new(remaining).take(uncompressed_len as u64 + 1); let mut payload = Vec::with_capacity(uncompressed_len as usize); decoder.read_to_end(&mut payload)?; if payload.len() != uncompressed_len as usize { @@ -100,6 +107,31 @@ mod tests { assert_eq!(decoded, frame); } + #[test] + fn rejects_negative_or_oversized_uncompressed_length() { + let compression = Compression::Enabled { threshold: 32 }; + for len in [-1, (crate::net::MAX_PACKET_LEN + 1) as i32] { + let mut writer = PacketWriter::new(); + writer.write_var_i32(len); + writer.write_bytes(&[0x78, 0x9c, 0x03, 0x00]); + assert!(decode_frame(&writer.into_inner(), compression).is_err()); + } + } + + #[test] + fn rejects_payload_inflating_past_declared_length() { + let compression = Compression::Enabled { threshold: 32 }; + let encoded = encode_frame(&PacketFrame::new(0x21, vec![7; 512]), compression).unwrap(); + let mut reader = PacketReader::new(&encoded); + let len = reader.read_var_i32().unwrap() as usize; + let mut data = PacketReader::new(reader.read_bytes(len).unwrap()); + let real = data.read_var_i32().unwrap(); + let mut forged = PacketWriter::new(); + forged.write_var_i32(real - 100); + forged.write_bytes(data.read_remaining()); + assert!(decode_frame(&forged.into_inner(), compression).is_err()); + } + #[test] fn compressed_payload_round_trips() { let frame = PacketFrame::new(0x21, vec![7; 512]); diff --git a/crates/fpsmaster_protocol/src/net.rs b/crates/fpsmaster_protocol/src/net.rs index d14bee3..8ccc092 100644 --- a/crates/fpsmaster_protocol/src/net.rs +++ b/crates/fpsmaster_protocol/src/net.rs @@ -31,13 +31,7 @@ pub fn ping_status_1_8_9(host: &str, port: u16, timeout: Duration) -> Result, - /// AES-128-CFB8 encryptor enabled after `EncryptionResponse`. - encryptor: Option, /// AES-128-CFB8 decryptor enabled after `EncryptionResponse`. decryptor: Option, } +/// The outbound half of a connection: deflate, encryption, socket write. +pub struct ClientWriter { + stream: TcpStream, + compression: Compression, + /// AES-128-CFB8 encryptor enabled after `EncryptionResponse`. + encryptor: Option, +} + +pub struct BlockingClient { + reader: ClientReader, + writer: ClientWriter, +} + impl BlockingClient { pub fn connect(addr: A) -> Result { let stream = TcpStream::connect(addr)?; stream.set_nodelay(true)?; + Self::from_stream(stream) + } + + fn from_stream(stream: TcpStream) -> Result { Ok(Self { - stream, - compression: Compression::Disabled, - read_buf: Vec::new(), - encryptor: None, - decryptor: None, + writer: ClientWriter { + stream: stream.try_clone()?, + compression: Compression::Disabled, + encryptor: None, + }, + reader: ClientReader { + stream, + compression: Compression::Disabled, + read_buf: Vec::new(), + decryptor: None, + }, }) } + /// Split into independently owned read and write halves (same socket), so + /// one thread can block on reads while another writes immediately. Call + /// after login: compression and encryption state is fixed from here on. + pub fn into_split(self) -> (ClientReader, ClientWriter) { + (self.reader, self.writer) + } + + fn set_compression(&mut self, compression: Compression) { + self.reader.compression = compression; + self.writer.compression = compression; + } + /// Offline (cracked) login — no encryption; works with `online-mode=false` servers. pub fn login_offline_1_8_9( &mut self, @@ -142,13 +174,13 @@ impl BlockingClient { let frame = self.read_frame()?; match ClientboundLoginPacket::from_frame(frame)? { ClientboundLoginPacket::SetCompression { threshold } => { - self.compression = Compression::Enabled { threshold }; + self.set_compression(Compression::Enabled { threshold }); } ClientboundLoginPacket::LoginSuccess { uuid, username } => { return Ok(OfflineLoginResult { uuid, username, - compression: self.compression, + compression: self.reader.compression, }); } ClientboundLoginPacket::Disconnect { reason_json } => { @@ -212,7 +244,7 @@ impl BlockingClient { return Ok(OfflineLoginResult { uuid, username, - compression: self.compression, + compression: self.reader.compression, }); } ClientboundLoginPacket::SetCompression { .. } => { @@ -261,21 +293,21 @@ impl BlockingClient { self.write_packet(encryption_response_frame(enc_secret, enc_token))?; // ── Enable AES-128-CFB8 on both stream directions ──────────────────── - self.encryptor = Some(crate::crypto::Aes128Cfb8::encryptor(&shared_secret)); - self.decryptor = Some(crate::crypto::Aes128Cfb8::decryptor(&shared_secret)); + self.writer.encryptor = Some(crate::crypto::Aes128Cfb8::encryptor(&shared_secret)); + self.reader.decryptor = Some(crate::crypto::Aes128Cfb8::decryptor(&shared_secret)); // ── Read post-encryption login packets ──────────────────────────────── loop { let frame = self.read_frame()?; match ClientboundLoginPacket::from_frame(frame)? { ClientboundLoginPacket::SetCompression { threshold } => { - self.compression = Compression::Enabled { threshold }; + self.set_compression(Compression::Enabled { threshold }); } ClientboundLoginPacket::LoginSuccess { uuid, username } => { return Ok(OfflineLoginResult { uuid, username, - compression: self.compression, + compression: self.reader.compression, }); } ClientboundLoginPacket::Disconnect { reason_json } => { @@ -291,14 +323,24 @@ impl BlockingClient { } pub fn read_play_packet_1_8_9(&mut self) -> Result { - ClientboundPlayPacket::from_frame(self.read_frame()?) + self.reader.read_play_packet_1_8_9() } pub fn set_read_timeout(&self, timeout: Option) -> Result<()> { - self.stream.set_read_timeout(timeout)?; + self.reader.stream.set_read_timeout(timeout)?; Ok(()) } + pub fn write_packet(&mut self, frame: PacketFrame) -> Result<()> { + self.writer.write_packet(frame) + } + + fn read_frame(&mut self) -> Result { + self.reader.read_frame() + } +} + +impl ClientWriter { pub fn write_packet(&mut self, frame: PacketFrame) -> Result<()> { let mut encoded = encode_frame(&frame, self.compression)?; if let Some(enc) = &mut self.encryptor { @@ -308,12 +350,29 @@ impl BlockingClient { Ok(()) } + /// Shut down both directions of the socket, unblocking a reader parked in + /// a blocking read on the other half. + pub fn shutdown(&self) { + let _ = self.stream.shutdown(std::net::Shutdown::Both); + } +} + +impl ClientReader { + pub fn read_play_packet_1_8_9(&mut self) -> Result { + ClientboundPlayPacket::from_frame(self.read_frame()?) + } + fn read_frame(&mut self) -> Result { loop { if let Some((length, header_len)) = read_var_i32_prefix(&self.read_buf)? { if length < 0 { return Err(ProtocolError::InvalidData("negative packet length")); } + // An oversized length would otherwise grow `read_buf` without + // bound waiting for bytes that may never come. + if length as usize > MAX_PACKET_LEN { + return Err(ProtocolError::InvalidData("packet length exceeds 2097152")); + } let frame_len = header_len + length as usize; if self.read_buf.len() >= frame_len { @@ -369,6 +428,22 @@ mod frame_read_tests { assert_eq!(read_var_i32_prefix(&[0x80, 0x01]).unwrap(), Some((128, 2))); } + /// A frame length above vanilla's limit is rejected up front instead of + /// buffering forever (or until allocation fails) for its body. + #[test] + fn oversized_frame_length_is_rejected() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let mut client = BlockingClient::connect(listener.local_addr().unwrap()).unwrap(); + let (mut server, _) = listener.accept().unwrap(); + let mut len = PacketWriter::new(); + len.write_var_i32(MAX_PACKET_LEN as i32 + 1); + server.write_all(&len.into_inner()).unwrap(); + assert_eq!( + client.read_frame().unwrap_err(), + ProtocolError::InvalidData("packet length exceeds 2097152") + ); + } + #[test] fn read_var_i32_prefix_rejects_too_long_varint() { assert_eq!( diff --git a/crates/fpsmaster_protocol/src/v1_8_9/packets.rs b/crates/fpsmaster_protocol/src/v1_8_9/packets.rs index 99f256b..44a114d 100644 --- a/crates/fpsmaster_protocol/src/v1_8_9/packets.rs +++ b/crates/fpsmaster_protocol/src/v1_8_9/packets.rs @@ -1129,7 +1129,8 @@ impl ClientboundPlayPacket { 0x22 => { let chunk_x = body.read_i32()?; let chunk_z = body.read_i32()?; - let count = body.read_var_i32()? as usize; + // Each record is a u16 position plus a VarInt state (>= 3 bytes). + let count = checked_count(body.read_var_i32()?, &body, 3)?; let mut changes = Vec::with_capacity(count); for _ in 0..count { let packed = body.read_u16()?; @@ -1190,7 +1191,8 @@ impl ClientboundPlayPacket { } 0x26 => { let sky_light_sent = body.read_bool()?; - let count = body.read_var_i32()? as usize; + // Each column header is two i32 coordinates plus a u16 mask. + let count = checked_count(body.read_var_i32()?, &body, 10)?; let mut meta = Vec::with_capacity(count); for _ in 0..count { meta.push((body.read_i32()?, body.read_i32()?, body.read_u16()?)); @@ -1324,7 +1326,7 @@ impl ClientboundPlayPacket { pitch: angle(body.read_i8()?), }), 0x13 => { - let count = body.read_var_i32()? as usize; + let count = checked_count(body.read_var_i32()?, &body, 1)?; let mut entity_ids = Vec::with_capacity(count); for _ in 0..count { entity_ids.push(body.read_var_i32()?); @@ -1444,11 +1446,8 @@ impl ClientboundPlayPacket { let name = body.read_string(64)?; let mode = body.read_u8()?; let read_players = |body: &mut PacketReader<'_>| -> Result> { - let count = body.read_var_i32()?; - if count < 0 { - return Err(ProtocolError::InvalidData("negative team player count")); - } - let mut players = Vec::with_capacity(count as usize); + let count = checked_count(body.read_var_i32()?, body, 1)?; + let mut players = Vec::with_capacity(count); for _ in 0..count { players.push(body.read_string(64)?); } @@ -1678,6 +1677,21 @@ impl ClientboundPlayPacket { } } +/// Validate a server-supplied element count before it sizes an allocation: +/// reject negatives (which cast to a huge `usize` and panic `with_capacity`) +/// and counts that cannot fit in the rest of the packet, given each element +/// takes at least `min_bytes`. +fn checked_count(count: i32, body: &PacketReader<'_>, min_bytes: usize) -> Result { + if count < 0 { + return Err(ProtocolError::InvalidData("negative element count")); + } + let count = count as usize; + if count.saturating_mul(min_bytes) > body.remaining() { + return Err(ProtocolError::InvalidData("element count exceeds packet size")); + } + Ok(count) +} + fn read_title_action(body: &mut PacketReader<'_>) -> Result { Ok(match body.read_var_i32()? { 0 => TitleAction::Title { @@ -1700,6 +1714,24 @@ fn read_title_action(body: &mut PacketReader<'_>) -> Result { #[cfg(test)] mod tests { use super::*; + + /// Hostile counts in list packets fail decoding instead of panicking in + /// `Vec::with_capacity` (negative -> huge usize) or aborting on allocation. + #[test] + fn hostile_element_counts_are_rejected() { + let frame = |id: i32, prefix: &[u8], count: i32| { + let mut body = PacketWriter::new(); + body.write_bytes(prefix); + body.write_var_i32(count); + body.write_bytes(&[0; 8]); + PacketFrame::new(id, body.into_inner()) + }; + for count in [-1, i32::MAX] { + assert!(ClientboundPlayPacket::from_frame(frame(0x13, &[], count)).is_err()); + assert!(ClientboundPlayPacket::from_frame(frame(0x22, &[0; 8], count)).is_err()); + assert!(ClientboundPlayPacket::from_frame(frame(0x26, &[1], count)).is_err()); + } + } use crate::v1_8_9::PROTOCOL_VERSION; #[test] diff --git a/crates/fpsmaster_render/src/lang/fpsmaster.en_us.lang b/crates/fpsmaster_render/src/lang/fpsmaster.en_us.lang index 4bb100f..51766e9 100644 --- a/crates/fpsmaster_render/src/lang/fpsmaster.en_us.lang +++ b/crates/fpsmaster_render/src/lang/fpsmaster.en_us.lang @@ -81,6 +81,7 @@ fpsmaster.shaders.clouds=Clouds # Progress / connect / auth screens fpsmaster.progress.loadingWorld=Loading world... fpsmaster.progress.connecting=Connecting to server... +fpsmaster.network.threadStopped=The network thread stopped unexpectedly (internal error) fpsmaster.progress.chunksLoaded=%s chunks loaded fpsmaster.server.startFailed=Failed to start server fpsmaster.server.startTimeout=Server did not respond within 60 seconds diff --git a/crates/fpsmaster_render/src/lang/fpsmaster.zh_cn.lang b/crates/fpsmaster_render/src/lang/fpsmaster.zh_cn.lang index 6f841d5..23888c3 100644 --- a/crates/fpsmaster_render/src/lang/fpsmaster.zh_cn.lang +++ b/crates/fpsmaster_render/src/lang/fpsmaster.zh_cn.lang @@ -79,6 +79,7 @@ fpsmaster.shaders.clouds=云 # 进度 / 连接 / 登录界面 fpsmaster.progress.loadingWorld=正在加载世界… fpsmaster.progress.connecting=正在连接服务器… +fpsmaster.network.threadStopped=网络线程意外停止（内部错误） fpsmaster.progress.chunksLoaded=已加载 %s 个区块 fpsmaster.server.startFailed=服务器启动失败 fpsmaster.server.startTimeout=服务器在 60 秒内未响应 From 089b6fcce82a9656f6abad80684b94df05133d5a Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:08:42 -0700 Subject: [PATCH 05/15] fix(mesh): cap the mesh job backlog and survive mesher panics - Each queued mesh job pins a 1-2 MB column snapshot and the job channel was unbounded: under a chunk burst the main thread kept submitting 40 sections a frame regardless of worker progress. Track in-flight jobs and stop taking ordinary dirty sections once the backlog reaches 16 per worker thread; they stay dirty for later frames. - A panic in build_section_mesh_neighborhood killed its worker and dropped the job. Catch it per job, log the section, count it and deliver an empty mesh; recover a poisoned queue lock instead of unwrapping it. - F3 shows the mesh queue depth and, if any, the failure count. --- crates/fpsmaster_app/src/gui/ingame.rs | 4 + crates/fpsmaster_app/src/main.rs | 7 +- crates/fpsmaster_render/src/mesh_worker.rs | 122 +++++++++++++++++--- crates/fpsmaster_render/src/renderer/mod.rs | 12 ++ 4 files changed, 129 insertions(+), 16 deletions(-) diff --git a/crates/fpsmaster_app/src/gui/ingame.rs b/crates/fpsmaster_app/src/gui/ingame.rs index 3f20525..0bf9fb2 100644 --- a/crates/fpsmaster_app/src/gui/ingame.rs +++ b/crates/fpsmaster_app/src/gui/ingame.rs @@ -261,7 +261,11 @@ fn draw_debug_overlay( format!("submit {}us acquire {}us", s.submit_us, s.acquire_us), format!("draws {} visible {}", s.draw_calls, s.visible_chunks), format!("tris {}", s.chunk_indices / 3), + format!("mesh queue {}", s.mesh_in_flight), ]; + if s.mesh_failures > 0 { + right.push(format!("mesh failures {} (see log)", s.mesh_failures)); + } // Per-phase CPU frame breakdown (from the frame profiler): which subsystem owns // the frame. `avg/max ms` over the last ~1s. Phases under 0.05ms max are hidden. let p = &info.phases; diff --git a/crates/fpsmaster_app/src/main.rs b/crates/fpsmaster_app/src/main.rs index 78e0123..ed940eb 100644 --- a/crates/fpsmaster_app/src/main.rs +++ b/crates/fpsmaster_app/src/main.rs @@ -2453,7 +2453,12 @@ fn render_frame( if !urgent_chunks.is_empty() { renderer.queue_chunk_meshes(&app.game.world, urgent_chunks.iter().copied()); } - let dirty_budget = MESH_SUBMITS_PER_FRAME.saturating_sub(urgent_chunks.len()); + // Backpressure: each queued job pins a column snapshot, so stop pulling + // ordinary dirty sections once the worker backlog is full (they stay in the + // dirty set). Urgent local edits above always go through. + let dirty_budget = MESH_SUBMITS_PER_FRAME + .saturating_sub(urgent_chunks.len()) + .min(renderer.mesh_submit_capacity()); let dirty_chunks = app.game.take_dirty_chunks_budget(dirty_budget); if !dirty_chunks.is_empty() { renderer.queue_chunk_meshes(&app.game.world, dirty_chunks); diff --git a/crates/fpsmaster_render/src/mesh_worker.rs b/crates/fpsmaster_render/src/mesh_worker.rs index f1656ac..ef3ddd7 100644 --- a/crates/fpsmaster_render/src/mesh_worker.rs +++ b/crates/fpsmaster_render/src/mesh_worker.rs @@ -1,5 +1,5 @@ use std::sync::{ - atomic::{AtomicBool, Ordering}, + atomic::{AtomicBool, AtomicUsize, Ordering}, mpsc::{channel, Receiver, Sender}, Arc, Mutex, }; @@ -33,8 +33,21 @@ pub struct MeshWorker { fast_leaves: Arc, /// Flat (greedy, smooth-lighting-off) meshing: same per-job read + toggle. flat: Arc, + /// Jobs submitted whose result has not been taken yet. Each queued job pins + /// a ~1-2 MB column snapshot, so the caller throttles submissions on this + /// (see `capacity`). + in_flight: usize, + /// Upper bound on `in_flight` before `capacity` reports zero. + max_in_flight: usize, + /// Mesh builds that panicked (skipped; the section renders empty). + failures: Arc, } +/// Queued-plus-unconsumed jobs allowed per worker thread before submissions +/// pause: enough to keep every worker busy across a frame, small enough that +/// a burst can't pile up hundreds of MB of snapshots. +const IN_FLIGHT_PER_THREAD: usize = 16; + impl MeshWorker { pub fn new(atlas: AtlasUv, biome: BiomeColors) -> Self { let (job_tx, job_rx) = channel::(); @@ -44,6 +57,7 @@ impl MeshWorker { let job_rx = Arc::new(Mutex::new(job_rx)); let fast_leaves = Arc::new(AtomicBool::new(false)); let flat = Arc::new(AtomicBool::new(false)); + let failures = Arc::new(AtomicUsize::new(0)); let threads = thread::available_parallelism() .map(|n| n.get().saturating_sub(2)) @@ -58,26 +72,44 @@ impl MeshWorker { let biome = biome.clone(); let fast_leaves = Arc::clone(&fast_leaves); let flat = Arc::clone(&flat); + let failures = Arc::clone(&failures); let _ = thread::Builder::new() .name(format!("mesh-worker-{index}")) .spawn(move || { loop { // Hold the receiver lock only while dequeuing; the mesh // build below runs lock-free so workers run in parallel. - let job = match job_rx.lock().unwrap().recv() { + // The lock only guards `recv`, so a poisoned mutex holds no + // broken state; don't let one dead worker take the rest down. + let job = match job_rx + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .recv() + { Ok(job) => job, Err(_) => break, // sender dropped — shut down. }; let col = job.neighborhood.position(); let pos = SectionPos::new(col.x, job.section_y, col.z); - let mesh = build_section_mesh_neighborhood( - &job.neighborhood, - job.section_y, - &atlas, - &biome, - fast_leaves.load(Ordering::Relaxed), - flat.load(Ordering::Relaxed), - ); + // A mesher bug on some odd block state must not kill the + // worker (the pool would drain and meshing stop for good): + // log it, count it for F3, and deliver an empty mesh so the + // job is still accounted for. + let built = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + build_section_mesh_neighborhood( + &job.neighborhood, + job.section_y, + &atlas, + &biome, + fast_leaves.load(Ordering::Relaxed), + flat.load(Ordering::Relaxed), + ) + })); + let mesh = built.unwrap_or_else(|_| { + failures.fetch_add(1, Ordering::Relaxed); + log::error!("mesh build panicked for section {pos:?}; left empty"); + ChunkMesh::default() + }); if result_tx.send((pos, job.generation, mesh)).is_err() { break; } @@ -90,9 +122,27 @@ impl MeshWorker { result_rx, fast_leaves, flat, + in_flight: 0, + max_in_flight: threads * IN_FLIGHT_PER_THREAD, + failures, } } + /// How many more sections may be submitted before the backlog cap. + pub fn capacity(&self) -> usize { + self.max_in_flight.saturating_sub(self.in_flight) + } + + /// Jobs submitted but not yet taken back with `try_recv`. + pub fn in_flight(&self) -> usize { + self.in_flight + } + + /// Mesh builds that panicked since this pool started. + pub fn failures(&self) -> usize { + self.failures.load(Ordering::Relaxed) + } + /// Set the Fast-leaves flag future mesh jobs read. The caller must re-queue /// the loaded sections for the change to reach already-built meshes. pub fn set_fast_leaves(&self, on: bool) { @@ -106,16 +156,58 @@ impl MeshWorker { } /// Queue one section of a column snapshot for meshing. - pub fn submit(&self, neighborhood: Arc, section_y: i32, generation: u64) { - let _ = self.job_tx.send(Job { + pub fn submit(&mut self, neighborhood: Arc, section_y: i32, generation: u64) { + let job = Job { neighborhood, section_y, generation, - }); + }; + if self.job_tx.send(job).is_ok() { + self.in_flight += 1; + } else { + // Only possible if every worker thread is gone. + log::error!("mesh workers are gone; section {section_y} not queued"); + } } /// Take one finished mesh, if any is ready. - pub fn try_recv(&self) -> Option<(SectionPos, u64, ChunkMesh)> { - self.result_rx.try_recv().ok() + pub fn try_recv(&mut self) -> Option<(SectionPos, u64, ChunkMesh)> { + let result = self.result_rx.try_recv().ok()?; + self.in_flight = self.in_flight.saturating_sub(1); + Some(result) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use fpsmaster_core::{BlockState, ChunkPos, World}; + + /// Submissions count against the backlog cap until their result is taken. + #[test] + fn in_flight_tracks_submit_and_receive() { + let atlas = crate::TextureAtlasImage::load_default().uv_table(); + let mut worker = MeshWorker::new(atlas, BiomeColors::default()); + let cap = worker.capacity(); + assert!(cap > 0); + let mut world = World::new(); + world.set_block(0, 0, 0, BlockState::STONE); + let snapshot = Arc::new(ChunkNeighborhood::snapshot(&world, ChunkPos::new(0, 0)).unwrap()); + for _ in 0..3 { + worker.submit(Arc::clone(&snapshot), 0, 1); + } + assert_eq!(worker.capacity(), cap - 3); + let mut received = 0; + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + while received < 3 && std::time::Instant::now() < deadline { + if worker.try_recv().is_some() { + received += 1; + } else { + thread::yield_now(); + } + } + assert_eq!(received, 3); + assert_eq!(worker.capacity(), cap); + assert_eq!(worker.failures(), 0); } } diff --git a/crates/fpsmaster_render/src/renderer/mod.rs b/crates/fpsmaster_render/src/renderer/mod.rs index 110ca39..67812d4 100644 --- a/crates/fpsmaster_render/src/renderer/mod.rs +++ b/crates/fpsmaster_render/src/renderer/mod.rs @@ -276,6 +276,10 @@ pub struct RenderStats { pub draw_calls: u32, /// Total indices submitted across all chunk draws. pub chunk_indices: u32, + /// Section mesh jobs queued or finished but not yet uploaded. + pub mesh_in_flight: u32, + /// Section mesh builds that panicked (those sections render empty). + pub mesh_failures: u32, } /// A single `draw_indexed_indirect` command (matches the GPU layout). @@ -6855,6 +6859,12 @@ impl Renderer { /// sections whose column is no longer loaded drop their GPU mesh now. The /// snapshot clone is the only main-thread cost; the mesh build itself runs /// off-thread, so chunk updates never stall the frame. + /// How many more dirty sections the background mesher accepts this frame + /// before its backlog cap; callers keep the rest dirty for later frames. + pub fn mesh_submit_capacity(&self) -> usize { + self.mesh_worker.capacity() + } + pub fn queue_chunk_meshes(&mut self, world: &World, sections: I) where I: IntoIterator, @@ -10790,6 +10800,8 @@ impl Renderer { visible_chunks, draw_calls, chunk_indices, + mesh_in_flight: self.mesh_worker.in_flight() as u32, + mesh_failures: self.mesh_worker.failures() as u32, }; Ok(()) } From e96809a5bef3c468a952302484a6eefcdbfe7c84 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:09:13 -0700 Subject: [PATCH 06/15] perf(settings): take &self in derived-value getters mouse_factor, fps_limit and the other derived getters took Settings by value, so every mouse event and frame cloned the whole struct (Vec/String fields included). Borrow instead and drop the clones at the call sites. --- crates/fpsmaster_app/src/gui/options.rs | 18 ++++++------- crates/fpsmaster_app/src/gui/performance.rs | 6 ++--- crates/fpsmaster_app/src/main.rs | 8 +++--- crates/fpsmaster_app/src/settings.rs | 28 ++++++++++----------- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/crates/fpsmaster_app/src/gui/options.rs b/crates/fpsmaster_app/src/gui/options.rs index aae2825..c411114 100644 --- a/crates/fpsmaster_app/src/gui/options.rs +++ b/crates/fpsmaster_app/src/gui/options.rs @@ -184,14 +184,14 @@ impl GuiScreen for GuiOptions { self.sensitivity_rect, s, ctx.settings.sensitivity, - &format!("{}: {:.0}%", tr("options.sensitivity"), ctx.settings.clone().sensitivity_percent()), + &format!("{}: {:.0}%", tr("options.sensitivity"), ctx.settings.sensitivity_percent()), ); draw_slider( ui, self.brightness_rect, s, - ctx.settings.clone().brightness_fraction(), - &format!("{}: {}%", tr("options.gamma"), ctx.settings.clone().brightness_percent()), + ctx.settings.brightness_fraction(), + &format!("{}: {}%", tr("options.gamma"), ctx.settings.brightness_percent()), ); if let Some(b) = &self.video_btn { b.draw(ui, s, ctx.mouse, ctx.mouse_down); @@ -210,7 +210,7 @@ impl GuiScreen for GuiOptions { } if let Some(gui_scale) = &mut self.gui_scale_btn { gui_scale.label = - format!("{}: {}", tr("options.guiScale"), ctx.settings.clone().gui_scale_label()); + format!("{}: {}", tr("options.guiScale"), ctx.settings.gui_scale_label()); gui_scale.draw(ui, s, ctx.mouse, ctx.mouse_down); } if let Some(done) = &self.done { @@ -442,7 +442,7 @@ impl GuiScreen for GuiVideoSettings { ui, self.render_distance_rect, s, - ctx.settings.clone().render_distance_fraction(), + ctx.settings.render_distance_fraction(), &format!( "{}: {} {}", tr("options.renderDistance"), @@ -454,8 +454,8 @@ impl GuiScreen for GuiVideoSettings { ui, self.fps_rect, s, - ctx.settings.clone().fps_fraction(), - &format!("{}: {}", tr("options.framerateLimit"), ctx.settings.clone().fps_label()), + ctx.settings.fps_fraction(), + &format!("{}: {}", tr("options.framerateLimit"), ctx.settings.fps_label()), ); if let Some(vsync) = &mut self.vsync { vsync.label = format!("{}: {}", tr("options.vsync"), on_off(ctx.settings.vsync)); @@ -475,14 +475,14 @@ impl GuiScreen for GuiVideoSettings { } if let Some(mipmaps) = &mut self.mipmaps { mipmaps.label = - format!("{}: {}", tr("options.mipmapLevels"), ctx.settings.clone().mipmap_label()); + format!("{}: {}", tr("options.mipmapLevels"), ctx.settings.mipmap_label()); mipmaps.draw(ui, s, ctx.mouse, ctx.mouse_down); } if let Some(resolution) = &mut self.resolution { resolution.label = format!( "{}: {}", tr("fpsmaster.options.resolution"), - ctx.settings.clone().resolution_label() + ctx.settings.resolution_label() ); resolution.draw(ui, s, ctx.mouse, ctx.mouse_down); } diff --git a/crates/fpsmaster_app/src/gui/performance.rs b/crates/fpsmaster_app/src/gui/performance.rs index ebb42b4..b873966 100644 --- a/crates/fpsmaster_app/src/gui/performance.rs +++ b/crates/fpsmaster_app/src/gui/performance.rs @@ -190,15 +190,15 @@ impl GuiScreen for GuiPerformance { ui, self.render_scale_rect, s, - st.clone().render_scale_fraction(), - &format!("{}: {}%", tr("fpsmaster.options.renderScale"), st.clone().render_scale_percent()), + st.render_scale_fraction(), + &format!("{}: {}%", tr("fpsmaster.options.renderScale"), st.render_scale_percent()), ); if cfg!(feature = "dlss") { draw_slider( ui, self.dlss_quality_rect, s, - st.clone().dlss_quality_fraction(), + st.dlss_quality_fraction(), &format!("DLSS {}: {}", tr("fpsmaster.perf.rt.quality"), dlss_quality_label(st.dlss_quality)), ); } diff --git a/crates/fpsmaster_app/src/main.rs b/crates/fpsmaster_app/src/main.rs index ed940eb..da71e39 100644 --- a/crates/fpsmaster_app/src/main.rs +++ b/crates/fpsmaster_app/src/main.rs @@ -713,7 +713,7 @@ impl ApplicationHandler for WinitApp { app.game.rotate_view( delta.0 as f32, delta.1 as f32, - app.settings.clone().mouse_factor(), + app.settings.mouse_factor(), ); } } @@ -909,7 +909,7 @@ impl ApplicationHandler for WinitApp { } if self.pass_bench.is_none() { - if let Some(cap) = app.settings.clone().fps_limit() { + if let Some(cap) = app.settings.fps_limit() { let deadline = self.last_frame + Duration::from_secs_f64(1.0 / cap as f64); let now = Instant::now(); if now < deadline { @@ -1034,7 +1034,7 @@ impl ApplicationHandler for WinitApp { } let now = Instant::now(); if self.adaptive_gpu_ms > 0.0 && (now - self.adaptive_last_adjust).as_secs_f32() >= 1.0 { - let target_fps = app.settings.clone().fps_limit().unwrap_or(60).min(120) as f32; + let target_fps = app.settings.fps_limit().unwrap_or(60).min(120) as f32; let budget = 1000.0 / target_fps; let max_scale = app.settings.render_scale; let new_scale = if self.adaptive_gpu_ms > budget * 0.95 { @@ -1576,7 +1576,7 @@ fn apply_ext_commands(app: &mut App) { fpsmaster_ext::ExtCommand::SetRotation { yaw, pitch, silent } => { // The silent look is snapped to the player's own mouse-rotation // quantum (sensitivity factor) so its deltas pass Grim's GCD check. - let step = app.settings.clone().mouse_factor(); + let step = app.settings.mouse_factor(); app.game.ext_set_rotation(yaw, pitch, silent, step); } fpsmaster_ext::ExtCommand::ClearSilentRotation => { diff --git a/crates/fpsmaster_app/src/settings.rs b/crates/fpsmaster_app/src/settings.rs index 3537e89..c165fbf 100644 --- a/crates/fpsmaster_app/src/settings.rs +++ b/crates/fpsmaster_app/src/settings.rs @@ -594,18 +594,18 @@ impl Settings { /// Degrees of view rotation per pixel of mouse motion. Reproduces the /// vanilla curve so 0.5 maps to the long-standing 0.15 default. - pub fn mouse_factor(self) -> f32 { + pub fn mouse_factor(&self) -> f32 { let f = self.sensitivity * 0.6 + 0.2; f * f * f * 8.0 * 0.15 } /// Sensitivity shown to the player as a 0..=200% value, vanilla-style. - pub fn sensitivity_percent(self) -> f32 { + pub fn sensitivity_percent(&self) -> f32 { self.sensitivity * 200.0 } /// The active frame cap, or `None` when the slider is at "unlimited". - pub fn fps_limit(self) -> Option { + pub fn fps_limit(&self) -> Option { if self.fps_cap >= FPS_MAX { None } else { @@ -613,7 +613,7 @@ impl Settings { } } - pub fn fps_label(self) -> String { + pub fn fps_label(&self) -> String { match self.fps_limit() { None => crate::i18n::tr("options.framerateLimit.max"), Some(cap) => format!("{cap} FPS"), @@ -621,7 +621,7 @@ impl Settings { } /// FPS slider fill fraction in 0..=1. - pub fn fps_fraction(self) -> f32 { + pub fn fps_fraction(&self) -> f32 { (self.fps_cap - FPS_MIN) as f32 / (FPS_MAX - FPS_MIN) as f32 } @@ -637,11 +637,11 @@ impl Settings { } /// Render-scale slider fill fraction in 0..=1. - pub fn render_scale_fraction(self) -> f32 { + pub fn render_scale_fraction(&self) -> f32 { (self.render_scale - RENDER_SCALE_MIN) / (1.0 - RENDER_SCALE_MIN) } - pub fn render_scale_percent(self) -> u32 { + pub fn render_scale_percent(&self) -> u32 { (self.render_scale * 100.0).round() as u32 } @@ -652,7 +652,7 @@ impl Settings { } /// DLSS-quality slider fill fraction in 0..=1 (knob snaps to the 4 stops). - pub fn dlss_quality_fraction(self) -> f32 { + pub fn dlss_quality_fraction(&self) -> f32 { self.dlss_quality.min(DLSS_QUALITY_MAX) as f32 / DLSS_QUALITY_MAX as f32 } @@ -661,7 +661,7 @@ impl Settings { } /// Render-distance slider fill fraction in 0..=1. - pub fn render_distance_fraction(self) -> f32 { + pub fn render_distance_fraction(&self) -> f32 { (self.render_distance - RENDER_DIST_MIN) as f32 / (RENDER_DIST_MAX - RENDER_DIST_MIN) as f32 } @@ -672,11 +672,11 @@ impl Settings { } /// Brightness slider fill fraction in 0..=1. - pub fn brightness_fraction(self) -> f32 { + pub fn brightness_fraction(&self) -> f32 { (self.brightness - BRIGHTNESS_MIN) / (BRIGHTNESS_MAX - BRIGHTNESS_MIN) } - pub fn brightness_percent(self) -> u32 { + pub fn brightness_percent(&self) -> u32 { (self.brightness * 100.0).round() as u32 } @@ -744,7 +744,7 @@ impl Settings { }; } - pub fn mipmap_label(self) -> String { + pub fn mipmap_label(&self) -> String { if self.mipmap_levels == 0 { crate::i18n::tr("options.off") } else { @@ -762,7 +762,7 @@ impl Settings { } /// Value label for the GUI-scale button: `Auto` at 0, else the number. - pub fn gui_scale_label(self) -> String { + pub fn gui_scale_label(&self) -> String { if self.gui_scale == 0 { crate::i18n::tr("options.guiScale.auto") } else { @@ -779,7 +779,7 @@ impl Settings { self.resolution = RESOLUTION_PRESETS[(idx + 1) % RESOLUTION_PRESETS.len()]; } - pub fn resolution_label(self) -> String { + pub fn resolution_label(&self) -> String { match self.resolution { None => crate::i18n::tr("fpsmaster.options.native"), Some((w, h)) => format!("{w}x{h}"), From 518de55d16b6fa09bb4a4cacc2975dfecfc2e587 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:09:42 -0700 Subject: [PATCH 07/15] perf(render): re-mesh block tint changes on the background mesher Applying extension block tints rebuilt and uploaded every loaded section synchronously on the main thread (upload_world), freezing the frame when tint mods reloaded in game. Install the tint table and mark all sections dirty instead, like the resource pack reload path. --- crates/fpsmaster_app/src/main.rs | 9 ++++++--- crates/fpsmaster_render/src/renderer/mod.rs | 9 --------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/crates/fpsmaster_app/src/main.rs b/crates/fpsmaster_app/src/main.rs index da71e39..0c9172f 100644 --- a/crates/fpsmaster_app/src/main.rs +++ b/crates/fpsmaster_app/src/main.rs @@ -924,10 +924,13 @@ impl ApplicationHandler for WinitApp { // Per-frame extension hook (on_frame), e.g. native render geometry. app.ext.dispatch_frame(&GameViews(&app.game)); apply_ext_commands(app); - // Apply pending extension block-tint overrides (re-meshes the loaded - // world once; future chunks read the renderer's global tint table). + // Apply pending extension block-tint overrides: install the global tint + // table the mesher reads, then re-mesh the loaded world through the + // budgeted background path (a synchronous rebuild froze the frame for + // hundreds of ms to seconds when tint mods reloaded in game). if app.tints_dirty && app.in_world { - renderer.set_block_tints(app.block_tints.clone(), &app.game.world); + fpsmaster_render::set_block_tints(app.block_tints.clone()); + app.game.mark_all_sections_dirty(); app.tints_dirty = false; } // Upload native render-hook geometry when a mod changed it. diff --git a/crates/fpsmaster_render/src/renderer/mod.rs b/crates/fpsmaster_render/src/renderer/mod.rs index 67812d4..e5a8953 100644 --- a/crates/fpsmaster_render/src/renderer/mod.rs +++ b/crates/fpsmaster_render/src/renderer/mod.rs @@ -6826,15 +6826,6 @@ impl Renderer { self.upload_dirty_sections(world, sections); } - /// Install extension block-tint overrides (the `setBlockTint` preset) and - /// re-mesh all loaded chunks so the change takes effect. A synchronous - /// re-mesh like an atlas reload — fine because tint changes are rare (mostly - /// at mod load). - pub fn set_block_tints(&mut self, table: crate::TintTable, world: &World) { - crate::set_block_tints(table); - self.upload_world(world); - } - pub fn upload_dirty_sections(&mut self, world: &World, sections: I) where I: IntoIterator, From f5837998711581d797c40e6f34f6874cec63ec48 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:11:38 -0700 Subject: [PATCH 08/15] fix(skin): reassign recycled skin rows, pool downloads, stop per-frame copies - Recycling one of the 64 player skin rows left the old owner marked as requested, so it never got a row again and showed the default skin for the rest of the session. Keep decoded skins cached and give rows to the players drawn this frame (spawned players, skull owners, local player), recycling the least recently wanted row; a returning player gets its row back from the cache without a re-download. - Downloads ran on one new thread per uuid with no timeout. Use four shared worker threads and a 15 s ureq timeout. - Filter the roster by is_requested before copying the base64 textures property, instead of cloning every property every frame. --- crates/fpsmaster_app/src/game.rs | 17 +++ crates/fpsmaster_app/src/main.rs | 24 ++-- crates/fpsmaster_app/src/skin.rs | 222 ++++++++++++++++++++++++++----- 3 files changed, 217 insertions(+), 46 deletions(-) diff --git a/crates/fpsmaster_app/src/game.rs b/crates/fpsmaster_app/src/game.rs index 5011256..3384b7a 100644 --- a/crates/fpsmaster_app/src/game.rs +++ b/crates/fpsmaster_app/src/game.rs @@ -6480,6 +6480,23 @@ impl GameState { } } + /// uuids whose skin is drawn in the loaded world: spawned players, block + /// skull owners and the local player (looked up by `local_name` in the tab + /// list). The skin manager keeps atlas rows for these first. + pub fn skin_owners_in_view(&self, local_name: &str) -> HashSet<[u8; 16]> { + let local = self + .player_list + .iter() + .find(|(_, info)| info.name == local_name) + .map(|(uuid, _)| *uuid); + self.entity_uuids + .values() + .copied() + .chain(self.skulls.values().filter_map(|skull| skull.owner)) + .chain(local) + .collect() + } + /// Owner profiles seen on player skulls that carry a skin texture, for the /// host to hand to the skin downloader (the same route the tab-list roster /// takes). Skulls can name players who were never in the player list, so diff --git a/crates/fpsmaster_app/src/main.rs b/crates/fpsmaster_app/src/main.rs index 0c9172f..9ceccae 100644 --- a/crates/fpsmaster_app/src/main.rs +++ b/crates/fpsmaster_app/src/main.rs @@ -2530,22 +2530,26 @@ fn render_frame( // so the entity model can sample their atlas rows. Skull owners need // their own pass: the head can name someone who never joined, so the // tab-list roster alone would leave it on the default skin. - let new_skins: Vec<([u8; 16], String)> = app + // Filter before copying the base64 property, so a steady roster costs + // no per-frame allocation. + let profiles = app .game .player_list .iter() - .filter_map(|(uuid, info)| info.texture_property().map(|t| (*uuid, t.to_owned()))) + .filter_map(|(uuid, info)| info.texture_property().map(|t| (uuid, t))) .chain( app.game .skull_skin_profiles() - .map(|(uuid, property)| (*uuid, property.clone())), - ) - .filter(|(uuid, _)| !app.skin_manager.is_requested(uuid)) - .collect(); - for (uuid, property) in new_skins { - app.skin_manager.request(uuid, &property); - } - app.skin_manager.poll(renderer); + .map(|(uuid, property)| (uuid, property.as_str())), + ); + for (uuid, property) in profiles { + if !app.skin_manager.is_requested(uuid) { + app.skin_manager.request(*uuid, property); + } + } + let wanted = app.game.skin_owners_in_view(&app.username); + app.skin_manager + .poll(&mut |row, rgba| renderer.upload_player_skin(row, rgba), &wanted); // Skip the whole entity-model + hand + nametag rebuild and its GPU upload // when nothing that feeds them changed since last frame — the renderer diff --git a/crates/fpsmaster_app/src/skin.rs b/crates/fpsmaster_app/src/skin.rs index e50c1c0..792ccd6 100644 --- a/crates/fpsmaster_app/src/skin.rs +++ b/crates/fpsmaster_app/src/skin.rs @@ -7,11 +7,21 @@ use std::collections::{HashMap, HashSet}; use std::io::Read; use std::sync::mpsc::{self, Receiver, Sender}; +use std::sync::{Arc, Mutex}; use std::thread; +use std::time::Duration; use base64::Engine; use fpsmaster_render::texture::{normalize_skin_png, PLAYER_SKIN_SLOTS}; -use fpsmaster_render::Renderer; + +/// Background download threads shared by every skin request (a lobby join used +/// to spawn one thread per player). +const DOWNLOAD_THREADS: usize = 4; +/// Whole-request timeout for one skin download. +const DOWNLOAD_TIMEOUT: Duration = Duration::from_secs(15); +/// Decoded skins kept in memory (16 KiB each) so a player whose atlas row was +/// recycled gets it back without a re-download. +const MAX_CACHED_SKINS: usize = 1024; /// A decoded skin ready to upload: the owner uuid and 64×64 RGBA bytes. struct LoadedSkin { @@ -19,17 +29,26 @@ struct LoadedSkin { rgba: Vec, } +struct DownloadJob { + uuid: [u8; 16], + property: String, +} + pub struct SkinManager { /// Allocated atlas row per player uuid (the value passed to the model). rows: HashMap<[u8; 16], u32>, /// Current owner uuid of each atlas row, so recycling a row can evict the /// stale uuid→row mapping (two uuids must never resolve to one row). row_owner: Vec>, + /// Last `poll` frame each row's owner was wanted on screen; the least + /// recently wanted row is recycled first. + row_used: Vec, + frame: u64, + /// Downloaded skins by uuid, resident in a row or not. + skins: HashMap<[u8; 16], Vec>, /// uuids already requested (downloading or done) — avoids re-fetching. requested: HashSet<[u8; 16]>, - /// Next free row in the per-player region, cycling round-robin. - next_row: u32, - tx: Sender, + jobs: Sender, rx: Receiver, } @@ -42,12 +61,38 @@ impl Default for SkinManager { impl SkinManager { pub fn new() -> Self { let (tx, rx) = mpsc::channel(); + let (jobs, job_rx) = mpsc::channel::(); + let job_rx = Arc::new(Mutex::new(job_rx)); + let agent = ureq::AgentBuilder::new().timeout(DOWNLOAD_TIMEOUT).build(); + for index in 0..DOWNLOAD_THREADS { + let job_rx = Arc::clone(&job_rx); + let tx: Sender = tx.clone(); + let agent = agent.clone(); + let spawned = thread::Builder::new() + .name(format!("skin-download-{index}")) + .spawn(move || loop { + let job = match job_rx.lock().unwrap_or_else(|e| e.into_inner()).recv() { + Ok(job) => job, + Err(_) => break, // manager dropped + }; + if let Some(rgba) = download_skin(&agent, &job.property) { + if tx.send(LoadedSkin { uuid: job.uuid, rgba }).is_err() { + break; + } + } + }); + if let Err(err) = spawned { + log::warn!("skin download thread failed to start: {err}"); + } + } Self { rows: HashMap::new(), row_owner: vec![None; PLAYER_SKIN_SLOTS as usize], + row_used: vec![0; PLAYER_SKIN_SLOTS as usize], + frame: 0, + skins: HashMap::new(), requested: HashSet::new(), - next_row: 0, - tx, + jobs, rx, } } @@ -57,42 +102,102 @@ impl SkinManager { self.requested.contains(uuid) } - /// Kick off a background download for `uuid` from its base64 `textures` + /// Queue a background download for `uuid` from its base64 `textures` /// property. No-op if already requested. pub fn request(&mut self, uuid: [u8; 16], texture_property: &str) { if !self.requested.insert(uuid) { return; } - let property = texture_property.to_owned(); - let tx = self.tx.clone(); - thread::spawn(move || { - if let Some(rgba) = download_skin(&property) { - let _ = tx.send(LoadedSkin { uuid, rgba }); - } - }); + let job = DownloadJob { + uuid, + property: texture_property.to_owned(), + }; + if self.jobs.send(job).is_err() { + log::warn!("skin download threads are gone; skin not fetched"); + } } - /// Upload any completed downloads into their atlas rows and record the - /// uuid→row mapping. Call once per frame with the renderer. - pub fn poll(&mut self, renderer: &mut Renderer) { + /// Take finished downloads and keep the atlas rows assigned to the skins in + /// use. `wanted` are the uuids drawn this frame (spawned players, skulls, + /// the local player): each gets a row, recycling the least recently wanted + /// one when all are taken. Other downloaded skins only take free rows. + /// Call once per frame; `upload` writes a skin into an atlas row. + pub fn poll(&mut self, upload: &mut dyn FnMut(u32, &[u8]), wanted: &HashSet<[u8; 16]>) { + self.frame += 1; + let mut fresh = Vec::new(); while let Ok(skin) = self.rx.try_recv() { - let row = match self.rows.get(&skin.uuid) { - Some(&row) => row, - None => { - let row = self.next_row; - self.next_row = (self.next_row + 1) % PLAYER_SKIN_SLOTS; - // Recycling a row: drop the previous owner's mapping so it - // never samples another player's skin (it falls back to the - // default player slot until re-fetched). - if let Some(old) = self.row_owner[row as usize].take() { - self.rows.remove(&old); - } - self.row_owner[row as usize] = Some(skin.uuid); - self.rows.insert(skin.uuid, row); - row - } - }; - renderer.upload_player_skin(row, &skin.rgba); + if let Some(&row) = self.rows.get(&skin.uuid) { + upload(row, &skin.rgba); + } else { + fresh.push(skin.uuid); + } + self.skins.insert(skin.uuid, skin.rgba); + } + for uuid in wanted { + if let Some(&row) = self.rows.get(uuid) { + self.row_used[row as usize] = self.frame; + } + } + for uuid in wanted { + if !self.rows.contains_key(uuid) && self.skins.contains_key(uuid) { + let Some(row) = self.pick_row(true) else { break }; + self.assign(upload, row, *uuid); + } + } + for uuid in fresh { + if !self.rows.contains_key(&uuid) { + let Some(row) = self.pick_row(false) else { break }; + self.assign(upload, row, uuid); + } + } + self.trim_cache(); + } + + /// A free row, or (if `recycle`) the least recently wanted row not wanted + /// this frame. + fn pick_row(&self, recycle: bool) -> Option { + if let Some(free) = self.row_owner.iter().position(Option::is_none) { + return Some(free as u32); + } + if !recycle { + return None; + } + (0..self.row_owner.len()) + .filter(|&row| self.row_used[row] != self.frame) + .min_by_key(|&row| self.row_used[row]) + .map(|row| row as u32) + } + + fn assign(&mut self, upload: &mut dyn FnMut(u32, &[u8]), row: u32, uuid: [u8; 16]) { + let Some(rgba) = self.skins.get(&uuid) else { return }; + // Recycling a row: drop the previous owner's mapping so it never + // samples another player's skin. Its decoded skin stays cached, so it + // gets a row back (without a download) once it is wanted again. + if let Some(old) = self.row_owner[row as usize].replace(uuid) { + self.rows.remove(&old); + } + self.row_used[row as usize] = self.frame; + self.rows.insert(uuid, row); + upload(row, rgba); + } + + /// Bound the decoded-skin cache: drop skins not resident in a row and + /// forget they were requested, so they are fetched again if needed. + fn trim_cache(&mut self) { + if self.skins.len() <= MAX_CACHED_SKINS { + return; + } + let excess = self.skins.len() - MAX_CACHED_SKINS; + let drop: Vec<[u8; 16]> = self + .skins + .keys() + .filter(|uuid| !self.rows.contains_key(*uuid)) + .take(excess) + .copied() + .collect(); + for uuid in drop { + self.skins.remove(&uuid); + self.requested.remove(&uuid); } } @@ -120,10 +225,11 @@ fn skin_url_from_property(texture_property: &str) -> Option { /// Decode the `textures` property, download the SKIN PNG and normalize it to a /// 64×64 RGBA skin. -fn download_skin(texture_property: &str) -> Option> { +fn download_skin(agent: &ureq::Agent, texture_property: &str) -> Option> { let url = skin_url_from_property(texture_property)?; let mut bytes = Vec::new(); - ureq::get(&url) + agent + .get(&url) .call() .ok()? .into_reader() @@ -137,6 +243,50 @@ fn download_skin(texture_property: &str) -> Option> { mod tests { use super::*; + fn uuid(n: u32) -> [u8; 16] { + let mut id = [0u8; 16]; + id[..4].copy_from_slice(&n.to_le_bytes()); + id + } + + /// With more players than atlas rows, rows go to the players on screen; a + /// player whose row was recycled gets one back from the cache (no + /// re-download) once it is wanted again, and nothing churns while the + /// wanted set is stable. + #[test] + fn recycled_rows_return_to_wanted_players() { + let slots = PLAYER_SKIN_SLOTS; + let mut manager = SkinManager::new(); + for n in 0..slots + 10 { + manager.skins.insert(uuid(n), vec![0; 4]); + manager.requested.insert(uuid(n)); + } + let mut uploads = 0; + let first: HashSet<_> = (0..slots).map(uuid).collect(); + manager.poll(&mut |_, _| uploads += 1, &first); + assert_eq!(uploads, slots); + assert!(first.iter().all(|id| manager.rows().contains_key(id))); + + // Ten newcomers on screen take the rows of ten players who left. + let second: HashSet<_> = (10..slots + 10).map(uuid).collect(); + manager.poll(&mut |_, _| uploads += 1, &second); + assert_eq!(uploads, slots + 10); + assert!(second.iter().all(|id| manager.rows().contains_key(id))); + assert!(!manager.rows().contains_key(&uuid(0))); + + // Stable wanted set: no further uploads. + manager.poll(&mut |_, _| uploads += 1, &second); + assert_eq!(uploads, slots + 10); + + // A recycled player comes back into view and gets a row again. + let mut third = second.clone(); + third.remove(&uuid(slots + 9)); + third.insert(uuid(0)); + manager.poll(&mut |_, _| uploads += 1, &third); + assert!(manager.rows().contains_key(&uuid(0))); + assert!(manager.is_requested(&uuid(0)), "served from cache, not re-fetched"); + } + #[test] fn extracts_skin_url_from_a_textures_property() { let json = r#"{"timestamp":1,"profileId":"abc","profileName":"Notch", From c1ee7c078ad11c0e631abf92b25b7ee8566e6e1a Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:14:01 -0700 Subject: [PATCH 09/15] fix(ext): interrupt runaway JS hooks and cap the JS heap JS mod hooks run synchronously on the render thread with no interrupt handler or memory limit, so a while(true) or unbounded allocation in a mod froze or exhausted the client. Install a QuickJS interrupt handler that aborts a hook after 50 ms (2 s for top-level code at load); an aborted or out-of-memory hook panics so the manager disables the mod as it does for other faulting hooks. Enforce a 128 MiB heap cap through a counting allocator, since rquickjs ignores set_memory_limit with rust-alloc. --- crates/fpsmaster_ext/src/js.rs | 193 +++++++++++++++++++++++++++----- crates/fpsmaster_ext/src/lib.rs | 34 ++++++ 2 files changed, 198 insertions(+), 29 deletions(-) diff --git a/crates/fpsmaster_ext/src/js.rs b/crates/fpsmaster_ext/src/js.rs index 793ac8e..0e3db1e 100644 --- a/crates/fpsmaster_ext/src/js.rs +++ b/crates/fpsmaster_ext/src/js.rs @@ -7,12 +7,15 @@ //! ergonomic `mc.*` / `hud.*` API (modelled on vanilla `mc.player` / `mc.world`) //! is built on top in the JS [`PRELUDE`]. +use std::cell::Cell; use std::path::Path; +use std::rc::Rc; +use rquickjs::allocator::{Allocator, RustAllocator}; use rquickjs::{CatchResultExt, Context, Function, Runtime}; use std::sync::OnceLock; -use std::time::Instant; +use std::time::{Duration, Instant}; use crate::bridge::{self, cur}; @@ -219,15 +222,117 @@ const PRELUDE: &str = r#" })(); "#; +/// Wall-clock budget for one hook dispatch. Hooks run synchronously on the +/// render thread, so a mod stuck in `while(true)` froze the whole client. +const HOOK_BUDGET: Duration = Duration::from_millis(50); +/// Budget for evaluating a mod's top-level source at load. +const LOAD_BUDGET: Duration = Duration::from_secs(2); +/// Cap on the JS heap shared by all JS mods. +const JS_HEAP_LIMIT: usize = 128 * 1024 * 1024; + +/// Deadline for the JS call in progress, read by the runtime's interrupt +/// handler. `tripped` records that the handler aborted a call. +#[derive(Default)] +struct Watchdog { + deadline: Cell>, + tripped: Cell, +} + +impl Watchdog { + /// Run `f` with a deadline `budget` from now; returns its result and + /// whether the interrupt handler had to abort it. + fn run(&self, budget: Duration, f: impl FnOnce() -> R) -> (R, bool) { + self.tripped.set(false); + self.deadline.set(Some(Instant::now() + budget)); + let result = f(); + self.deadline.set(None); + (result, self.tripped.take()) + } + + fn expired(&self) -> bool { + let expired = self.deadline.get().is_some_and(|d| Instant::now() >= d); + if expired { + self.tripped.set(true); + } + expired + } +} + +/// The Rust global allocator with a cap on live bytes. rquickjs ignores +/// `Runtime::set_memory_limit` under its `rust-alloc` feature, so the limit is +/// enforced here: past it allocations fail and QuickJS throws "out of memory" +/// in the mod that asked, instead of the heap growing until the process dies. +struct LimitedAllocator { + used: usize, + limit: usize, +} + +unsafe impl Allocator for LimitedAllocator { + fn alloc(&mut self, size: usize) -> *mut u8 { + if self.used.saturating_add(size) > self.limit { + return std::ptr::null_mut(); + } + let ptr = RustAllocator.alloc(size); + if !ptr.is_null() { + self.used += unsafe { RustAllocator::usable_size(ptr) }; + } + ptr + } + + fn calloc(&mut self, count: usize, size: usize) -> *mut u8 { + if self.used.saturating_add(count.saturating_mul(size)) > self.limit { + return std::ptr::null_mut(); + } + let ptr = RustAllocator.calloc(count, size); + if !ptr.is_null() { + self.used += unsafe { RustAllocator::usable_size(ptr) }; + } + ptr + } + + unsafe fn dealloc(&mut self, ptr: *mut u8) { + self.used -= RustAllocator::usable_size(ptr); + RustAllocator.dealloc(ptr); + } + + unsafe fn realloc(&mut self, ptr: *mut u8, new_size: usize) -> *mut u8 { + if ptr.is_null() { + return self.alloc(new_size); + } + let old = RustAllocator::usable_size(ptr); + if new_size > old && (self.used - old).saturating_add(new_size) > self.limit { + return std::ptr::null_mut(); + } + let new = RustAllocator.realloc(ptr, new_size); + if !new.is_null() { + self.used = self.used - old + RustAllocator::usable_size(new); + } + new + } + + unsafe fn usable_size(ptr: *mut u8) -> usize { + RustAllocator::usable_size(ptr) + } +} + /// Owns the shared QuickJS runtime. Mod contexts are created from it. pub struct JsRuntime { rt: Runtime, + watchdog: Rc, } impl JsRuntime { pub fn new() -> Result { - let rt = Runtime::new().map_err(JsError::from_rquickjs)?; - Ok(Self { rt }) + Self::with_heap_limit(JS_HEAP_LIMIT) + } + + fn with_heap_limit(limit: usize) -> Result { + let rt = Runtime::new_with_alloc(LimitedAllocator { used: 0, limit }) + .map_err(JsError::from_rquickjs)?; + let watchdog = Rc::new(Watchdog::default()); + let handler_watchdog = Rc::clone(&watchdog); + rt.set_interrupt_handler(Some(Box::new(move || handler_watchdog.expired()))); + Ok(Self { rt, watchdog }) } /// Build a [`JsPlugin`] from a mod id, its source and its directory. @@ -271,14 +376,23 @@ impl JsRuntime { ctx.eval::<(), _>(PRELUDE) .catch(&ctx) .map_err(|e| JsError::Eval(e.to_string()))?; - ctx.eval::<(), _>(source) - .catch(&ctx) - .map_err(|e| JsError::Eval(e.to_string()))?; - Ok(()) + let (result, tripped) = self.watchdog.run(LOAD_BUDGET, || { + ctx.eval::<(), _>(source) + .catch(&ctx) + .map_err(|e| JsError::Eval(e.to_string())) + }); + if tripped { + return Err(JsError::Eval(format!( + "top-level code ran longer than {} ms", + LOAD_BUDGET.as_millis() + ))); + } + result })?; Ok(JsPlugin { id: id.to_string(), ctx, + watchdog: Rc::clone(&self.watchdog), }) } } @@ -287,39 +401,60 @@ impl JsRuntime { pub struct JsPlugin { id: String, ctx: Context, + watchdog: Rc, } impl JsPlugin { fn call_void(&self, name: &str, json: Option<&str>) { - let id = &self.id; - self.ctx.with(|ctx| { - let Ok(f) = ctx.globals().get::<_, Function>(name) else { - return; - }; - let res = match json { - Some(j) => f.call::<_, ()>((j.to_string(),)).catch(&ctx), - None => f.call::<_, ()>(()).catch(&ctx), - }; - if let Err(err) = res { - log::error!("[ext:{id}] dispatcher {name}: {err}"); - } + self.call(name, |ctx, f| match json { + Some(j) => f.call::<_, ()>((j.to_string(),)).catch(ctx).map_err(|e| e.to_string()), + None => f.call::<_, ()>(()).catch(ctx).map_err(|e| e.to_string()), }); } fn call_bool(&self, name: &str, json: &str) -> bool { + self.call(name, |ctx, f| { + f.call::<_, bool>((json.to_string(),)) + .catch(ctx) + .map_err(|e| e.to_string()) + }) + .unwrap_or(false) + } + + /// Call global dispatcher `name` under the hook budget. Errors are logged. + /// A call aborted for overrunning the budget, or one that exhausted the JS + /// heap, leaves the mod in an unknown state: it panics so the manager's + /// `catch_unwind` disables the mod, like any other faulting hook. + fn call( + &self, + name: &str, + f: impl FnOnce(&rquickjs::Ctx<'_>, Function<'_>) -> Result, + ) -> Option { let id = &self.id; - self.ctx.with(|ctx| { - let Ok(f) = ctx.globals().get::<_, Function>(name) else { - return false; - }; - match f.call::<_, bool>((json.to_string(),)).catch(&ctx) { - Ok(b) => b, - Err(err) => { - log::error!("[ext:{id}] dispatcher {name}: {err}"); - false + let (result, tripped) = self.watchdog.run(HOOK_BUDGET, || { + self.ctx.with(|ctx| { + let func = ctx.globals().get::<_, Function>(name).ok()?; + Some(f(&ctx, func)) + }) + }); + let result = result?; + if tripped { + log::error!( + "[ext:{id}] {name} ran longer than {} ms and was interrupted", + HOOK_BUDGET.as_millis() + ); + std::panic::panic_any(format!("js mod '{id}' exceeded its hook time budget")); + } + match result { + Ok(value) => Some(value), + Err(err) => { + log::error!("[ext:{id}] dispatcher {name}: {err}"); + if err.contains("out of memory") { + std::panic::panic_any(format!("js mod '{id}' exhausted the JS heap")); } + None } - }) + } } } diff --git a/crates/fpsmaster_ext/src/lib.rs b/crates/fpsmaster_ext/src/lib.rs index f53edb7..720f569 100644 --- a/crates/fpsmaster_ext/src/lib.rs +++ b/crates/fpsmaster_ext/src/lib.rs @@ -256,6 +256,40 @@ mod tests { mgr } + /// A JS hook stuck in an endless loop is interrupted after its time budget + /// (the JS try/catch around handlers cannot swallow it) and the mod is + /// disabled instead of freezing the render thread. + #[test] + fn js_endless_hook_is_interrupted_and_disabled() { + let mut mgr = js_mod("mc.on('tick', () => { while (true) {} });"); + let start = std::time::Instant::now(); + mgr.dispatch_tick(&MockViews); + assert!(start.elapsed() < std::time::Duration::from_secs(5)); + assert!(!mgr.mods()[0].enabled, "runaway mod must be disabled"); + // Later dispatches skip it. + mgr.dispatch_tick(&MockViews); + } + + #[test] + fn js_endless_top_level_code_fails_to_load() { + let rt = js::JsRuntime::new().expect("js runtime"); + let start = std::time::Instant::now(); + assert!(rt.load("spin", "for (;;) {}", std::path::Path::new(".")).is_err()); + assert!(start.elapsed() < std::time::Duration::from_secs(10)); + } + + /// A mod that keeps growing its heap hits the JS heap cap instead of + /// taking the process down with it. + #[test] + fn js_heap_growth_is_capped() { + let mut mgr = js_mod( + "const keep = []; mc.on('tick', () => { for (let i = 0; i < 64; i++) keep.push(new Array(1 << 20).fill(1)); });", + ); + for _ in 0..64 { + mgr.dispatch_tick(&MockViews); + } + } + #[test] fn js_tick_reads_views_and_enqueues_chat() { let mut mgr = From db731a165f3b8f685466db989c0e888b9cb62163 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:15:09 -0700 Subject: [PATCH 10/15] perf(render): coalesce freed chunk-buffer ranges and release empty pages ChunkLayer kept freed slots as separate (vertex, index) pairs that never merged, so long flights fragmented pages until new 128 MB pages were created, and pages were never freed. Allocate each page's vertex and index ranges with a first-fit allocator that coalesces neighbours and rewinds the bump cursor, place sections on the lowest page with room, release trailing empty pages, and keep only one page on clear(). --- crates/fpsmaster_render/src/renderer/mod.rs | 202 +++++++++++++------- 1 file changed, 131 insertions(+), 71 deletions(-) diff --git a/crates/fpsmaster_render/src/renderer/mod.rs b/crates/fpsmaster_render/src/renderer/mod.rs index e5a8953..5e82d85 100644 --- a/crates/fpsmaster_render/src/renderer/mod.rs +++ b/crates/fpsmaster_render/src/renderer/mod.rs @@ -308,11 +308,70 @@ const CHUNK_VERTEX_SIZE: u64 = std::mem::size_of::() as u64; const PAGE_VERTEX_CAP: u32 = 1 << 22; // 4M vertices const PAGE_INDEX_CAP: u32 = 1 << 23; // 8M indices = 16 MB +/// First-fit range allocator over `[0, cap)` for one page buffer. Freed ranges +/// are coalesced with their neighbours, and a free range that reaches the bump +/// cursor is folded back into it, so churn (sections re-meshed at new sizes, +/// columns unloaded while flying) reuses space instead of fragmenting a page +/// until a fresh 128 MB page is needed. +struct RangeAlloc { + cap: u32, + cursor: u32, + /// Free ranges below `cursor`: offset -> length, never adjacent. + free: std::collections::BTreeMap, +} + +impl RangeAlloc { + fn new(cap: u32) -> Self { + Self { + cap, + cursor: 0, + free: std::collections::BTreeMap::new(), + } + } + + fn alloc(&mut self, len: u32) -> Option { + if let Some((&off, &free_len)) = self.free.iter().find(|(_, &l)| l >= len) { + self.free.remove(&off); + if free_len > len { + self.free.insert(off + len, free_len - len); + } + return Some(off); + } + if self.cap - self.cursor >= len { + let off = self.cursor; + self.cursor += len; + return Some(off); + } + None + } + + fn free(&mut self, mut off: u32, mut len: u32) { + if len == 0 { + return; + } + if let Some((&prev, &prev_len)) = self.free.range(..off).next_back() { + if prev + prev_len == off { + self.free.remove(&prev); + off = prev; + len += prev_len; + } + } + if let Some(next_len) = self.free.remove(&(off + len)) { + len += next_len; + } + if off + len == self.cursor { + self.cursor = off; + } else { + self.free.insert(off, len); + } + } +} + struct ChunkPage { vertex_buf: wgpu::Buffer, index_buf: wgpu::Buffer, - vertex_cursor: u32, - index_cursor: u32, + vertices: RangeAlloc, + indices: RangeAlloc, active_slots: u32, } @@ -320,13 +379,13 @@ struct ChunkPage { /// fixed-size pages (64 MB vertex, 16 MB index each) to stay within GPU /// buffer-size limits while still batching draws. /// -/// Freed slot regions are tracked in a free list so new allocations can reuse -/// them instead of always advancing the page cursor. When a page has zero -/// active slots its cursors reset, reclaiming the entire page. +/// Each page allocates its vertex and index ranges with a coalescing +/// [`RangeAlloc`]; new sections go to the lowest page with room. Empty pages +/// at the end of the list are released (page 0 is kept), so VRAM shrinks back +/// after a large world is left behind. struct ChunkLayer { pages: Vec, slots: HashMap, - free_list: Vec, label: &'static str, } @@ -335,7 +394,6 @@ impl ChunkLayer { Self { pages: Vec::new(), slots: HashMap::new(), - free_list: Vec::new(), label, } } @@ -354,12 +412,24 @@ impl ChunkLayer { usage: wgpu::BufferUsages::INDEX | wgpu::BufferUsages::COPY_DST, mapped_at_creation: false, }), - vertex_cursor: 0, - index_cursor: 0, + vertices: RangeAlloc::new(PAGE_VERTEX_CAP), + indices: RangeAlloc::new(PAGE_INDEX_CAP), active_slots: 0, } } + /// Reserve `vc` vertices and `ic` indices on one page. + fn page_alloc(page: &mut ChunkPage, vc: u32, ic: u32) -> Option<(u32, u32)> { + let vo = page.vertices.alloc(vc)?; + match page.indices.alloc(ic) { + Some(io) => Some((vo, io)), + None => { + page.vertices.free(vo, vc); + None + } + } + } + fn insert( &mut self, device: &wgpu::Device, @@ -398,58 +468,28 @@ impl ChunkLayer { self.free_slot(old); } - // Try the free list (best-fit: smallest region that fits both dimensions). - let fit = self - .free_list - .iter() - .enumerate() - .filter(|(_, f)| f.vertex_alloc >= vc && f.index_alloc >= ic) - .min_by_key(|(_, f)| (f.vertex_alloc, f.index_alloc)) - .map(|(i, _)| i); - if let Some(fi) = fit { - let free = self.free_list.swap_remove(fi); - let page = &self.pages[free.page as usize]; - queue.write_buffer( - &page.vertex_buf, - free.vertex_offset as u64 * CHUNK_VERTEX_SIZE, - bytemuck::cast_slice(&buf.vertices), - ); - queue.write_buffer( - &page.index_buf, - free.index_offset as u64 * 2, - bytemuck::cast_slice(&buf.indices), - ); - self.pages[free.page as usize].active_slots += 1; - self.slots.insert( - pos, - LayerSlot { - page: free.page, - vertex_offset: free.vertex_offset, - vertex_alloc: free.vertex_alloc, - index_offset: free.index_offset, - index_alloc: free.index_alloc, - index_count: ic, - }, - ); - return; + // Lowest page with room, or a new one. + let mut placed = None; + for (pi, page) in self.pages.iter_mut().enumerate() { + if let Some(offsets) = Self::page_alloc(page, vc, ic) { + placed = Some((pi, offsets)); + break; + } } - - // Find a page with enough space, or create one. - let pi = self - .pages - .iter() - .position(|p| { - p.vertex_cursor + vc <= PAGE_VERTEX_CAP - && p.index_cursor + ic <= PAGE_INDEX_CAP - }) - .unwrap_or_else(|| { - self.pages.push(Self::create_page(device, self.label)); - self.pages.len() - 1 - }); + let (pi, (vo, io)) = match placed { + Some(placed) => placed, + None => { + let mut page = Self::create_page(device, self.label); + let Some(offsets) = Self::page_alloc(&mut page, vc, ic) else { + log::error!("section {pos:?} mesh exceeds a whole chunk page; not drawn"); + return; + }; + self.pages.push(page); + (self.pages.len() - 1, offsets) + } + }; let page = &mut self.pages[pi]; - let vo = page.vertex_cursor; - let io = page.index_cursor; queue.write_buffer( &page.vertex_buf, vo as u64 * CHUNK_VERTEX_SIZE, @@ -472,18 +512,16 @@ impl ChunkLayer { index_count: ic, }, ); - page.vertex_cursor += vc; - page.index_cursor += ic; } fn free_slot(&mut self, slot: LayerSlot) { - self.pages[slot.page as usize].active_slots -= 1; - if self.pages[slot.page as usize].active_slots == 0 { - self.pages[slot.page as usize].vertex_cursor = 0; - self.pages[slot.page as usize].index_cursor = 0; - self.free_list.retain(|f| f.page != slot.page); - } else { - self.free_list.push(slot); + let page = &mut self.pages[slot.page as usize]; + page.vertices.free(slot.vertex_offset, slot.vertex_alloc); + page.indices.free(slot.index_offset, slot.index_alloc); + page.active_slots -= 1; + // Release trailing empty pages (indices of the others stay valid). + while self.pages.len() > 1 && self.pages.last().is_some_and(|p| p.active_slots == 0) { + self.pages.pop(); } } @@ -495,10 +533,10 @@ impl ChunkLayer { fn clear(&mut self) { self.slots.clear(); - self.free_list.clear(); + self.pages.truncate(1); for page in &mut self.pages { - page.vertex_cursor = 0; - page.index_cursor = 0; + page.vertices = RangeAlloc::new(PAGE_VERTEX_CAP); + page.indices = RangeAlloc::new(PAGE_INDEX_CAP); page.active_slots = 0; } } @@ -10910,7 +10948,7 @@ impl Renderer { #[cfg(test)] mod tests { - use super::{collect_layer_batches, glint_scroll, halton, ChunkLayer, LayerSlot}; + use super::{collect_layer_batches, glint_scroll, halton, ChunkLayer, LayerSlot, RangeAlloc}; use fpsmaster_core::SectionPos; /// A layer whose sections land on the given pages. `index_offset` doubles as @@ -10936,6 +10974,28 @@ mod tests { (layer, visible) } + /// Freed neighbours coalesce, and space freed at the end rolls the cursor + /// back, so a page fully drained returns to empty and a hole can host an + /// allocation as large as the merged neighbours. + #[test] + fn range_alloc_coalesces_and_rewinds() { + let mut a = RangeAlloc::new(100); + let x = a.alloc(10).unwrap(); + let y = a.alloc(20).unwrap(); + let z = a.alloc(30).unwrap(); + assert_eq!((x, y, z), (0, 10, 30)); + assert_eq!(a.alloc(41), None, "only 40 left"); + a.free(x, 10); + a.free(y, 20); + assert_eq!(a.free.get(&0), Some(&30), "neighbours merged into one hole"); + assert_eq!(a.alloc(25), Some(0)); + a.free(0, 25); + a.free(z, 30); + assert_eq!(a.cursor, 0, "freeing the tail rewinds the cursor"); + assert!(a.free.is_empty()); + assert_eq!(a.alloc(100), Some(0)); + } + /// Alpha blending is order-dependent, so the back-to-front section list the /// caller builds must survive batching. Gathering every section of a page /// into one batch (correct for the opaque layers) silently reorders them. From 595633d17a71c78a88e265802b5b2cd07fd63645 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:25:33 -0700 Subject: [PATCH 11/15] fix(ext): give each JS mod its own runtime and heap cap The 128 MiB heap cap was shared by every JS mod, so one mod leaking up to it made every other mod's allocations fail too. JsRuntime::load now consumes the runtime, and the manager creates one per mod: a leak is confined to the mod that leaks. --- crates/fpsmaster_ext/src/js.rs | 18 +++++++++------ crates/fpsmaster_ext/src/lib.rs | 34 ++++++++++++++++++++++++----- crates/fpsmaster_ext/src/manager.rs | 29 ++++++++---------------- 3 files changed, 48 insertions(+), 33 deletions(-) diff --git a/crates/fpsmaster_ext/src/js.rs b/crates/fpsmaster_ext/src/js.rs index 0e3db1e..c170291 100644 --- a/crates/fpsmaster_ext/src/js.rs +++ b/crates/fpsmaster_ext/src/js.rs @@ -1,6 +1,7 @@ //! JS extension backend (QuickJS via `rquickjs`). //! -//! Each `.js` mod gets its own QuickJS [`Context`] (isolation + hot reload). The +//! Each `.js` mod gets its own QuickJS [`Runtime`] and [`Context`] (isolation + +//! hot reload; the heap cap and hook watchdog are per mod). The //! Rust<->JS interchange is the shared all-JSON [`crate::bridge`] protocol: three //! native globals `__rcf_cmd` / `__rcf_query` / `__rcf_hud`, and the host calls //! global `__rcf_dispatch_*` functions to fan out to registered handlers. The @@ -227,7 +228,8 @@ const PRELUDE: &str = r#" const HOOK_BUDGET: Duration = Duration::from_millis(50); /// Budget for evaluating a mod's top-level source at load. const LOAD_BUDGET: Duration = Duration::from_secs(2); -/// Cap on the JS heap shared by all JS mods. +/// Cap on each JS mod's heap. Every mod has its own runtime, so one mod that +/// leaks up to the cap cannot push the others into out-of-memory. const JS_HEAP_LIMIT: usize = 128 * 1024 * 1024; /// Deadline for the JS call in progress, read by the runtime's interrupt @@ -315,7 +317,8 @@ unsafe impl Allocator for LimitedAllocator { } } -/// Owns the shared QuickJS runtime. Mod contexts are created from it. +/// A QuickJS runtime with its own heap cap and watchdog. [`JsRuntime::load`] +/// consumes it, so each mod gets a runtime of its own. pub struct JsRuntime { rt: Runtime, watchdog: Rc, @@ -326,7 +329,7 @@ impl JsRuntime { Self::with_heap_limit(JS_HEAP_LIMIT) } - fn with_heap_limit(limit: usize) -> Result { + pub(crate) fn with_heap_limit(limit: usize) -> Result { let rt = Runtime::new_with_alloc(LimitedAllocator { used: 0, limit }) .map_err(JsError::from_rquickjs)?; let watchdog = Rc::new(Watchdog::default()); @@ -339,7 +342,7 @@ impl JsRuntime { /// Registers the native globals, injects the mod's config dir + persisted /// `config.json` (for `mc.config`), evals the prelude, then evals the mod /// source (a syntax/runtime error at load is reported and the mod rejected). - pub fn load(&self, id: &str, source: &str, dir: &Path) -> Result { + pub fn load(self, id: &str, source: &str, dir: &Path) -> Result { let config_json = std::fs::read_to_string(dir.join("config.json")).unwrap_or_else(|_| "{}".to_string()); let dir_str = dir.to_string_lossy().to_string(); @@ -392,12 +395,13 @@ impl JsRuntime { Ok(JsPlugin { id: id.to_string(), ctx, - watchdog: Rc::clone(&self.watchdog), + watchdog: self.watchdog, }) } } -/// A single loaded JS mod (its own QuickJS context). +/// A single loaded JS mod (its own QuickJS runtime and context; the context +/// keeps the runtime alive). pub struct JsPlugin { id: String, ctx: Context, diff --git a/crates/fpsmaster_ext/src/lib.rs b/crates/fpsmaster_ext/src/lib.rs index 720f569..c5242d2 100644 --- a/crates/fpsmaster_ext/src/lib.rs +++ b/crates/fpsmaster_ext/src/lib.rs @@ -278,16 +278,38 @@ mod tests { assert!(start.elapsed() < std::time::Duration::from_secs(10)); } - /// A mod that keeps growing its heap hits the JS heap cap instead of - /// taking the process down with it. + /// A mod that keeps growing its heap hits its own heap cap instead of + /// taking the process down, and a healthy mod loaded alongside it keeps + /// running: the cap is per mod, not shared. #[test] - fn js_heap_growth_is_capped() { - let mut mgr = js_mod( - "const keep = []; mc.on('tick', () => { for (let i = 0; i < 64; i++) keep.push(new Array(1 << 20).fill(1)); });", - ); + fn js_heap_growth_is_capped_per_mod() { + let mut mgr = ExtManager::new(); + for (id, source) in [ + ( + "leaker", + "const keep = []; mc.on('tick', () => { for (let i = 0; i < 4096; i++) keep.push(new Array(1 << 10).fill(i)); });", + ), + ( + "healthy", + "mc.on('tick', () => { const a = new Array(1 << 16).fill(1); mc.log('ok', a.length); });", + ), + ] { + let rt = js::JsRuntime::with_heap_limit(16 << 20).expect("js runtime"); + let plugin = rt.load(id, source, std::path::Path::new(".")).expect("load js mod"); + mgr.register(Box::new(plugin)); + } for _ in 0..64 { mgr.dispatch_tick(&MockViews); } + assert!(mgr.mods()[1].enabled, "a healthy mod must not pay for another mod's leak"); + mgr.take_commands(); + mgr.dispatch_tick(&MockViews); + let cmds = mgr.take_commands(); + assert!( + cmds.iter() + .any(|c| matches!(c, ExtCommand::Log(_, s) if s.contains("ok 65536"))), + "healthy mod should still run: {cmds:?}" + ); } #[test] diff --git a/crates/fpsmaster_ext/src/manager.rs b/crates/fpsmaster_ext/src/manager.rs index a31c50c..6e8bd01 100644 --- a/crates/fpsmaster_ext/src/manager.rs +++ b/crates/fpsmaster_ext/src/manager.rs @@ -62,7 +62,6 @@ pub struct ModInfo { pub struct ExtManager { plugins: Vec, commands: Vec, - js: Option, } impl ExtManager { @@ -96,11 +95,10 @@ impl ExtManager { }); } - /// Hot-reload directory mods: drop everything loaded from `mods/` (and the JS - /// runtime), then reload from disk. In-process registrations are kept. + /// Hot-reload directory mods: drop everything loaded from `mods/` (with their + /// JS runtimes), then reload from disk. In-process registrations are kept. pub fn reload_mods(&mut self, dir: &Path) -> Vec { self.plugins.retain(|p| !p.from_dir); - self.js = None; // drops all JS contexts; load_mods recreates the runtime log::info!("[ext] hot-reloading mods from {}", dir.display()); self.load_mods(dir) } @@ -146,9 +144,13 @@ impl ExtManager { continue; } }; - let rt = match self.js_runtime() { - Some(rt) => rt, - None => continue, + // One runtime per mod: its heap cap and watchdog are its own. + let rt = match js::JsRuntime::new() { + Ok(rt) => rt, + Err(e) => { + log::error!("[ext] could not start a JS runtime for '{}': {e}", manifest.id); + continue; + } }; match rt.load(&manifest.id, &source, path) { Ok(plugin) => { @@ -175,19 +177,6 @@ impl ExtManager { loaded } - fn js_runtime(&mut self) -> Option<&js::JsRuntime> { - if self.js.is_none() { - match js::JsRuntime::new() { - Ok(rt) => self.js = Some(rt), - Err(e) => { - log::error!("[ext] could not start the JS runtime: {e}"); - return None; - } - } - } - self.js.as_ref() - } - pub fn plugin_count(&self) -> usize { self.plugins.len() } From 43c140b32f697474ee4fcbbabee8e2731a2bf326 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 20:26:26 -0700 Subject: [PATCH 12/15] fix(ext): disable a JS mod whenever its heap cap refuses an allocation QuickJS raises out-of-memory as a catchable exception and the prelude wraps every handler in try/catch, so the string check on the dispatcher error almost never fired: a mod at its cap stayed enabled, spinning on failed allocations every hook. The allocator now records a refusal and JsPlugin::call disables the mod on it. --- crates/fpsmaster_ext/src/js.rs | 50 +++++++++++++++++++++++++-------- crates/fpsmaster_ext/src/lib.rs | 8 ++++-- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/crates/fpsmaster_ext/src/js.rs b/crates/fpsmaster_ext/src/js.rs index c170291..4c8a7cb 100644 --- a/crates/fpsmaster_ext/src/js.rs +++ b/crates/fpsmaster_ext/src/js.rs @@ -264,15 +264,25 @@ impl Watchdog { /// `Runtime::set_memory_limit` under its `rust-alloc` feature, so the limit is /// enforced here: past it allocations fail and QuickJS throws "out of memory" /// in the mod that asked, instead of the heap growing until the process dies. +/// `refused` records that the cap was hit: the prelude's per-handler try/catch +/// swallows the JS exception, so the error alone does not reach the host. struct LimitedAllocator { used: usize, limit: usize, + refused: Rc>, +} + +impl LimitedAllocator { + fn refuse(&self) -> *mut u8 { + self.refused.set(true); + std::ptr::null_mut() + } } unsafe impl Allocator for LimitedAllocator { fn alloc(&mut self, size: usize) -> *mut u8 { if self.used.saturating_add(size) > self.limit { - return std::ptr::null_mut(); + return self.refuse(); } let ptr = RustAllocator.alloc(size); if !ptr.is_null() { @@ -283,7 +293,7 @@ unsafe impl Allocator for LimitedAllocator { fn calloc(&mut self, count: usize, size: usize) -> *mut u8 { if self.used.saturating_add(count.saturating_mul(size)) > self.limit { - return std::ptr::null_mut(); + return self.refuse(); } let ptr = RustAllocator.calloc(count, size); if !ptr.is_null() { @@ -303,7 +313,7 @@ unsafe impl Allocator for LimitedAllocator { } let old = RustAllocator::usable_size(ptr); if new_size > old && (self.used - old).saturating_add(new_size) > self.limit { - return std::ptr::null_mut(); + return self.refuse(); } let new = RustAllocator.realloc(ptr, new_size); if !new.is_null() { @@ -322,6 +332,7 @@ unsafe impl Allocator for LimitedAllocator { pub struct JsRuntime { rt: Runtime, watchdog: Rc, + heap_refused: Rc>, } impl JsRuntime { @@ -330,12 +341,21 @@ impl JsRuntime { } pub(crate) fn with_heap_limit(limit: usize) -> Result { - let rt = Runtime::new_with_alloc(LimitedAllocator { used: 0, limit }) - .map_err(JsError::from_rquickjs)?; + let heap_refused = Rc::new(Cell::new(false)); + let rt = Runtime::new_with_alloc(LimitedAllocator { + used: 0, + limit, + refused: Rc::clone(&heap_refused), + }) + .map_err(JsError::from_rquickjs)?; let watchdog = Rc::new(Watchdog::default()); let handler_watchdog = Rc::clone(&watchdog); rt.set_interrupt_handler(Some(Box::new(move || handler_watchdog.expired()))); - Ok(Self { rt, watchdog }) + Ok(Self { + rt, + watchdog, + heap_refused, + }) } /// Build a [`JsPlugin`] from a mod id, its source and its directory. @@ -396,6 +416,7 @@ impl JsRuntime { id: id.to_string(), ctx, watchdog: self.watchdog, + heap_refused: self.heap_refused, }) } } @@ -406,6 +427,8 @@ pub struct JsPlugin { id: String, ctx: Context, watchdog: Rc, + /// Set by the runtime's allocator when it refuses an allocation at the cap. + heap_refused: Rc>, } impl JsPlugin { @@ -426,21 +449,27 @@ impl JsPlugin { } /// Call global dispatcher `name` under the hook budget. Errors are logged. - /// A call aborted for overrunning the budget, or one that exhausted the JS - /// heap, leaves the mod in an unknown state: it panics so the manager's - /// `catch_unwind` disables the mod, like any other faulting hook. + /// A call aborted for overrunning the budget, or one that hit the mod's heap + /// cap (even if a JS try/catch swallowed the error), leaves the mod in an + /// unknown state: it panics so the manager's `catch_unwind` disables the + /// mod, like any other faulting hook. fn call( &self, name: &str, f: impl FnOnce(&rquickjs::Ctx<'_>, Function<'_>) -> Result, ) -> Option { let id = &self.id; + self.heap_refused.set(false); let (result, tripped) = self.watchdog.run(HOOK_BUDGET, || { self.ctx.with(|ctx| { let func = ctx.globals().get::<_, Function>(name).ok()?; Some(f(&ctx, func)) }) }); + if self.heap_refused.take() { + log::error!("[ext:{id}] {name} hit the JS heap cap"); + std::panic::panic_any(format!("js mod '{id}' exhausted its JS heap")); + } let result = result?; if tripped { log::error!( @@ -453,9 +482,6 @@ impl JsPlugin { Ok(value) => Some(value), Err(err) => { log::error!("[ext:{id}] dispatcher {name}: {err}"); - if err.contains("out of memory") { - std::panic::panic_any(format!("js mod '{id}' exhausted the JS heap")); - } None } } diff --git a/crates/fpsmaster_ext/src/lib.rs b/crates/fpsmaster_ext/src/lib.rs index c5242d2..c62d94e 100644 --- a/crates/fpsmaster_ext/src/lib.rs +++ b/crates/fpsmaster_ext/src/lib.rs @@ -278,9 +278,10 @@ mod tests { assert!(start.elapsed() < std::time::Duration::from_secs(10)); } - /// A mod that keeps growing its heap hits its own heap cap instead of - /// taking the process down, and a healthy mod loaded alongside it keeps - /// running: the cap is per mod, not shared. + /// A mod that keeps growing its heap hits its own heap cap and is disabled + /// (although the prelude's try/catch swallows the JS out-of-memory error) + /// instead of taking the process down, and a healthy mod loaded alongside + /// it keeps running: the cap is per mod, not shared. #[test] fn js_heap_growth_is_capped_per_mod() { let mut mgr = ExtManager::new(); @@ -301,6 +302,7 @@ mod tests { for _ in 0..64 { mgr.dispatch_tick(&MockViews); } + assert!(!mgr.mods()[0].enabled, "the leaking mod is disabled at its cap"); assert!(mgr.mods()[1].enabled, "a healthy mod must not pay for another mod's leak"); mgr.take_commands(); mgr.dispatch_tick(&MockViews); From 81e59ca69d247a72139cfaddc982e81ce1e75dd3 Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 21:47:26 -0700 Subject: [PATCH 13/15] fix(sound): stream sounds.json entries flagged stream (records) Records carry "stream": true in vanilla's sounds.json. They went through the static path, so every play decoded a multi-minute ogg on the main thread, and since such clips are now too long to cache, each play did it again. Honor the flag and stream those entries like music. --- crates/fpsmaster_app/src/sound.rs | 53 +++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/crates/fpsmaster_app/src/sound.rs b/crates/fpsmaster_app/src/sound.rs index 896872e..8abaf89 100644 --- a/crates/fpsmaster_app/src/sound.rs +++ b/crates/fpsmaster_app/src/sound.rs @@ -159,6 +159,9 @@ struct SoundEntry { volume: f32, pitch: f32, weight: u32, + /// `"stream": true` (records): a multi-minute clip, streamed instead of + /// decoded whole on the main thread. + stream: bool, } /// One `sounds.json` event: its category (shared by all variants, vanilla puts @@ -295,21 +298,31 @@ impl SoundManager { return; } - let Some(data) = self.sound_data(&entry.name) else { - return; - }; // Vanilla clamps the combined pitch to [0.5, 2.0] (MCP-919 line 421). let rate = clamp_pitch(sound.pitch * entry.pitch) as f64; - let data = data - .volume(amplitude_to_decibels(gain)) - .panning(Panning(panning)) - .playback_rate(PlaybackRate(rate)); - - if let Some(manager) = self.manager.as_mut() { - // The handle is dropped immediately; the sound plays to completion. - if let Err(err) = manager.play(data) { - log::debug!("audio: play failed: {err}"); - } + // The handle is dropped immediately; the sound plays to completion. + let played = if entry.stream { + let Some(data) = open_ogg_stream(&self.asset_roots, &entry.name) else { + log::debug!("audio: missing sound asset {}", entry.name); + return; + }; + let data = data + .volume(amplitude_to_decibels(gain)) + .panning(Panning(panning)) + .playback_rate(PlaybackRate(rate)); + self.manager.as_mut().map(|m| m.play(data).map(drop).map_err(|e| e.to_string())) + } else { + let Some(data) = self.sound_data(&entry.name) else { + return; + }; + let data = data + .volume(amplitude_to_decibels(gain)) + .panning(Panning(panning)) + .playback_rate(PlaybackRate(rate)); + self.manager.as_mut().map(|m| m.play(data).map(drop).map_err(|e| e.to_string())) + }; + if let Some(Err(err)) = played { + log::debug!("audio: play failed: {err}"); } } @@ -563,6 +576,7 @@ fn parse_sounds_json(text: &str) -> HashMap { volume: 1.0, pitch: 1.0, weight: 1, + stream: false, }, serde_json::Value::Object(obj) => { let Some(name) = obj.get("name").and_then(|n| n.as_str()) else { @@ -573,6 +587,7 @@ fn parse_sounds_json(text: &str) -> HashMap { volume: obj.get("volume").and_then(|v| v.as_f64()).unwrap_or(1.0) as f32, pitch: obj.get("pitch").and_then(|v| v.as_f64()).unwrap_or(1.0) as f32, weight: obj.get("weight").and_then(|v| v.as_u64()).unwrap_or(1) as u32, + stream: obj.get("stream").and_then(|v| v.as_bool()).unwrap_or(false), } } _ => continue, @@ -756,6 +771,18 @@ mod tests { assert!((rabbit[0].volume - 0.25).abs() < 1e-6); } + /// Records are flagged `"stream": true` in vanilla's sounds.json; those + /// entries are streamed rather than decoded whole on the main thread. + #[test] + fn parses_stream_flag() { + let events = parse_sounds_json( + r#"{ "records.cat": { "category": "record", + "sounds": [{"name": "records/cat", "stream": true}] } }"#, + ); + assert!(events["records.cat"].entries[0].stream); + assert!(!parse_sounds_json(SAMPLE)["dig.stone"].entries[0].stream); + } + #[test] fn parses_event_category() { let events = parse_sounds_json(SAMPLE); From d5692e0abaa0f2c162fa014b1a3dd34d94ac714f Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Thu, 15 Oct 2026 21:47:53 -0700 Subject: [PATCH 14/15] perf(render): release empty chunk pages only on section removal insert() frees a section's old slot before placing its new mesh. When that section was alone on the last page, the page was released and a new 128 MB page created right after, on every re-mesh that grew it. Release trailing empty pages from remove() only. --- crates/fpsmaster_render/src/renderer/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/fpsmaster_render/src/renderer/mod.rs b/crates/fpsmaster_render/src/renderer/mod.rs index 5e82d85..707b787 100644 --- a/crates/fpsmaster_render/src/renderer/mod.rs +++ b/crates/fpsmaster_render/src/renderer/mod.rs @@ -519,15 +519,18 @@ impl ChunkLayer { page.vertices.free(slot.vertex_offset, slot.vertex_alloc); page.indices.free(slot.index_offset, slot.index_alloc); page.active_slots -= 1; - // Release trailing empty pages (indices of the others stay valid). - while self.pages.len() > 1 && self.pages.last().is_some_and(|p| p.active_slots == 0) { - self.pages.pop(); - } } fn remove(&mut self, pos: SectionPos) { if let Some(slot) = self.slots.remove(&pos) { self.free_slot(slot); + // Release trailing empty pages (indices of the others stay valid). + // Only here, not when `insert` frees a slot it is about to replace: + // a growing section alone on the last page would otherwise drop and + // immediately recreate a 128 MB page on every re-mesh. + while self.pages.len() > 1 && self.pages.last().is_some_and(|p| p.active_slots == 0) { + self.pages.pop(); + } } } From 5bc3f52981b2b1ee169d99e8c261694cfb35b33c Mon Sep 17 00:00:00 2001 From: gaoyu06 Date: Fri, 16 Oct 2026 03:13:15 -0700 Subject: [PATCH 15/15] fix(particle): cap total live particles across a burst of S2A packets The per-packet clamp did not bound the total: pump_network drains every queued packet in one frame and only tick() trimmed the Vec, so many huge-count packets could still push millions of particles. spawn now evicts the oldest up front so the live set never exceeds MAX_PARTICLES. --- crates/fpsmaster_app/src/particle.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/fpsmaster_app/src/particle.rs b/crates/fpsmaster_app/src/particle.rs index 2614a82..ce9c909 100644 --- a/crates/fpsmaster_app/src/particle.rs +++ b/crates/fpsmaster_app/src/particle.rs @@ -215,8 +215,13 @@ impl ParticleSystem { // `count` is a server-supplied i32: a hostile or buggy S2A with a huge // count would otherwise push millions of particles in one call (stall / // OOM) before `tick` trims to MAX_PARTICLES. Anything past the cap would - // be trimmed anyway, so clamp here. + // be trimmed anyway, so clamp here. The network pump drains every queued + // packet in one frame, so the per-packet clamp alone does not bound the + // total: also evict the oldest up front (the same rule as `tick`) so the + // live set never exceeds MAX_PARTICLES across a burst of packets. let n = (((base as f32) * self.density).round() as usize).min(MAX_PARTICLES); + let evict = (self.particles.len() + n).saturating_sub(MAX_PARTICLES); + self.particles.drain(0..evict); for _ in 0..n { let p_pos = if colour_carrying { pos @@ -668,6 +673,23 @@ mod tests { assert_eq!(sys.particles.len(), MAX_PARTICLES); } + /// Many huge packets in one frame (no `tick` in between) stay within the + /// cap in total, not just per packet. + #[test] + fn burst_of_huge_packets_is_capped_in_total() { + let mut sys = ParticleSystem::new(); + sys.set_density(4.0); + for _ in 0..50 { + sys.spawn(0, Vec3::ZERO, Vec3::ONE, 0.1, i32::MAX, &[]); + assert!(sys.particles.len() <= MAX_PARTICLES); + } + let marker = Vec3::splat(100.0); + sys.spawn(26, marker, Vec3::ZERO, 0.0, 1, &[]); + assert_eq!(sys.particles.len(), MAX_PARTICLES); + // The oldest are evicted, so the newest spawn survives. + assert_eq!(sys.particles.last().unwrap().pos, marker); + } + /// A flame's render scale shrinks as it ages (vanilla `1 - f*f*0.5`). #[test] fn flame_loses_scale_over_life() {