From effb83a0d6dddd27738d1865ab70ec924e60359b Mon Sep 17 00:00:00 2001
From: Jeroen Schweitzer <jpmschweitzer@gmail.com>
Date: Tue, 21 Apr 2026 12:01:48 +0200
Subject: [PATCH] =?UTF-8?q?fix(ui):=20PR=20#134=20review=20=E2=80=94=20cha?=
 =?UTF-8?q?racter=20creation=20bugs=20+=20protocol=20default?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses Hoshe's 3 code-quality items from the sprint-36 client review.

- character_creation: drop CARDINAL_NAMES (was [south, east, north,
  west]) and use CARDINAL_DIRS ([south, west, north, east]) for both
  facing and screenshot filename label. The two arrays indexed by the
  same _screenshot_cardinal_idx produced swapped labels at indices 1
  and 3 — screenshots at those positions had filenames that did not
  match the character's actual facing.
- character_creation: Enter/KP_ENTER now honors _footer_start.disabled.
  Without a bookmark selected the Start button disables, but the
  keyboard path called _on_start() unconditionally — a player could
  confirm creation with empty bookmark/location strings. Guard at the
  top of _on_start.
- protocol.gd: raw_bm.get("career", "tycoon") hardcoded a content
  default in the wire decoder — a missing server field silently became
  "tycoon". Empty string is the correct protocol default;
  _make_bookmark_card already skips the career label when empty.
---
 client/scripts/protocol/protocol.gd             |  2 +-
 .../character_creation/character_creation.gd    | 17 ++++++++++++-----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/client/scripts/protocol/protocol.gd b/client/scripts/protocol/protocol.gd
index 798dde0b7..f8230bfba 100644
--- a/client/scripts/protocol/protocol.gd
+++ b/client/scripts/protocol/protocol.gd
@@ -409,7 +409,7 @@ static func decode_snapshot(bytes: PackedByteArray) -> Variant:
 					"default_location": str(raw_bm.get("default_location", "")),
 					"allowed_locations": al,
 					"allowed_locations_cultures": alc,
-					"career": str(raw_bm.get("career", "tycoon")),
+					"career": str(raw_bm.get("career", "")),
 					"starting_capital_tractus": int(raw_bm.get("starting_capital_tractus", 0)),
 				}
 			)
diff --git a/client/ui/meta/screens/character_creation/character_creation.gd b/client/ui/meta/screens/character_creation/character_creation.gd
index 0ed963751..d3d13ecbf 100644
--- a/client/ui/meta/screens/character_creation/character_creation.gd
+++ b/client/ui/meta/screens/character_creation/character_creation.gd
@@ -193,7 +193,6 @@ const MANIFEST_PATH := "res://assets/characters/manifest.json"
 const APPEARANCE_SUB_NAMES := ["Body", "Head", "Hair", "Clothing", "Accessories"]
 
 const SCREENSHOT_DIR := "user://screenshots/"
-const CARDINAL_NAMES: Array[String] = ["south", "east", "north", "west"]
 
 # --- Descriptor and preview state ---
 var _descriptor: CharacterVisualDescriptor
@@ -1609,9 +1608,11 @@ func _take_screenshot(suffix: String = "") -> void:
 	DirAccess.make_dir_recursive_absolute(SCREENSHOT_DIR)
 
 	if _screenshot_cardinals:
-		# Take screenshot for current cardinal, then advance
-		var dir_name := CARDINAL_NAMES[_screenshot_cardinal_idx]
-		_char_visual.set_facing(CARDINAL_DIRS[_screenshot_cardinal_idx])
+		# Take screenshot for current cardinal, then advance. Single array for
+		# both facing and filename label — previously two arrays with different
+		# orderings produced swapped labels at indices 1 and 3.
+		var dir_name := CARDINAL_DIRS[_screenshot_cardinal_idx]
+		_char_visual.set_facing(dir_name)
 		suffix = dir_name
 
 	var filename := (
@@ -1626,7 +1627,7 @@ func _take_screenshot(suffix: String = "") -> void:
 
 	if _screenshot_cardinals:
 		_screenshot_cardinal_idx += 1
-		if _screenshot_cardinal_idx < CARDINAL_NAMES.size():
+		if _screenshot_cardinal_idx < CARDINAL_DIRS.size():
 			# More directions to capture
 			_schedule_screenshot()
 			return
@@ -2059,6 +2060,12 @@ func _on_back() -> void:
 
 
 func _on_start() -> void:
+	# Footer Start button owns the "is confirmation allowed" state
+	# (requires bookmark + location selection). Honor that gating for
+	# keyboard Enter as well — otherwise a player can press Enter with
+	# no bookmark and confirm with empty strings.
+	if _footer_start != null and _footer_start.disabled:
+		return
 	var profile = CharacterProfile.new()  # untyped — avoids parse-time CharacterVisualDescriptor resolution
 	profile.descriptor = _descriptor
 	profile.bookmark_id = _selected_bookmark_id