Address review feedback: Implement chunk streaming system in GDScript #20

2026-08-18T13:49:59Z

coder-bot commented

2026-08-18 13:49:59 +00:00

Auto-opened by coder for task bd718b31b595.

Dashboard: http://cloudbison:7070/task/bd718b31b595

Reviewer requested changes on PR #18 for this task.

Feedback:
The project does not compile — verified with godot --headless --path . --quit. Real defects, not just missing polish:

Blocking:

chunk_data_decoder.gd:111 — nested class TileDef: collides with the real global TileDef class (scripts/tile_def.gd, added in this same PR). Godot: "Class "TileDef" hides a global script class." Remove the nested duplicate and use the real TileDef class instead.
chunk_data_decoder.gd:164 — const MAGIC: PackedByteArray = PackedByteArray([...]) is rejected: "Assigned value for constant "MAGIC" isn't a constant expression." PackedByteArray(...) constructor calls aren't foldable as GDScript constants even with literal arguments. Use static var MAGIC := PackedByteArray([0x41, 0x47, 0x50, 0x43]) instead of const.
chunk_data_decoder.gd:671 — int("0x" + hex_str.substr(i, 2), 16): GDScript's int() takes exactly one argument, no base parameter (that's Python). Use ("0x" + hex_str.substr(i, 2)).hex_to_int().
chunk_data_decoder.gd:679 — SHA256Hash doesn't exist in Godot 4. Use HashingContext: var ctx := HashingContext.new(); ctx.start(HashingContext.HASH_SHA256); ctx.update(data); var hash := ctx.finish().
chunk_scene.gd:95 and :105 — assigning obj.rotation (a Vector4 quaternion, per world_object.gd:22) directly to Node3D.rotation (Vector3 Euler angles). Use .quaternion instead: static_obj.quaternion = Quaternion(obj.rotation.x, obj.rotation.y, obj.rotation.z, obj.rotation.w).

Minor: .godot/ cache files (global_script_class_cache.cfg, uid_cache.bin, filesystem_cache10, etc.) are committed — these are generated editor cache, not source, and should be gitignored.

None of this is a sign the approach is wrong — the chunk streaming design (manager/chunk/decoder/tile split) is reasonable and matches the architecture docs. These are real, fixable GDScript API mistakes (some Godot-3-vs-4, one a naming collision) that godot --headless --path . --quit would have caught immediately. Please fix and I'll re-review.

Your own branch already contains this PR's code (it was branched from agent/1a6e82153969, not from scratch) — no need to fetch, diff against, or go looking for that branch separately. Just read the files directly and apply the fixes below.
PR (for reference only): daddy/agp-game#18

Original task description:
Godot script that manages chunk load/unload based on player position. Chunks should be loaded from the server and instantiated as Godot scenes.

Auto-opened by coder for task `bd718b31b595`. Dashboard: http://cloudbison:7070/task/bd718b31b595 Reviewer requested changes on PR #18 for this task. Feedback: The project does not compile — verified with `godot --headless --path . --quit`. Real defects, not just missing polish: **Blocking:** 1. **`chunk_data_decoder.gd:111` — nested `class TileDef:` collides with the real global `TileDef` class** (`scripts/tile_def.gd`, added in this same PR). Godot: "Class "TileDef" hides a global script class." Remove the nested duplicate and use the real `TileDef` class instead. 2. **`chunk_data_decoder.gd:164` — `const MAGIC: PackedByteArray = PackedByteArray([...])` is rejected**: "Assigned value for constant "MAGIC" isn't a constant expression." `PackedByteArray(...)` constructor calls aren't foldable as GDScript constants even with literal arguments. Use `static var MAGIC := PackedByteArray([0x41, 0x47, 0x50, 0x43])` instead of `const`. 3. **`chunk_data_decoder.gd:671` — `int("0x" + hex_str.substr(i, 2), 16)`**: GDScript's `int()` takes exactly one argument, no base parameter (that's Python). Use `("0x" + hex_str.substr(i, 2)).hex_to_int()`. 4. **`chunk_data_decoder.gd:679` — `SHA256Hash` doesn't exist in Godot 4.** Use `HashingContext`: `var ctx := HashingContext.new(); ctx.start(HashingContext.HASH_SHA256); ctx.update(data); var hash := ctx.finish()`. 5. **`chunk_scene.gd:95` and `:105` — assigning `obj.rotation` (a `Vector4` quaternion, per `world_object.gd:22`) directly to `Node3D.rotation` (`Vector3` Euler angles).** Use `.quaternion` instead: `static_obj.quaternion = Quaternion(obj.rotation.x, obj.rotation.y, obj.rotation.z, obj.rotation.w)`. **Minor:** `.godot/` cache files (`global_script_class_cache.cfg`, `uid_cache.bin`, `filesystem_cache10`, etc.) are committed — these are generated editor cache, not source, and should be gitignored. None of this is a sign the approach is wrong — the chunk streaming design (manager/chunk/decoder/tile split) is reasonable and matches the architecture docs. These are real, fixable GDScript API mistakes (some Godot-3-vs-4, one a naming collision) that `godot --headless --path . --quit` would have caught immediately. Please fix and I'll re-review. Your own branch already contains this PR's code (it was branched from agent/1a6e82153969, not from scratch) — no need to fetch, diff against, or go looking for that branch separately. Just read the files directly and apply the fixes below. PR (for reference only): https://git.cloudbison.duckdns.org/daddy/agp-game/pulls/18 Original task description: Godot script that manages chunk load/unload based on player position. Chunks should be loaded from the server and instantiated as Godot scenes.

coder-bot added 20 commits 2026-08-18 13:49:59 +00:00

WIP checkpoint: Implement chunk streaming system in GDScript 7e81c09265

WIP checkpoint: Implement chunk streaming system in GDScript 2ad8e34971

WIP checkpoint: Implement chunk streaming system in GDScript 2d4a7e91cb

WIP checkpoint: Implement chunk streaming system in GDScript 8e2f17f159

WIP checkpoint: Implement chunk streaming system in GDScript ce2690e8b9

WIP checkpoint: Implement chunk streaming system in GDScript 62172e0ee4

WIP checkpoint: Implement chunk streaming system in GDScript cc27d73ca6

WIP checkpoint: Implement chunk streaming system in GDScript 5c0a61870f

WIP checkpoint: Implement chunk streaming system in GDScript 67f6a9c43f

WIP checkpoint: Implement chunk streaming system in GDScript 504c92301a

Recovered orphaned WIP before switching to a new task d752cf5280

WIP checkpoint: Implement chunk streaming system in GDScript 720cf6f6b9

WIP checkpoint: Implement chunk streaming system in GDScript 97949d6b44

WIP checkpoint: Implement chunk streaming system in GDScript 342d0f69d9

WIP checkpoint: Implement chunk streaming system in GDScript 97e0804dfe

WIP checkpoint: Implement chunk streaming system in GDScript 1c5c0f8c8e

WIP checkpoint: Address review feedback: Implement chunk streaming system in GDScript 929c69e5da

WIP checkpoint: Address review feedback: Implement chunk streaming system in GDScript 25f2dffc53

WIP checkpoint: Address review feedback: Implement chunk streaming system in GDScript 93d83a28fc

WIP checkpoint: Address review feedback: Implement chunk streaming system in GDScript 88cd255040

reviewer-bot added 1 commit 2026-08-18 14:40:59 +00:00

Merge remote-tracking branch 'origin/main' into merge-fix dbdc01f8d6

# Conflicts:
#	.gitignore
#	project.godot

reviewer-bot merged commit 99762887be into main

2026-08-18 14:41:08 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: daddy/modbison#20