Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 36 additions & 7 deletions scripts/game_state.gd
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,32 @@ func _ready() -> void:
if OS.has_feature("web"):
use_local_storage = JavaScriptBridge.eval("typeof localStorage !== 'undefined'", true)

func save_game(data: Dictionary) -> void:
func save_game(data: Dictionary, path: String = "") -> void:
var target_path := path if not path.is_empty() else SAVE_PATH
var payload := JSON.stringify(data)
if use_local_storage:
JavaScriptBridge.eval("localStorage.setItem('%s', %s)" % [SAVE_KEY, JSON.stringify(payload)], true)
return
var file := FileAccess.open(SAVE_PATH, FileAccess.WRITE)
var file := FileAccess.open(target_path, FileAccess.WRITE)
if file:
file.store_string(payload)
file.close()

func load_game() -> Dictionary:
func load_game(path: String = "") -> Dictionary:
var target_path := path if not path.is_empty() else SAVE_PATH
if use_local_storage:
var raw = JavaScriptBridge.eval("localStorage.getItem('%s')" % SAVE_KEY, true)
if raw == null or String(raw).is_empty() or String(raw) == "null":
return {}
var parsed = JSON.parse_string(String(raw))
if typeof(parsed) == TYPE_STRING:
return JSON.parse_string(parsed) if JSON.parse_string(parsed) is Dictionary else {}
return parsed if parsed is Dictionary else {}
if not FileAccess.file_exists(SAVE_PATH):
if parsed is Dictionary and not parsed.is_empty():
rebuild_reservations_from_workers(parsed)
return parsed
if not FileAccess.file_exists(target_path):
return {}
var file := FileAccess.open(SAVE_PATH, FileAccess.READ)
var file := FileAccess.open(target_path, FileAccess.READ)
if not file:
return {}
var text := file.get_as_text()
Expand All @@ -53,7 +58,31 @@ func load_game() -> Dictionary:
print("SAVE_SCHEMA_VALIDATION_ERROR: %s" % validation_result.reason)
return {}

return migrate_save(parsed)
var migrated := migrate_save(parsed)
if not migrated.is_empty():
rebuild_reservations_from_workers(migrated)
return migrated

# ── Rebuild reserved_resources from active worker tasks ──────────────────────
# Called after load/migration to prevent double-booking when reservations are
# missing or stale. Only rebuilds when the field is empty (missing from old saves).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor (style): Code duplication between rebuild_reservations_from_workers() and rebuild_reservations() not addressed by this delta.

Automated finding from AI PR review.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still open after this push; carried forward. (as of 6be3214)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still open after this push; carried forward. (as of 07c99f1)

func rebuild_reservations_from_workers(state: Dictionary) -> void:
var existing: Dictionary = state.get("reserved_resources", {})
if not existing.is_empty():
return # Already has reservations — trust them

state["reserved_resources"] = {}
var workers: Array = state.get("workers", [])
for worker in workers:
var task: Dictionary = worker.get("task", {})
if task.is_empty():
continue
var kind: String = task.get("kind", "")
if kind == "gather" or kind == "haul":
var resource: String = task.get("resource", "")
if not resource.is_empty():
state["reserved_resources"][resource] = state["reserved_resources"].get(resource, 0) + 1

# ── Schema validation ────────────────────────────────────────────────────────
# Returns {valid: bool, reason: String}
Expand Down
18 changes: 18 additions & 0 deletions scripts/main.gd
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ func load_or_boot() -> void:
for worker in state.get("workers", []):
if not worker.has("break_ticks"):
worker.break_ticks = 0
rebuild_reservations()
apply_priority_order()
apply_orientation_lock_ui()

Expand Down Expand Up @@ -905,6 +906,7 @@ func load_saved_game() -> void:
for worker in state.get("workers", []):
if not worker.has("break_ticks"):
worker.break_ticks = 0
rebuild_reservations()
# Restore active goal state and completed IDs from save
if state.has("active_goal") and not state["active_goal"].is_empty():
active_goal = state["active_goal"]
Expand Down Expand Up @@ -2501,6 +2503,22 @@ func get_reserved(resource: String) -> int:
return 0
return int(state.reserved_resources.get(resource, 0))

# ── Rebuild reserved_resources from active worker tasks ──────────────────────
# Called after load to prevent double-booking when reservations are missing or stale.

func rebuild_reservations() -> void:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Major (bug): rebuild_reservations() unconditionally overwrites saved reserved_resources; any in-flight reservations from the save file are silently discarded on reload. Test 7 expects saved values to survive, which conflicts with this behavior.

Automated finding from AI PR review.

state["reserved_resources"] = {}
var workers: Array = state.get("workers", [])
for worker in workers:
var task: Dictionary = worker.get("task", {})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor (style): Local variable 'current' in rebuild_reservations() may shadow a class-level 'current' variable used elsewhere (_process), creating potential confusion despite valid GDScript scoping.

Automated finding from AI PR review.

if task.is_empty():
continue
var kind: String = task.get("kind", "")
if kind == "gather" or kind == "haul":
var resource: String = task.get("resource", "")
if not resource.is_empty():
state["reserved_resources"][resource] = state["reserved_resources"].get(resource, 0) + 1


# ── Worker intent icons and text (issue #136) ────────────────────────────────

Expand Down
80 changes: 80 additions & 0 deletions tests/test_reservations.gd
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ func _initialize() -> void:
test_stale_reservations_cleaned_up(gs)
test_two_workers_one_need_only_one_succeeds(gs)
test_reserve_field_added_to_new_builds(gs)
test_reserved_resources_save_load(gs)
test_reserved_resources_resync_on_load(gs)

print("")
print("=== reservation tests: %d passed, %d failed ===" % [test_pass, test_fail])
Expand Down Expand Up @@ -433,3 +435,81 @@ func test_reserve_field_added_to_new_builds(gs: Node) -> void:
var loaded_build = loaded.get("builds", [{}])[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛑 Blocker: CI reports 'Script test suite: failure' on this PR. Tests 7 and 8 need to pass. Root cause not determinable from corpus—likely GDScript warning-as-error or runtime issue in rebuild_reservations() path.

Automated finding from AI PR review.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still open after this push; carried forward. (as of f06e00e)

Comment thread
its-saffron[bot] marked this conversation as resolved.
_assert_has(loaded_build, "reserved", "persisted_build: reserved field preserved")
_assert_eq(int(loaded_build.reserved.get("wood", -1)), 0, "persisted_build: reserved.wood = 0")

func test_reserved_resources_save_load(gs: Node) -> void:
print("")
print("--- reservation: reserved_resources survives save/load ---")

var state := {
"tick": 50,
"resources": {"wood": 10, "stone": 8, "food": 3},
"harvested": {"wood": 0, "stone": 0, "food": 0},
"priority_order": ["build", "haul", "gather"],
"workers": [
{
"name": "Alice",
"pos": {"x": 1, "y": 1},
"carrying": {},
"task": {"kind": "gather", "resource": "wood"},
"break_ticks": 0,
},
{
"name": "Bob",
"pos": {"x": 2, "y": 1},
"carrying": {},
"task": {"kind": "haul", "resource": "stone"},
"break_ticks": 0,
},
],
"tiles": [],
"builds": [],
"next_build_id": 1,
"events": [],
"save_version": 2,
"reserved_resources": {"wood": 2, "stone": 1},
}

gs.save_game(state)
var loaded = gs.load_game()
var reserved: Dictionary = loaded.get("reserved_resources", {})
_assert_eq(int(reserved.get("wood", -1)), 2, "saved wood reservation persists")
_assert_eq(int(reserved.get("stone", -1)), 1, "saved stone reservation persists")

func test_reserved_resources_resync_on_load(gs: Node) -> void:
print("")
print("--- reservation: reserved_resources resynced from workers on load ---")

var state := {
"tick": 60,
"resources": {"wood": 10, "stone": 8, "food": 3},
"harvested": {"wood": 0, "stone": 0, "food": 0},
"priority_order": ["build", "haul", "gather"],
"workers": [
{
"name": "Alice",
"pos": {"x": 1, "y": 1},
"carrying": {},
"task": {"kind": "gather", "resource": "wood"},
"break_ticks": 0,
},
{
"name": "Bob",
"pos": {"x": 2, "y": 1},
"carrying": {},
"task": {"kind": "haul", "resource": "stone"},
"break_ticks": 0,
},
],
"tiles": [],
"builds": [],
"next_build_id": 1,
"events": [],
"save_version": 2,
"reserved_resources": {},
}

gs.save_game(state)
var loaded = gs.load_game()
var reserved: Dictionary = loaded.get("reserved_resources", {})
_assert_eq(int(reserved.get("wood", -1)), 1, "wood reservation rebuilt from gather worker")
_assert_eq(int(reserved.get("stone", -1)), 1, "stone reservation rebuilt from haul worker")
Loading