style(ui): atlas review polish — guarded ids, stable keys, dead-code cleanup (#128)

10. Dropped the "no body data" fallback string in the picker panel — the
    generator always writes entry["bodies"], so the fallback was dead. Use
    the "—" convention the rest of the panel already follows.
12. Removed the autoload parse-order comment from AtlasPanel (it is
    scene-instanced, not an autoload, so the rule does not apply), and
    collapsed _build_heightmap_viewer to a direct AtlasViewer.new() —
    mirroring the rest of the file rather than dancing around a risk that
    is not real for this class.
13. AtlasMarkerOverlay._draw_cities now compares hover/selection by a
    stable _city_key() (name → city_id → pos → hash) instead of
    Dictionary.==, which was O(fields) per city per redraw. Preps the
    renderer for much larger city counts without a rewrite.
14. Orbital click/draw handlers guard against missing body_id /
    station_id by reading through str(dict.get(..., "")) and skipping
    empty ids. Matches the defensive style already used for parent_body_id
    and keeps a NULL id from crashing _draw_bodies / _handle_orbital_click.
This commit is contained in:
2026-04-15 09:02:27 +02:00
parent f8b81e1fe7
commit 489aec48d8
2 changed files with 62 additions and 25 deletions
+39 -7
View File
@@ -172,23 +172,55 @@ func _draw_cities(markers: Dictionary) -> void:
if cities.is_empty():
return
var font := ThemeDB.fallback_font
var hovered: Dictionary = viewer.get_hovered_city()
var selected: Dictionary = viewer.get_selected_city()
# Compare by stable key, not Dictionary.== — deep equality was O(fields)
# per city per redraw (review #13). `name` is unique per body in the
# D-191 §8 schema; fall back to `body_id` or the hash of the dict for
# un-named markers so the comparison still works in transitional data.
var hovered_key: String = _city_key(viewer.get_hovered_city())
var selected_key: String = _city_key(viewer.get_selected_city())
for c: Dictionary in cities:
var pos: Vector2 = viewer.city_canvas_pos(c)
var tier: int = int(c.get("population_tier", 1))
var r: float = 2.5 + float(tier) * 0.8
var key: String = _city_key(c)
var is_selected: bool = not selected_key.is_empty() and key == selected_key
var is_hovered: bool = not hovered_key.is_empty() and key == hovered_key
var col: Color = COLOR_CITY
if c == selected:
if is_selected:
col = COLOR_CITY_SELECTED
elif c == hovered:
elif is_hovered:
col = COLOR_CITY_HOVER
draw_circle(pos, r + 1.0, Color(0.0, 0.0, 0.0, 0.55))
draw_circle(pos, r, col)
var name_str: String = c.get("name", "") if c.get("name") else ""
if not name_str.is_empty() and (tier >= 3 or c == hovered or c == selected):
var lcolor: Color = COLOR_CITY_HOVER if c == hovered or c == selected else Color(0.88, 0.90, 0.96, 0.85)
draw_string(font, pos + Vector2(r + 2.0, r * 0.4), name_str, HORIZONTAL_ALIGNMENT_LEFT, -1, 8, lcolor)
if not name_str.is_empty() and (tier >= 3 or is_hovered or is_selected):
var lcolor: Color = (
COLOR_CITY_HOVER if is_hovered or is_selected
else Color(0.88, 0.90, 0.96, 0.85)
)
draw_string(
font, pos + Vector2(r + 2.0, r * 0.4), name_str,
HORIZONTAL_ALIGNMENT_LEFT, -1, 8, lcolor
)
static func _city_key(city: Dictionary) -> String:
if city.is_empty():
return ""
var name_val: Variant = city.get("name")
if name_val != null and not str(name_val).is_empty():
return "n:" + str(name_val)
var id_val: Variant = city.get("city_id")
if id_val != null and not str(id_val).is_empty():
return "i:" + str(id_val)
# Last resort: positional key from pos/lat/lon so two unnamed cities at
# different coordinates still compare unequal.
var pos: Variant = city.get("pos")
if pos != null:
return "p:" + str(pos)
if city.has("lat") and city.has("lon"):
return "ll:%s:%s" % [city["lat"], city["lon"]]
return "h:%d" % city.hash()
# =============================================================================
+23 -18
View File
@@ -85,7 +85,6 @@ func _ready() -> void:
HudGroups.register(self, APP_PATH)
HudGroups.app_changed.connect(_on_app_changed)
# Autoload parse-order rule: load via load() in method body, not _ready() cache
_implant_theme = load("res://ui/implant/default_implant.tres")
_load_system_list()
@@ -225,12 +224,15 @@ func _compute_body_positions() -> void:
var count: int = ring_bodies.size()
for i: int in range(count):
var b: Dictionary = ring_bodies[i]
var bid: String = str(b.get("body_id", ""))
if bid.is_empty():
continue
var angle: float
if count == 1:
angle = -PI / 2.0 # top position (12 o'clock)
else:
angle = -PI / 2.0 + TAU * float(i) / float(count)
_body_positions[b["body_id"]] = center + Vector2(cos(angle), sin(angle)) * ring_r
_body_positions[bid] = center + Vector2(cos(angle), sin(angle)) * ring_r
# Place moons near their parent body. Moons per parent count drives the
# angular spacing — a hard-coded divisor made the 5th+ moon overlap moon 1
@@ -256,20 +258,26 @@ func _compute_body_positions() -> void:
var count: int = siblings.size()
for i: int in range(count):
var moon: Dictionary = siblings[i]
var moon_id: String = str(moon.get("body_id", ""))
if moon_id.is_empty():
continue
var angle: float = -PI / 2.0 + TAU * float(i) / float(count)
_body_positions[moon["body_id"]] = (
_body_positions[moon_id] = (
parent_pos + Vector2(cos(angle), sin(angle)) * MOON_ORBIT_RADIUS
)
# Place stations near their parent body (offset right + slightly up)
for s: Dictionary in _orbital_stations:
var station_id: String = str(s.get("station_id", ""))
if station_id.is_empty():
continue
var parent_id: Variant = s.get("orbits_body_id")
var parent_pos: Vector2
var station_parent_pos: Vector2
if parent_id != null and _body_positions.has(str(parent_id)):
parent_pos = _body_positions[str(parent_id)]
station_parent_pos = _body_positions[str(parent_id)]
else:
parent_pos = center # fallback to star position
_station_positions[s["station_id"]] = parent_pos + Vector2(20.0, -10.0)
station_parent_pos = center # fallback to star position
_station_positions[station_id] = station_parent_pos + Vector2(20.0, -10.0)
# =============================================================================
@@ -382,8 +390,8 @@ func _draw_orbit_rings(center: Vector2) -> void:
func _draw_bodies() -> void:
var font := get_theme_default_font()
for b: Dictionary in _orbital_bodies:
var bid: String = b["body_id"]
if not _body_positions.has(bid):
var bid: String = str(b.get("body_id", ""))
if bid.is_empty() or not _body_positions.has(bid):
continue
var pos: Vector2 = _body_positions[bid]
var body_type: String = b.get("body_type", "")
@@ -430,8 +438,8 @@ func _draw_bodies() -> void:
func _draw_stations() -> void:
var font := get_theme_default_font()
for s: Dictionary in _orbital_stations:
var sid: String = s["station_id"]
if not _station_positions.has(sid):
var sid: String = str(s.get("station_id", ""))
if sid.is_empty() or not _station_positions.has(sid):
continue
var pos: Vector2 = _station_positions[sid]
var half: float = STATION_SIZE / 2.0
@@ -511,7 +519,7 @@ func _handle_orbital_click(pos: Vector2) -> void:
var bid: String = _find_nearest_body(pos)
if not bid.is_empty():
for b: Dictionary in _orbital_bodies:
if b["body_id"] == bid:
if str(b.get("body_id", "")) == bid:
_enter_body_entry(b)
return
@@ -519,7 +527,7 @@ func _handle_orbital_click(pos: Vector2) -> void:
var sid: String = _find_nearest_station(pos)
if not sid.is_empty():
for s: Dictionary in _orbital_stations:
if s["station_id"] == sid:
if str(s.get("station_id", "")) == sid:
_selected_station = s
_rebuild_station_panel()
if _station_panel:
@@ -627,7 +635,7 @@ func _rebuild_picker_panel() -> void:
_picker_panel.add_component(ImplantDataRow.new(sector + " CORRIDOR"))
if not czone.is_empty():
_picker_panel.add_component(ImplantDataRow.new("currency " + czone))
_picker_panel.add_component(ImplantDataRow.new(sys.get("bodies", "no body data")))
_picker_panel.add_component(ImplantDataRow.new(sys.get("bodies", "")))
_picker_panel.add_component(ImplantDataRow.new("pop " + sys.get("population", "")))
_picker_panel.add_component(ImplantSeparator.new())
@@ -701,10 +709,7 @@ func _build_station_panel() -> void:
func _build_heightmap_viewer() -> void:
# #835: Regional viewer instanced at runtime (parse-order rule — AtlasMarkerOverlay
# class_name is resolved by the time this _ready() runs since AtlasPanel is not an autoload).
var ViewerScript := load("res://ui/implant/atlas_viewer.gd")
_viewer = ViewerScript.new()
_viewer = AtlasViewer.new()
_viewer.name = "AtlasViewer"
_viewer.visible = false
add_child(_viewer)