2026-06-11 08:52:33 +02:00
1 changed files with 20 additions and 0 deletions
@@ -0,0 +1,20 @@
 # Issues
 1. (Major) File bytes are JSON-encoded — ~3–4× wire bloat + heavy CPU. StreamInstallFrame::FileChunk { bytes: Bytes } goes over the wire via serde_json (the Message impl). serde_json has no native byte type, so Bytes serializes as a JSON array of integers ([12,255,7,…]) — roughly 3–4 ASCII chars per payload byte, plus serialize/parse cost on both ends. The existing transfer path (peer.rs::stream_file_bytes) sends raw bytes straight on the QUIC stream for exactly this reason. For a feature whose entire purpose is moving file bytes efficiently on constrained peers, routing the payload through JSON undermines the goal. The 3 MiB S39 fixture hides it; a multi-GB game would not. Consider a binary codec (postcard/bincode) for the frame stream, or keep the control frames as-is but stream raw bytes for file content.
 2. (Design) Solid archives → O(n²) extraction. Each file is pulled with a separate unrar p archive <file> (stream_unrar_file). For a solid .eti, unrar must re-decompress every preceding file on each invocation. solid is detected and sent in ArchiveBegin but never acted on. A solid multi-file archive could make streamed install pathologically slow. At minimum document the limitation; ideally a single-pass extraction for the solid case.
 3. (Robustness) Single peer, no fallback. handle_stream_install_game_command picks one peer (peers.sort(); peers.into_iter().next()). If that peer declines (can_serve_game false) or the connection drops, the whole install fails — no retry against the other validated peers, unlike the normal multi-peer download. Fine for a prototype, worth a TODO.
 4. (Minor) unrar p <name> is a wildcard mask. unrar treats the file argument as a pattern; a name containing */?/[ could match the wrong file or multiple files (multiple matches concatenate into stdout → corrupt stream). CRC32 would catch it as a failure rather than silent corruption, but it's a sharp edge. Worth noting / pinning exact-match if the unrar build supports it.
 5. (Minor) CRC32 is optional. If unrar omits CRC32: (older builds, header-encrypted archives), crc32 is None and the receiver verifies size only.
 6. (Minor / cosmetic)
   - DownloadGameFileChunkFinished reports relative_path as "{game_id}/local/…" while bytes actually land in staging (.local.installing); and there's no total-size up front, so the UI can't show a percentage.
   - parse_unrar_listing depends on unrar's human-readable lt output (Details:/Name:/Type:/Size:/CRC32:). -cfg- guards against user config, but it's still brittle across unrar versions/locales.
 ## Nits
 - PeerStartOptions dropped #[derive(Debug)] for a hand-written Debug (Arc isn't Debug) — correct, and Clone/Default are retained. Good.
 - commit() renames staging into local/, so the .lanspread_owned marker lands inside local/ — but that's identical to the existing install_inner behavior, so no regression.