#224: refuse to build a structure on top of another

BuildingField.place() dropped a structure at the fleet centroid with no
overlap check of any kind, so they could be stacked.

It reads worse than it looks. BuildingField decides which structure the
fleet is standing on by testing the centroid against interact_radius and
returning the FIRST match, so two structures inside one another make that a
question of list order rather than of where the player flew - you could fly
to your workshop and open the crafter.

The separation is interact_radius itself (150) rather than a new number,
because that is exactly the circle the ambiguity comes from. Buildings draw
to a radius of roughly 60, so this leaves a visible gap rather than letting
them touch.

Gated in three places: the info popup (the button greys out and says which
structure is in the way), _confirm_build, and _try_place - the preview goes
stale as the fleet keeps moving.

Ordering matters in one case. A unique structure replaces itself by
demolishing the old one BEFORE placing the new one, so a placement refused
afterwards would leave the player with neither. The check runs before the
demolition and excludes the structure being replaced. Case D pins it.

Two existing tests encoded the old permissive behaviour and now fly clear
first; neither is about placement:

  test_build_info_popup builds every menu option from one position, so
  each structure blocked the next
  test_payments_use_fleet_hold builds a mothership on the ship lab that
  an earlier case injects at the origin

Also fixes a bug from 37e2e89: Sfx.warm() called add_child() from _ready(),
which Godot refuses outright - it printed an error and never pre-built the
pool, defeating the point of warming it. Deferred, like Music.ensure().

173/173.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019AJMdB7iuHuLFZM1aBu3GD

audio/Sfx.gd

@@ -94,7 +94,13 @@ const PITCH_SPREAD: Dictionary = {
 static func warm(tree: SceneTree) -> void:
 	if tree == null:
 		return
-	_pool(tree)
+	# Deferred, like Music.ensure(): this is called from _ready(), where the
+	# parent is still setting up children and a direct add_child() is
+	# refused outright. The lazy path in play() runs during an ordinary
+	# frame and does not need it.
+	var build: Callable = func() -> void:
+		_pool(tree)
+	build.call_deferred()
 
 
 static func _pool(tree: SceneTree) -> Node:

controllers/BuildController.gd

@@ -426,8 +426,17 @@ func _show_building_info(type: String) -> void:
 		else:
 			lines.append("Will mine %s from %s." % [
 				GameState.resource_name(planet.resource_type), planet.planet_name])
-	elif can_build and definition.unique and GameState.has_building_of_type(type):
+	elif can_build:
-		lines.append("WARNING: only one may exist - this destroys your current one.")
+		if definition.unique and GameState.has_building_of_type(type):
+			lines.append("WARNING: only one may exist - this destroys your current one.")
+		# A unique structure replaces itself, so the one being demolished is
+		# not in the way.
+		var blocker: Dictionary = GameState.building_blocking(fleet.get_centroid(),
+			type if definition.unique else "")
+		if not blocker.is_empty():
+			lines.append("Too close to your %s - fly clear and try again." \
+				% _building_name(String(blocker.get("type", ""))))
+			can_build = false
 	if can_build and not GameState.can_afford_resources(costs, fleet.get_centroid()):
 		can_build = false
 	pending_build_type = type if can_build else ""
@@ -468,6 +477,16 @@ func _confirm_build() -> void:
 			planet.resource_type, planet.planet_id)
 		build_menu.close_menu()
 		return
+	# BEFORE the demolition below, not after: a unique structure replaces
+	# itself by removing the old one first, so a blocked placement discovered
+	# afterwards would leave the player with neither. The old one is excluded
+	# because it is the thing being replaced.
+	var blocker: Dictionary = GameState.building_blocking(fleet.get_centroid(),
+		type if definition.unique else "")
+	if not blocker.is_empty():
+		build_menu.set_status("Too close to your %s" \
+			% _building_name(String(blocker.get("type", ""))))
+		return
 	if definition.unique and GameState.has_building_of_type(type):
 		if not GameState.can_afford_resources(GameState.building_costs(type), fleet.get_centroid()):
 			build_menu.set_status(_cannot_afford(type))
@@ -477,6 +496,11 @@ func _confirm_build() -> void:
 	_try_place(type)
 
 
+func _building_name(type: String) -> String:
+	var definition: Resource = BUILDING_DEFINITIONS.get_definition(type)
+	return String(definition.display_name) if definition != null else type.capitalize()
+
+
 func _cannot_afford(type: String) -> String:
 	var missing: Array = []
 	for resource_type in GameState.building_costs(type):
@@ -489,6 +513,14 @@ func _cannot_afford(type: String) -> String:
 
 
 func _try_place(type: String) -> void:
+	# Re-checked here and not only in the preview: the fleet keeps moving
+	# between opening the menu and confirming, and this is the one place
+	# every space structure is actually placed.
+	var blocker: Dictionary = GameState.building_blocking(fleet.get_centroid())
+	if not blocker.is_empty():
+		build_menu.set_status("Too close to your %s" \
+			% _building_name(String(blocker.get("type", ""))))
+		return
 	if not GameState.spend_resources(GameState.building_costs(type), fleet.get_centroid()):
 		build_menu.set_status(_cannot_afford(type))
 		return

core/GameState.gd

@@ -2707,6 +2707,28 @@ func has_mothership() -> bool:
 	return get_mothership_position() != null
 
 
+# Space structures may not be stacked (#224). The distance is Building's own
+# interact_radius, rather than a new number: that is the circle BuildingField
+# tests the fleet centroid against to decide which structure you are standing
+# on, and it returns the FIRST match - so two structures inside one another
+# make that a question of list order rather than of where you flew.
+const MIN_BUILDING_SEPARATION: float = 150.0
+
+
+# The structure a new one at this position would sit on top of, or an empty
+# dictionary when the spot is clear. ignore_type skips a type that is about
+# to be demolished, which is how the unique structures replace themselves -
+# without it a mothership could never be rebuilt where the old one stood.
+func building_blocking(at_position: Vector2, ignore_type: String = "") -> Dictionary:
+	for building in buildings:
+		if ignore_type != "" and String(building.get("type", "")) == ignore_type:
+			continue
+		var at: Vector2 = building.get("position", Vector2.ZERO)
+		if at.distance_to(at_position) < MIN_BUILDING_SEPARATION:
+			return building
+	return {}
+
+
 func has_building_of_type(type: String) -> bool:
 	for building in buildings:
 		if building.type == type:

tests/test_build_info_popup.gd

@@ -17,6 +17,7 @@ extends SceneTree
 var scene: Node
 var game_state: Node
 var controller: Node
+var fleet: Node2D
 var popup: Control
 var title: Label
 var message: Label
@@ -55,6 +56,7 @@ func _initialize() -> void:
 	scene = load("res://world/Main.tscn").instantiate()
 	root.add_child(scene)
 	controller = scene.get_node("BuildController")
+	fleet = scene.get_node("Fleet")
 	popup = scene.get_node("UI/BuildMenu/InfoPopup")
 	title = popup.get_node("Panel/TitleLabel")
 	message = popup.get_node("Panel/MessageLabel")
@@ -83,8 +85,15 @@ func _process(delta: float) -> bool:
 			if elapsed < 0.2:
 				return false
 
-			for option in OPTIONS:
+			for i in range(OPTIONS.size()):
+				var option: Dictionary = OPTIONS[i]
 				var type: String = option.type
+				# #224: structures may not be stacked, and this loop BUILDS
+				# each option in turn. Without moving between them the second
+				# one is refused for sitting on the first, which is the rule
+				# working - but this test is about what the popup says, not
+				# about where things may go.
+				fleet.teleport_to(Vector2(float(i + 2) * GameState.MIN_BUILDING_SEPARATION * 2.0, 0.0))
 				var expected_cost: int = controller.cost_of(type)
 				var rocks_before: int = game_state.rocks
 				var built_before: int = _count(type)

tests/test_building_separation.gd

@@ -0,0 +1,135 @@
+extends SceneTree
+
+# Forgejo #224: a structure may not be built on top of one that is already
+# there.
+#
+# It matters beyond the look of it. BuildingField decides which structure the
+# fleet is standing on by testing the centroid against interact_radius and
+# returning the FIRST match, so two structures inside one another make that a
+# question of list order rather than of where the player flew - you would fly
+# to your workshop and open the crafter.
+#
+# Covers:
+#   A. the spot a structure already occupies is refused
+#   B. clear of it, the same build goes through
+#   C. a unique structure can still be rebuilt where the old one stands,
+#      because the old one is what is being replaced
+#   D. and a REFUSED rebuild does not demolish the existing one first
+
+var game_state: Node
+var scene: Node
+var controller: Node
+var fleet: Node2D
+var elapsed: float = 0.0
+var stage: int = 0
+
+
+func _initialize() -> void:
+	game_state = root.get_node("GameState")
+	game_state.buildings.clear()
+	game_state.extractors.clear()
+	game_state.reset_progression()
+	game_state.reset_new_game()
+	game_state.intro_active = false
+	game_state.reset_quests()
+	game_state.rocks = 100000
+	game_state.bank_inventory.set_amount(GameState.RESOURCE_GAS, 100000)
+	game_state.bank_inventory.set_amount(GameState.RESOURCE_ENERGY, 100000)
+	scene = load("res://world/Main.tscn").instantiate()
+	root.add_child(scene)
+	current_scene = scene
+	controller = scene.get_node("BuildController")
+	fleet = scene.get_node("Fleet")
+
+
+func _process(delta: float) -> bool:
+	elapsed += delta
+	if elapsed < 0.4:
+		return false
+
+	var sep: float = GameState.MIN_BUILDING_SEPARATION
+
+	# _confirm_build() returns SILENTLY on a locked structure, which would
+	# make every refusal below look like the separation rule working when it
+	# was only the unlock gate. Unlock what this test builds, and assert it.
+	for type in [GameState.BUILDING_COMMAND_CENTER, GameState.BUILDING_MOTHERSHIP,
+			GameState.BUILDING_SHIP_CRAFTER]:
+		var node_id: String = game_state.research_node_for_building(type)
+		if node_id != "" and not game_state.is_unlocked(node_id):
+			game_state.unlocked.append(node_id)
+		if not game_state.is_building_unlocked(type):
+			return _fail("'%s' is still locked, so a refusal would prove nothing" % type)
+
+	# --- A. the occupied spot is refused --------------------------------
+	game_state.buildings.clear()
+	game_state.add_building(GameState.BUILDING_SHIP_CRAFTER, Vector2.ZERO)
+
+	if game_state.building_blocking(Vector2.ZERO).is_empty():
+		return _fail("a structure's own position reads as a clear spot")
+	# Just inside the separation, and just outside it.
+	if game_state.building_blocking(Vector2(sep - 1.0, 0.0)).is_empty():
+		return _fail("a spot %.0f units away is not blocked, but the rule is %.0f" % [sep - 1.0, sep])
+	if not game_state.building_blocking(Vector2(sep + 1.0, 0.0)).is_empty():
+		return _fail("a spot %.0f units away is blocked, but the rule is %.0f" % [sep + 1.0, sep])
+	var blocker: Dictionary = game_state.building_blocking(Vector2.ZERO)
+	if String(blocker.get("type", "")) != GameState.BUILDING_SHIP_CRAFTER:
+		return _fail("the blocking structure is reported as '%s'" % blocker.get("type", ""))
+	print("a spot inside %.0f units of a structure is refused, outside it is clear" % sep)
+
+	# --- B. and the whole build path agrees -----------------------------
+	# On top of the crafter: refused, and nothing is built or spent.
+	fleet.teleport_to(Vector2.ZERO)
+	var before: int = game_state.buildings.size()
+	controller.pending_build_type = GameState.BUILDING_COMMAND_CENTER
+	controller._confirm_build()
+	if game_state.buildings.size() != before:
+		return _fail("a structure was built on top of the crafter")
+	print("the build path refuses it too: %s" % controller.build_menu.status.text)
+
+	# Clear of it: the same build goes through.
+	fleet.teleport_to(Vector2(sep * 3.0, 0.0))
+	controller.pending_build_type = GameState.BUILDING_COMMAND_CENTER
+	controller._confirm_build()
+	if game_state.buildings.size() != before + 1:
+		return _fail("the same build was refused on a clear spot")
+	print("clear of it, the same build goes through")
+
+	# --- C. a unique structure replaces itself in place -----------------
+	game_state.buildings.clear()
+	game_state.add_building(GameState.BUILDING_MOTHERSHIP, Vector2.ZERO)
+	fleet.teleport_to(Vector2.ZERO)
+	if not game_state.building_blocking(Vector2.ZERO, GameState.BUILDING_MOTHERSHIP).is_empty():
+		return _fail("a unique structure blocks its own replacement")
+	controller.pending_build_type = GameState.BUILDING_MOTHERSHIP
+	controller._confirm_build()
+	if game_state.buildings.size() != 1:
+		return _fail("rebuilding the mothership in place left %d structures" % game_state.buildings.size())
+	if String(game_state.buildings[0].type) != GameState.BUILDING_MOTHERSHIP:
+		return _fail("the rebuilt structure is not a mothership")
+	print("a unique structure can still be rebuilt where the old one stood")
+
+	# --- D. a refused rebuild demolishes nothing ------------------------
+	# The mothership is unique and is removed before the new one is placed.
+	# Something ELSE in the way has to be caught before that demolition, or
+	# the player is left with neither.
+	game_state.buildings.clear()
+	var keep: int = game_state.add_building(GameState.BUILDING_MOTHERSHIP, Vector2.ZERO)
+	game_state.add_building(GameState.BUILDING_SHIP_CRAFTER, Vector2(sep * 4.0, 0.0))
+	fleet.teleport_to(Vector2(sep * 4.0, 0.0))
+	controller.pending_build_type = GameState.BUILDING_MOTHERSHIP
+	controller._confirm_build()
+	if game_state.find_building(keep).is_empty():
+		return _fail("a refused rebuild demolished the existing mothership - the player is left with neither")
+	if game_state.buildings.size() != 2:
+		return _fail("a refused rebuild changed the structure count to %d" % game_state.buildings.size())
+	print("a refused rebuild leaves the existing structure standing")
+
+	print("TEST_BUILDING_SEPARATION: PASS")
+	quit(0)
+	return true
+
+
+func _fail(message: String) -> bool:
+	print("TEST_BUILDING_SEPARATION: FAIL (%s)" % message)
+	quit(1)
+	return true

tests/test_payments_use_fleet_hold.gd

@@ -270,6 +270,12 @@ func _process(delta: float) -> bool:
 			game_state.fleet_inventory.set_amount(GameState.RESOURCE_ORE, HOLD_ORE)
 			var before: int = game_state.buildings.size()
 
+			# #224: structures may not be stacked, and _check_ship_lab left a
+			# lab standing at the origin as a prerequisite for an earlier
+			# case. Fly clear of it - the subject here is which purse the ore
+			# comes out of, not where a mothership may stand.
+			scene.get_node("Fleet").teleport_to(
+				Vector2(GameState.MIN_BUILDING_SEPARATION * 4.0, 0.0))
 			build_controller.pending_build_type = GameState.BUILDING_MOTHERSHIP
 			build_controller._confirm_build()