fix(client): address PR #67 review — error propagation, test cleanup

SessionManager.new_game() now returns "" on dir creation failure
instead of proceeding with a broken game-id. Main menu guards
against empty return. Test suite tracks and cleans up created
save directories in after_test().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-02-25 12:08:48 +01:00
co-authored by Claude Opus 4.6
parent cafa892b1e
commit 5d1d0d000c
3 changed files with 32 additions and 10 deletions
+1 -1
View File
@@ -30,6 +30,7 @@ func new_game() -> String:
if err != OK:
push_error("SessionManager: failed to create save dir %s: %s" % [
save_path, error_string(err)])
return ""
GameState.current_game_id = game_id
return game_id
@@ -74,7 +75,6 @@ func quit_to_menu(node: Node) -> void:
if _quit_dialog != null and is_instance_valid(_quit_dialog):
return # Dialog already open
_quit_dialog = ConfirmationDialog.new()
_quit_dialog.title = ""
_quit_dialog.dialog_text = UIStrings.get_text("menu.confirm_quit")
_quit_dialog.ok_button_text = UIStrings.get_text("menu.confirm_yes")
_quit_dialog.cancel_button_text = UIStrings.get_text("menu.confirm_no")
+27 -8
View File
@@ -4,6 +4,9 @@
class_name TestSessionManagerSprint19
extends GdUnitTestSuite
# Game IDs created during the current test — deleted in after_test().
var _created_ids: Array = []
# ---------------------------------------------------------------------------
# Lifecycle
@@ -11,12 +14,28 @@ extends GdUnitTestSuite
func before_test() -> void:
GameState.current_game_id = ""
_created_ids = []
func after_test() -> void:
for game_id in _created_ids:
var path := "user://saves/" + game_id
DirAccess.remove_absolute(ProjectSettings.globalize_path(path))
_created_ids.clear()
GameState.current_game_id = ""
# ---------------------------------------------------------------------------
# Helper: call new_game() and track the created directory for cleanup.
# ---------------------------------------------------------------------------
func _new_game() -> String:
var game_id := SessionManager.new_game()
if not game_id.is_empty():
_created_ids.append(game_id)
return game_id
# ---------------------------------------------------------------------------
# GameState.current_game_id field
# ---------------------------------------------------------------------------
@@ -53,14 +72,14 @@ func test_session_manager_autoload_exists() -> void:
# ---------------------------------------------------------------------------
func test_new_game_returns_non_empty_string() -> void:
var game_id := SessionManager.new_game()
var game_id := _new_game()
assert_str(game_id).override_failure_message(
"SessionManager.new_game() must return a non-empty game-id string"
).is_not_empty()
func test_new_game_sets_current_game_id_on_gamestate() -> void:
var game_id := SessionManager.new_game()
var game_id := _new_game()
assert_str(GameState.current_game_id).override_failure_message(
"new_game() must set GameState.current_game_id"
).is_equal(game_id)
@@ -68,7 +87,7 @@ func test_new_game_sets_current_game_id_on_gamestate() -> void:
func test_new_game_id_format_has_two_dashes() -> void:
## Format: <YYYYMMDD>-<HHMMSS>-<hex6> — two separator dashes.
var game_id := SessionManager.new_game()
var game_id := _new_game()
var parts := game_id.split("-")
assert_int(parts.size()).override_failure_message(
"game-id must have format <YYYYMMDD>-<HHMMSS>-<hex6> (3 parts separated by '-')"
@@ -77,7 +96,7 @@ func test_new_game_id_format_has_two_dashes() -> void:
func test_new_game_id_first_part_is_8_digits() -> void:
## First part is YYYYMMDD — 8 decimal digits.
var game_id := SessionManager.new_game()
var game_id := _new_game()
var parts := game_id.split("-")
assert_int(parts[0].length()).override_failure_message(
"game-id first part (date) must be 8 characters (YYYYMMDD)"
@@ -86,7 +105,7 @@ func test_new_game_id_first_part_is_8_digits() -> void:
func test_new_game_id_second_part_is_6_digits() -> void:
## Second part is HHMMSS — 6 decimal digits.
var game_id := SessionManager.new_game()
var game_id := _new_game()
var parts := game_id.split("-")
assert_int(parts[1].length()).override_failure_message(
"game-id second part (time) must be 6 characters (HHMMSS)"
@@ -95,7 +114,7 @@ func test_new_game_id_second_part_is_6_digits() -> void:
func test_new_game_id_third_part_is_6_hex_chars() -> void:
## Third part is 6 hex characters (RNG seed).
var game_id := SessionManager.new_game()
var game_id := _new_game()
var parts := game_id.split("-")
assert_int(parts[2].length()).override_failure_message(
"game-id third part (hex seed) must be 6 characters"
@@ -105,8 +124,8 @@ func test_new_game_id_third_part_is_6_hex_chars() -> void:
func test_new_game_ids_are_unique() -> void:
## Two rapid new_game() calls should produce different IDs
## (different RNG seeds; same-second timestamps are valid but seeds differ).
var id1 := SessionManager.new_game()
var id2 := SessionManager.new_game()
var id1 := _new_game()
var id2 := _new_game()
# Check that hex seeds differ (they almost certainly will)
var seed1 := id1.split("-")[2]
var seed2 := id2.split("-")[2]
+4 -1
View File
@@ -32,7 +32,10 @@ func _refresh_continue_state() -> void:
func _on_new_game() -> void:
SessionManager.new_game()
var game_id := SessionManager.new_game()
if game_id.is_empty():
push_error("MainMenu: new_game() failed to create save directory — cannot start")
return
get_tree().change_scene_to_file(GAME_SCENE)