daddy/modbison
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Editor save path: single/batch chunk PUT, 409 retry, non-joined Edit World entry

Browse Source 
Implements world_persistence_impl.md §9 client/editor task items 2-3:

- ChunkSaveService (scripts/editor/chunk_save_service.gd): single-chunk
  PUT with base_revision, batch PUT for multi-chunk edits, and a real
  409 -> re-fetch -> retry loop. Full-chunk overwrite (§8.2 D5) means
  "re-apply the edit" is just retrying with the locally-edited payload
  at the freshly-learned revision, not delta replay.
- InGameEditor gets a Save button (+ Ctrl+S) that surfaces live_session
  as an explicit warning instead of ignoring it, per §6 D4.
- "Edit World" entry point on the world list (owner-gated) that calls
  Session.begin_edit() instead of ApiClient.join_world() -- editing
  never joins, opens no ENet connection, spawns no dedicated server.
- game.gd wires Session.editing to fetch the real (0,0,0) chunk over
  HTTP and open the editor against it, falling back to an empty chunk
  for an unwritten coordinate.

Verified against a real backend (register/world/save/reload, a genuine
409 from two racing saves, atomic 2-chunk batch, live_session field) --
new scripts/smoke_test_editor_save.gd (19/19) plus an extended
smoke_test_world_list.gd covering the non-joined entry point (11/11).
Full existing Godot smoke suite (18 files) and backend pytest (132)
still pass unchanged.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2FESCaMXeJpC9gSctiEeD
This commit is contained in:
daddy
2026-09-14 23:35:33 +00:00
parent 31f6974c7a
commit 4ff74ae823
7 changed files with 742 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/editor/chunk_save_service.gd
+280
View File
@@ -0,0 +1,280 @@
class_name ChunkSaveService
extends Node
## HTTP save path for the in-game editor
## (docs/architecture/world_persistence_impl.md §8.1 chunk write / §8.2 D5).
##
## Owns the write half of editor persistence: single-chunk PUT with
## `X-Base-Revision`/`base_revision`, batch PUT for a multi-chunk edit, and
## the §5.4 optimistic-concurrency retry loop on `409`.
##
## Conflict handling ("re-fetch current chunk, re-apply the edit, retry"):
## per §8.2 D5 this backend stores and accepts **whole chunks**, not
## JSON-Patch deltas -- "lost-update protection comes from `base_revision`,
## not from the patch". That means there is no delta to replay against a
## freshly-fetched chunk; "re-apply the edit" is exactly "keep the locally
## edited payload, learn the chunk's current revision, and retry at that
## revision". `fetch_chunk()` is still called on a conflict (rather than
## just trusting the 409 body's `current_revision`) so the editor's own
## last-known state stays in sync with the store, matching what a real
## client does before retrying a write it doesn't fully trust yet.
##
## Deliberately does **not** use `ApiClient.request()`: that helper discards
## the parsed JSON body on any non-2xx response (see its `_make_result`
## calls), and the `409` body's `current_revision`/`current_checksum` are
## exactly what this retry loop needs. Uses a raw per-request `HTTPRequest`
## child instead, the same pattern `scripts/http_chunk_source.gd` uses for
## the read side.
##
## Must be added to the scene tree before use.
## The world this service saves chunks for (string form of the backend's
## numeric world id).
var world_id: String = ""
## The room slug within that world. world.md's default room is "main".
var room_slug: String = "main"
## Bounded retries on a 409 conflict -- refuses to loop forever against a
## chunk under sustained contention.
const MAX_RETRIES := 3
## Fetch the current chunk at [param coord].
##
## Returns:
## {"ok": bool, "status": int, "model": ChunkEditorModel|null,
## "revision": int, "checksum": String, "detail": String}
##
## A `404` (unwritten coordinate) is reported as `ok: true, model: null` --
## world_persistence_impl.md §8.1: "an unwritten coordinate is not an error".
## Callers wanting a fresh chunk to edit should fall back to
## `ChunkEditorModel.create_empty()` at `base_revision: 0` in that case.
func fetch_chunk(coord: Vector3i) -> Dictionary:
var http := HTTPRequest.new()
http.timeout = 15.0
add_child(http)
var api := _api_client()
var headers := PackedStringArray(["Accept: application/octet-stream"])
if api.has_token():
headers.append("Authorization: " + str(api.bearer_token))
var url: String = str(api.base_url) + _chunk_path(coord)
var err := http.request(url, headers, HTTPClient.METHOD_GET)
if err != OK:
http.queue_free()
return _fetch_result(false, 0, null, 0, "", "request failed to start (%d)" % err)
var args: Array = await http.request_completed
var result_code: int = args[0]
var response_code: int = args[1]
var response_headers: PackedStringArray = args[2]
var body: PackedByteArray = args[3]
http.queue_free()
if result_code != HTTPRequest.RESULT_SUCCESS:
return _fetch_result(false, 0, null, 0, "", "transport error (%d)" % result_code)
if response_code == 404:
return _fetch_result(true, 404, null, 0, "", "not found (unwritten chunk)")
if response_code != 200:
return _fetch_result(false, response_code, null, 0, "", _error_detail_from_body(body))
var model := ChunkEditorModel.from_binary(body)
if model == null:
return _fetch_result(false, response_code, null, 0, "", "failed to decode chunk payload")
return _fetch_result(
true, 200, model,
_int_header(response_headers, "x-chunk-revision"),
_etag_from_headers(response_headers),
"OK",
)
## Save one chunk. `model.encode_binary()` is sent as the new payload;
## `base_revision` is the revision the editor last knew about (0 = create).
##
## Returns:
## {"ok": bool, "status": int, "revision": int, "checksum": String,
## "live_session": bool, "retried": bool, "detail": String}
func save_chunk(model: ChunkEditorModel, coord: Vector3i, base_revision: int) -> Dictionary:
var attempt_revision := base_revision
var retried := false
for attempt in range(MAX_RETRIES + 1):
var body := {
"base_revision": attempt_revision,
"payload_b64": Marshalls.raw_to_base64(model.encode_binary()),
}
var resp := await _request_json(HTTPClient.METHOD_PUT, _chunk_path(coord), body)
if resp["status"] == 200:
var d: Dictionary = resp["data"]
return {
"ok": true, "status": 200,
"revision": int(d.get("revision", attempt_revision)),
"checksum": str(d.get("checksum", "")),
"live_session": bool(d.get("live_session", false)),
"retried": retried, "detail": "OK",
}
if resp["status"] == 409 and attempt < MAX_RETRIES:
var conflict: Dictionary = resp["data"] if resp["data"] is Dictionary else {}
var current_rev := int(conflict.get("current_revision", attempt_revision))
# Doc-mandated "re-fetch current chunk" step before retrying --
# keeps this service's view of the chunk current even though the
# retry itself only needs the revision number from the 409 body.
await fetch_chunk(coord)
attempt_revision = current_rev
retried = true
continue
return {"ok": false, "status": resp["status"], "revision": 0, "checksum": "",
"live_session": false, "retried": retried, "detail": _error_detail(resp)}
return {"ok": false, "status": 409, "revision": 0, "checksum": "", "live_session": false,
"retried": true, "detail": "conflict retry limit exceeded"}
## Save multiple chunks in one atomic batch PUT (world_persistence_impl.md
## §8.1 batch write: "an editor brush stroke or an object drag routinely
## straddles a chunk boundary"). `entries` is an Array of
## `{"coord": Vector3i, "model": ChunkEditorModel, "base_revision": int}`.
## All-or-nothing: a `409` names the offending coordinate and nothing is
## written, so only that entry's `base_revision` is refreshed before the
## whole batch is retried.
##
## Returns:
## {"ok": bool, "status": int, "live_session": bool,
## "chunks": Array, "retried": bool, "detail": String}
func save_batch(entries: Array) -> Dictionary:
var working: Array = []
for e in entries:
working.append((e as Dictionary).duplicate())
var retried := false
for attempt in range(MAX_RETRIES + 1):
var chunks_json: Array = []
for e in working:
var c: Vector3i = e["coord"]
var m: ChunkEditorModel = e["model"]
chunks_json.append({
"cx": c.x, "cy": c.y, "cz": c.z,
"base_revision": int(e["base_revision"]),
"payload_b64": Marshalls.raw_to_base64(m.encode_binary()),
})
var resp := await _request_json(HTTPClient.METHOD_PUT, _batch_path(), {"chunks": chunks_json})
if resp["status"] == 200:
var d: Dictionary = resp["data"]
return {"ok": true, "status": 200, "live_session": bool(d.get("live_session", false)),
"chunks": d.get("chunks", []), "retried": retried, "detail": "OK"}
if resp["status"] == 409 and attempt < MAX_RETRIES:
var conflict: Dictionary = resp["data"] if resp["data"] is Dictionary else {}
var current_rev := int(conflict.get("current_revision", 0))
var offending: Variant = conflict.get("coord")
if offending is Array and offending.size() == 3:
var oc := Vector3i(int(offending[0]), int(offending[1]), int(offending[2]))
for e in working:
if (e["coord"] as Vector3i) == oc:
await fetch_chunk(oc)
e["base_revision"] = current_rev
break
retried = true
continue
return {"ok": false, "status": resp["status"], "live_session": false, "chunks": [],
"retried": retried, "detail": _error_detail(resp)}
return {"ok": false, "status": 409, "live_session": false, "chunks": [],
"retried": true, "detail": "conflict retry limit exceeded"}
# ─────────────────────────── internals ───────────────────────────
## Resolved via an absolute node path, not the bare `ApiClient` autoload
## identifier -- see the identical note on `HttpChunkSource._api_client()`
## (bare autoload identifiers don't resolve when a script is compiled for a
## headless `godot --script ...` run, which is how this service's smoke
## test drives it).
func _api_client() -> Node:
return get_node("/root/ApiClient")
func _chunk_path(coord: Vector3i) -> String:
return "/api/worlds/%s/rooms/%s/chunks/%d/%d/%d" % [world_id, room_slug, coord.x, coord.y, coord.z]
func _batch_path() -> String:
return "/api/worlds/%s/rooms/%s/chunks" % [world_id, room_slug]
func _request_json(method: HTTPClient.Method, path: String, body_dict: Dictionary) -> Dictionary:
var http := HTTPRequest.new()
http.timeout = 15.0
add_child(http)
var api := _api_client()
var headers := PackedStringArray(["Content-Type: application/json", "Accept: application/json"])
if api.has_token():
headers.append("Authorization: " + str(api.bearer_token))
var url: String = str(api.base_url) + path
var err := http.request(url, headers, method, JSON.stringify(body_dict))
if err != OK:
http.queue_free()
return {"status": 0, "data": null, "raw": "request failed to start (%d)" % err}
var args: Array = await http.request_completed
var result_code: int = args[0]
var response_code: int = args[1]
var body_bytes: PackedByteArray = args[3]
http.queue_free()
if result_code != HTTPRequest.RESULT_SUCCESS:
return {"status": 0, "data": null, "raw": "transport error (%d)" % result_code}
var text := body_bytes.get_string_from_utf8()
var parsed: Variant = JSON.parse_string(text) if text != "" else null
return {"status": response_code, "data": parsed, "raw": text}
func _error_detail(resp: Dictionary) -> String:
var data: Variant = resp.get("data")
if data is Dictionary and (data as Dictionary).has("detail"):
return str((data as Dictionary)["detail"])
if int(resp.get("status", 0)) == 0:
return str(resp.get("raw", "network error"))
return "HTTP error %d" % int(resp.get("status", 0))
func _error_detail_from_body(body: PackedByteArray) -> String:
var text := body.get_string_from_utf8()
var parsed: Variant = JSON.parse_string(text) if text != "" else null
if parsed is Dictionary and (parsed as Dictionary).has("detail"):
return str((parsed as Dictionary)["detail"])
return text
func _fetch_result(ok: bool, status: int, model: ChunkEditorModel, revision: int,
checksum: String, detail: String) -> Dictionary:
return {"ok": ok, "status": status, "model": model, "revision": revision,
"checksum": checksum, "detail": detail}
func _int_header(headers: PackedStringArray, key: String) -> int:
var prefix := key + ":"
for h in headers:
if h.to_lower().begins_with(prefix):
return int(h.substr(h.find(":") + 1).strip_edges())
return 0
func _etag_from_headers(headers: PackedStringArray) -> String:
for h in headers:
if h.to_lower().begins_with("etag:"):
var v := h.substr(h.find(":") + 1).strip_edges()
return v.trim_prefix('"').trim_suffix('"')
return ""
scripts/editor/ui/in_game_editor.gd
+85 -4
View File
@@ -16,10 +16,12 @@ extends Control
## InspectorPanel -- property editor for the selected object
## EditorCore -- shared core (passed in, not owned/created here)
##
## This UI (and `EditorCore`) never talk to the network -- persisting edits
## is a sibling task's job (world_persistence_impl.md's batch-PUT save
## path). Closing this editor just discards the UI; the in-memory
## `ChunkEditorModel` it was pointed at is unaffected either way.
## Persisting edits (world_persistence_impl.md's single/batch-PUT save path,
## §8.1-8.2) is wired through `ChunkSaveService`, set via
## `setup_persistence()`. Without it (e.g. the F2 scratch-chunk path, or
## `smoke_test_editor_core.gd`'s bare `InGameEditor.new(core)`), the Save
## button is present but disabled -- closing the editor just discards the
## UI and the in-memory `ChunkEditorModel` is unaffected either way.
signal closed
@@ -30,6 +32,18 @@ var _palette: PalettePanel
var _inspector: InspectorPanel
var _viewport: EditorViewport
var _close_button: Button
var _save_button: Button
var _save_status_label: Label
## Save-path state (world_persistence_impl.md §8.1/§8.2), set via
## `setup_persistence()`. `_save_service == null` means there is nothing to
## save to (see the class doc above) -- the Save button stays disabled.
var _save_service: ChunkSaveService = null
var _world_id: String = ""
var _room_slug: String = "main"
var _coord: Vector3i = Vector3i.ZERO
var _base_revision: int = 0
var _saving: bool = false
func _init(p_core: EditorCore) -> void:
@@ -39,6 +53,21 @@ func _init(p_core: EditorCore) -> void:
mouse_filter = Control.MOUSE_FILTER_STOP
## Wire this editor's Save button/Ctrl+S to a real backend round-trip.
## `save_service` must already be in the scene tree (game.gd adds it as a
## sibling before constructing this editor). `base_revision` is whatever the
## caller last fetched for `coord` (0 if the chunk didn't exist yet).
func setup_persistence(save_service: ChunkSaveService, world_id: String, room_slug: String,
coord: Vector3i, base_revision: int) -> void:
_save_service = save_service
_world_id = world_id
_room_slug = room_slug
_coord = coord
_base_revision = base_revision
if _save_button != null:
_save_button.disabled = false
func _ready() -> void:
_build_ui()
_refresh_history_buttons()
@@ -66,11 +95,22 @@ func _build_ui() -> void:
_toolbar.size_flags_horizontal = Control.SIZE_EXPAND_FILL
top_bar.add_child(_toolbar)
_save_button = Button.new()
_save_button.text = "Save (Ctrl+S)"
_save_button.disabled = (_save_service == null)
_save_button.pressed.connect(_on_save_pressed)
top_bar.add_child(_save_button)
_close_button = Button.new()
_close_button.text = "Close (F2)"
_close_button.pressed.connect(_on_close_pressed)
top_bar.add_child(_close_button)
_save_status_label = Label.new()
_save_status_label.autowrap_mode = TextServer.AUTOWRAP_WORD
_save_status_label.visible = false
root_vbox.add_child(_save_status_label)
var body := HBoxContainer.new()
body.size_flags_vertical = Control.SIZE_EXPAND_FILL
root_vbox.add_child(body)
@@ -119,6 +159,9 @@ func _unhandled_key_input(event: InputEvent) -> void:
elif key.ctrl_pressed and key.keycode == KEY_Y:
_on_redo_pressed()
get_viewport().set_input_as_handled()
elif key.ctrl_pressed and key.keycode == KEY_S:
_on_save_pressed()
get_viewport().set_input_as_handled()
elif key.keycode == KEY_DELETE and core.selection.has_selection():
core.delete_object(core.selection.get_selected())
_on_edit_applied()
@@ -167,3 +210,41 @@ func _refresh_history_buttons() -> void:
func _on_close_pressed() -> void:
closed.emit()
queue_free()
## Single-chunk save (world_persistence_impl.md §8.1). `ChunkSaveService`
## owns the actual HTTP round trip, retrying on `409` internally
## (§8.2/§5.4) -- this just reflects the outcome and, per §6 D4, surfaces
## the `live_session` flag as a warning instead of ignoring it.
func _on_save_pressed() -> void:
if _save_service == null or _saving:
return
_saving = true
_save_button.disabled = true
_set_save_status("Saving…", Color(0.85, 0.85, 0.85))
var result: Dictionary = await _save_service.save_chunk(core.model, _coord, _base_revision)
_saving = false
_save_button.disabled = false
if result.get("ok", false):
_base_revision = int(result.get("revision", _base_revision))
var msg := "Saved (revision %d)." % _base_revision
if bool(result.get("retried", false)):
msg += " (resolved a save conflict automatically)"
if bool(result.get("live_session", false)):
msg += " This world has players in it right now -- they'll see your changes when the session restarts."
_set_save_status(msg, Color(1.0, 0.8, 0.25))
else:
_set_save_status(msg, Color(0.55, 1.0, 0.55))
else:
_set_save_status("Save failed: %s" % str(result.get("detail", "unknown error")), Color(1.0, 0.45, 0.45))
func _set_save_status(text: String, color: Color) -> void:
if _save_status_label == null:
return
_save_status_label.text = text
_save_status_label.add_theme_color_override("font_color", color)
_save_status_label.visible = true