Fix latent client-side encode_binary bounds min+size/min+max bug (nonzero position)

Found during review of the persistence-plumbing branch: _build_metadata_section
wrote raw bounds.size into the wire's 2nd float triple, but the wire format
(and _parse_metadata_section) treats that triple as max_pos, computing
size = max - min. AABB(Vector3.ZERO, size) masks this since size == max when
position is 0 -- exactly what every existing test used. Any chunk at a real,
nonzero world position would silently corrupt on its own client-local
encode/decode round-trip. Same bug class as the backend's min+size/min+max
mismatch fixed in chunk_codec.py (eb39e43), just on the client side this
time. Added a regression case to smoke_test_chunk_codec_interop.gd using a
real nonzero chunk position (96,-32,0) -- 33/33 passing. Full sibling smoke
suite re-verified clean afterward.
This commit is contained in:
2026-09-14 23:00:17 +00:00
parent fd88cef32f
commit 65995f30a6
2 changed files with 38 additions and 3 deletions
+13 -3
View File
@@ -492,12 +492,22 @@ static func _build_metadata_section(payload: ChunkPayload) -> PackedByteArray:
data.append_array(_write_int32(payload.coord.x)) data.append_array(_write_int32(payload.coord.x))
data.append_array(_write_int32(payload.coord.y)) data.append_array(_write_int32(payload.coord.y))
data.append_array(_write_int32(payload.coord.z)) data.append_array(_write_int32(payload.coord.z))
# Wire format is min + max (world.md §3, and _parse_metadata_section
# below reads the 2nd triple as max_pos and computes size = max - min)
# -- writing raw `.size` here instead of `.position + .size` was a
# latent bug that only escaped every test because AABB(Vector3.ZERO, ...)
# happens to make size == max when position is exactly zero. Any chunk
# at a real, nonzero world position (i.e. every real chunk except the
# one at the origin) would round-trip through encode_binary/decode_binary
# with a corrupted size. Same bug class as the backend's min+size/
# min+max mismatch fixed in chunk_codec.py (commit eb39e43).
var bounds_max: Vector3 = payload.bounds.position + payload.bounds.size
data.append_array(_write_float32(payload.bounds.position.x)) data.append_array(_write_float32(payload.bounds.position.x))
data.append_array(_write_float32(payload.bounds.position.y)) data.append_array(_write_float32(payload.bounds.position.y))
data.append_array(_write_float32(payload.bounds.position.z)) data.append_array(_write_float32(payload.bounds.position.z))
data.append_array(_write_float32(payload.bounds.size.x)) data.append_array(_write_float32(bounds_max.x))
data.append_array(_write_float32(payload.bounds.size.y)) data.append_array(_write_float32(bounds_max.y))
data.append_array(_write_float32(payload.bounds.size.z)) data.append_array(_write_float32(bounds_max.z))
# Every other section builder wraps its body with `_wrap_section` # Every other section builder wraps its body with `_wrap_section`
# (section_id + uint32 length prefix) -- this one didn't, so # (section_id + uint32 length prefix) -- this one didn't, so
# `encode_binary()` spliced the raw metadata bytes directly into the # `encode_binary()` spliced the raw metadata bytes directly into the
+25
View File
@@ -28,11 +28,36 @@ func _init() -> void:
_test_empty_chunk_fixture() _test_empty_chunk_fixture()
_test_simple_tile_chunk_fixture() _test_simple_tile_chunk_fixture()
_test_chunk_with_objects_fixture() _test_chunk_with_objects_fixture()
_test_nonzero_bounds_roundtrip()
print("\n=== SMOKE RESULT: %d/%d passed ===" % [passed, passed + failed]) print("\n=== SMOKE RESULT: %d/%d passed ===" % [passed, passed + failed])
quit(0 if failed == 0 else 1) quit(0 if failed == 0 else 1)
func _test_nonzero_bounds_roundtrip() -> void:
print("--- client-local encode_binary/decode_binary round-trip, nonzero bounds position ---")
# Regression for a real bug: _build_metadata_section used to write raw
# `bounds.size` into the wire's 2nd float triple, but the wire format
# (and _parse_metadata_section) treats that triple as max_pos, deriving
# size = max - min. AABB(Vector3.ZERO, size) masks this (size == max
# when position is 0), so this specifically uses a real, nonzero chunk
# position -- e.g. the chunk at coord (3,-1,0), matching the backend
# golden fixture's own bounds.position (96,-32,0) -- to catch it for real.
var payload := ChunkDataDecoder.ChunkPayload.new()
payload.world_id = "1"
payload.room_id = "main"
payload.coord = Vector3i(3, -1, 0)
payload.bounds = AABB(Vector3(96.0, -32.0, 0.0), Vector3(32.0, 32.0, 1.0))
var encoded := ChunkDataDecoder.encode_binary(payload)
var decoded: ChunkDataDecoder.ChunkPayload = ChunkDataDecoder.decode_binary(encoded)
_check("decoded without error", decoded != null)
if decoded == null:
return
_check("bounds.position survives round-trip (96,-32,0)", decoded.bounds.position.is_equal_approx(Vector3(96.0, -32.0, 0.0)))
_check("bounds.size survives round-trip (32,32,1) -- the actual regression check", decoded.bounds.size.is_equal_approx(Vector3(32.0, 32.0, 1.0)))
func _read_fixture(name: String) -> PackedByteArray: func _read_fixture(name: String) -> PackedByteArray:
var f := FileAccess.open(FIXTURES_DIR + name, FileAccess.READ) var f := FileAccess.open(FIXTURES_DIR + name, FileAccess.READ)
if f == null: if f == null: