fix issue where construction sites could be placed outside of game world and add tests

This commit is contained in:
2026-06-22 21:13:01 +02:00
parent 59688e6532
commit e5017ab3c5
6 changed files with 251 additions and 19 deletions

View File

@@ -235,12 +235,18 @@ std::vector<Item> BuildingSystem::rollReprocessingOutput(const RecipeDef& recipe
BuildingId BuildingSystem::place(BuildingType type, QPoint anchor, BuildingId BuildingSystem::place(BuildingType type, QPoint anchor,
Rotation rotation, Tick currentTick) Rotation rotation, Tick currentTick)
{ {
const BuildingId id = m_allocateBuildingId();
const BuildingDef* def = findBuildingDef(type); const BuildingDef* def = findBuildingDef(type);
assert(def != nullptr); assert(def != nullptr);
const ParsedSurfaceMask mask = parseSurfaceMask(def->surfaceMask, rotation); const ParsedSurfaceMask mask = parseSurfaceMask(def->surfaceMask, rotation);
// Reject placements that fall outside the world (REQ-BLD-PLACE-VALID).
if (!bodyCellsWithinWorldBounds(mask.bodyCells, anchor))
{
return kInvalidBuildingId;
}
const BuildingId id = m_allocateBuildingId();
// Record tile occupancy for body cells. // Record tile occupancy for body cells.
for (const QPoint& cell : mask.bodyCells) for (const QPoint& cell : mask.bodyCells)
{ {
@@ -270,6 +276,70 @@ BuildingId BuildingSystem::place(BuildingType type, QPoint anchor,
return id; return id;
} }
bool BuildingSystem::bodyCellsWithinWorldBounds(const std::vector<QPoint>& bodyCells,
QPoint anchor) const
{
const int heightTiles = m_config.world.heightTiles;
const int leftEdgeX = -m_config.world.regions.asteroidWidth_tiles;
for (const QPoint& cell : bodyCells)
{
const QPoint worldCell = anchor + cell;
if (worldCell.y() < 0 || worldCell.y() >= heightTiles)
{
return false;
}
if (worldCell.x() < leftEdgeX)
{
return false;
}
}
return true;
}
bool BuildingSystem::isPlacementValid(BuildingType type, QPoint anchor,
Rotation rotation) const
{
const BuildingDef* def = findBuildingDef(type);
if (def == nullptr)
{
return false;
}
const ParsedSurfaceMask mask = parseSurfaceMask(def->surfaceMask, rotation);
if (!bodyCellsWithinWorldBounds(mask.bodyCells, anchor))
{
return false;
}
// Terrain: ship-dock (S) cells must sit in space (x >= 0); all other body
// (A) cells must sit on the asteroid (x < 0). (REQ-BLD-PLACE-VALID)
for (const QPoint& cell : mask.bodyCells)
{
const QPoint worldCell = anchor + cell;
bool isShipDock = false;
for (const QPoint& dock : mask.shipDockCells)
{
if (dock == cell)
{
isShipDock = true;
break;
}
}
if (isShipDock)
{
if (worldCell.x() < 0)
{
return false;
}
}
else if (worldCell.x() >= 0)
{
return false;
}
}
return true;
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Demolish // Demolish
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------

View File

@@ -40,11 +40,23 @@ public:
std::mt19937& rng); std::mt19937& rng);
// -- Placement / demolish ------------------------------------------------ // -- Placement / demolish ------------------------------------------------
// Returns the new entity id. Belt and Splitter register with BeltSystem // Returns the new entity id, or kInvalidBuildingId if the placement falls
// directly; other types enter the construction queue. // outside the world bounds (vertical extent and asteroid left edge). Belt
// and Splitter register with BeltSystem directly; other types enter the
// construction queue. Terrain type (A vs S) is NOT checked here so that
// tests can stage arbitrary layouts; the player-facing entry point
// (Simulation::tryPlaceBuilding) enforces the full rule via isPlacementValid.
BuildingId place(BuildingType type, QPoint anchor, Rotation rotation, BuildingId place(BuildingType type, QPoint anchor, Rotation rotation,
Tick currentTick); Tick currentTick);
// Returns true if the placement satisfies REQ-BLD-PLACE-VALID terrain and
// world-bounds rules: every ship-dock (S) cell sits in space (x >= 0), every
// other body (A) cell sits on the asteroid (x < 0 and x >= the left edge),
// and every cell has 0 <= y < world.height_tiles. There is no right-side
// bound — space extends rightward. Tile occupancy is NOT checked here.
bool isPlacementValid(BuildingType type, QPoint anchor,
Rotation rotation) const;
// Remove a building or construction site by id. Returns the refund in // Remove a building or construction site by id. Returns the refund in
// building blocks (floor(cost * refundPercentage / 100)). Returns 0 for // building blocks (floor(cost * refundPercentage / 100)). Returns 0 for
// unknown ids. // unknown ids.
@@ -136,6 +148,9 @@ private:
void initShipyardBuffers(Building& b) const; void initShipyardBuffers(Building& b) const;
std::vector<Port> computeInputPorts(const Building& b) const; std::vector<Port> computeInputPorts(const Building& b) const;
std::vector<Item> rollReprocessingOutput(const RecipeDef& recipe); std::vector<Item> rollReprocessingOutput(const RecipeDef& recipe);
bool bodyCellsWithinWorldBounds(
const std::vector<QPoint>& bodyCells,
QPoint anchor) const;
const GameConfig& m_config; const GameConfig& m_config;
BeltSystem& m_belts; BeltSystem& m_belts;

View File

@@ -947,6 +947,11 @@ bool Simulation::isModuleSchematicUnlocked(const std::string& moduleId) const
BuildingId Simulation::tryPlaceBuilding(BuildingType type, QPoint anchor, Rotation rotation) BuildingId Simulation::tryPlaceBuilding(BuildingType type, QPoint anchor, Rotation rotation)
{ {
if (!m_buildingSystem->isPlacementValid(type, anchor, rotation))
{
return kInvalidBuildingId;
}
int cost = 0; int cost = 0;
for (const BuildingDef& def : m_config.buildings.buildings) for (const BuildingDef& def : m_config.buildings.buildings)
{ {

View File

@@ -589,6 +589,49 @@ TEST_CASE("Blueprint placement: insufficient blocks returns kInvalidBuildingId a
REQUIRE(sim.buildingBlocksStock() == blocksBeforeAttempt); REQUIRE(sim.buildingBlocksStock() == blocksBeforeAttempt);
} }
TEST_CASE("Simulation: tryPlaceBuilding rejects terrain-invalid placement and charges nothing",
"[blueprint]")
{
Simulation sim(loadConfig());
const int startBlocks = sim.buildingBlocksStock();
// A miner is all-asteroid; placing it in space (x >= 0) violates the terrain
// rule, so it must be rejected without consuming building blocks.
const BuildingId id =
sim.tryPlaceBuilding(BuildingType::Miner, QPoint(0, 0), Rotation::East);
REQUIRE(id == kInvalidBuildingId);
REQUIRE(sim.buildingBlocksStock() == startBlocks);
REQUIRE(sim.buildings().allSites().empty());
}
TEST_CASE("Simulation: tryPlaceBuilding accepts a valid asteroid spot, occupies tiles, charges cost",
"[blueprint]")
{
Simulation sim(loadConfig());
int minerCost = 0;
for (const BuildingDef& def : sim.config().buildings.buildings)
{
if (def.type == BuildingType::Miner) { minerCost = def.cost; break; }
}
REQUIRE(minerCost > 0);
const int startBlocks = sim.buildingBlocksStock();
// Miner mask ["AA","A>"] East at (-3,0) → all-asteroid body at
// (-3,0),(-2,0),(-3,1); a valid spot.
const BuildingId id =
sim.tryPlaceBuilding(BuildingType::Miner, QPoint(-3, 0), Rotation::East);
REQUIRE(id != kInvalidBuildingId);
REQUIRE(sim.buildingBlocksStock() == startBlocks - minerCost);
REQUIRE(sim.buildings().isTileOccupied(QPoint(-3, 0)));
REQUIRE(sim.buildings().isTileOccupied(QPoint(-2, 0)));
REQUIRE(sim.buildings().isTileOccupied(QPoint(-3, 1)));
// The output-port tile (1,1)+anchor = (-2,1) is not a body cell.
REQUIRE_FALSE(sim.buildings().isTileOccupied(QPoint(-2, 1)));
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Recipe / schematic capture and re-application // Recipe / schematic capture and re-application
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------

View File

@@ -64,6 +64,27 @@ static void runTicks(BuildingSystem& bs, BeltSystem& belts, int n, Tick& tick)
} }
} }
// Owns a BuildingSystem and its dependencies for placement-bounds tests.
struct PlacementFixture
{
GameConfig cfg = loadConfig();
BeltSystem belts{cfg.world.beltSpeed_tps};
int stock = 0;
std::mt19937 rng{0};
BuildingId nextBuildingId = 1;
BuildingSystem bs;
PlacementFixture()
: bs(cfg, belts,
[this]() { return nextBuildingId++; },
[this](int n) { stock += n; },
[](const std::string&, QVector2D, const std::optional<ShipLayoutConfig>&) {},
[](const std::string&) -> bool { return true; },
rng)
{
}
};
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Placement // Placement
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@@ -93,6 +114,88 @@ TEST_CASE("BuildingSystem: place miner occupies expected body tiles", "[building
REQUIRE_FALSE(bs.isTileOccupied(QPoint(1, 1))); REQUIRE_FALSE(bs.isTileOccupied(QPoint(1, 1)));
} }
// -- World-bounds rejection (REQ-BLD-PLACE-VALID) ---------------------------
TEST_CASE("BuildingSystem: place rejects a building above the world (y < 0)", "[building]")
{
PlacementFixture f;
// Miner mask ["AA","A>"] East → body at (0,0),(1,0),(0,1); at y=-1 the top
// row sits above the world.
const BuildingId id = f.bs.place(BuildingType::Miner, QPoint(0, -1), Rotation::East, 0);
REQUIRE(id == kInvalidBuildingId);
REQUIRE(f.bs.allSites().empty());
REQUIRE_FALSE(f.bs.isTileOccupied(QPoint(0, 0)));
}
TEST_CASE("BuildingSystem: place rejects a building below the world (y >= height)", "[building]")
{
PlacementFixture f;
const int heightTiles = f.cfg.world.heightTiles;
// Anchored on the last in-bounds row, the miner's lower body row reaches
// y == heightTiles, which is outside the world.
const BuildingId id = f.bs.place(BuildingType::Miner,
QPoint(0, heightTiles - 1), Rotation::East, 0);
REQUIRE(id == kInvalidBuildingId);
REQUIRE(f.bs.allSites().empty());
}
TEST_CASE("BuildingSystem: place rejects a building left of the asteroid edge", "[building]")
{
PlacementFixture f;
const int leftEdgeX = -f.cfg.world.regions.asteroidWidth_tiles;
const BuildingId id = f.bs.place(BuildingType::Miner,
QPoint(leftEdgeX - 1, 0), Rotation::East, 0);
REQUIRE(id == kInvalidBuildingId);
REQUIRE(f.bs.allSites().empty());
}
TEST_CASE("BuildingSystem: place accepts a building flush against the world's left edge",
"[building]")
{
PlacementFixture f;
const int leftEdgeX = -f.cfg.world.regions.asteroidWidth_tiles;
// Miner body min relative x is 0, so its leftmost cell sits exactly on the edge.
const BuildingId id = f.bs.place(BuildingType::Miner,
QPoint(leftEdgeX, 0), Rotation::East, 0);
REQUIRE(id != kInvalidBuildingId);
REQUIRE(f.bs.isTileOccupied(QPoint(leftEdgeX, 0)));
}
TEST_CASE("BuildingSystem: place imposes no right-side bound (space extends rightward)",
"[building]")
{
PlacementFixture f;
const BuildingId id = f.bs.place(BuildingType::Miner,
QPoint(1000, 0), Rotation::East, 0);
REQUIRE(id != kInvalidBuildingId);
}
TEST_CASE("BuildingSystem: isPlacementValid enforces terrain and world bounds", "[building]")
{
PlacementFixture f;
const int leftEdgeX = -f.cfg.world.regions.asteroidWidth_tiles;
// Miner is all-asteroid (A): valid only fully on the asteroid (x < 0).
REQUIRE(f.bs.isPlacementValid(BuildingType::Miner, QPoint(-3, 0), Rotation::East));
REQUIRE_FALSE(f.bs.isPlacementValid(BuildingType::Miner, QPoint(0, 0), Rotation::East)); // A cells in space
REQUIRE_FALSE(f.bs.isPlacementValid(BuildingType::Miner, QPoint(0, -1), Rotation::East)); // above world
REQUIRE(f.bs.isPlacementValid(BuildingType::Miner, QPoint(leftEdgeX, 0), Rotation::East));
REQUIRE_FALSE(f.bs.isPlacementValid(BuildingType::Miner,
QPoint(leftEdgeX - 1, 0), Rotation::East)); // past left edge
// Shipyard mask ["AAAS>","AAAS "] straddles the boundary: A cells on the
// asteroid, the S (dock) cell in space. At anchor (-3,0) the A cells land at
// x=-3..-1 and the dock at x=0.
REQUIRE(f.bs.isPlacementValid(BuildingType::Shipyard, QPoint(-3, 0), Rotation::East));
REQUIRE_FALSE(f.bs.isPlacementValid(BuildingType::Shipyard, QPoint(0, 0), Rotation::East)); // A cells in space
REQUIRE_FALSE(f.bs.isPlacementValid(BuildingType::Shipyard, QPoint(-4, 0), Rotation::East)); // dock on asteroid
}
TEST_CASE("BuildingSystem: placing a belt registers it with BeltSystem after construction", TEST_CASE("BuildingSystem: placing a belt registers it with BeltSystem after construction",
"[building]") "[building]")
{ {

View File

@@ -385,31 +385,27 @@ const BuildingDef* GameWorldView::findBuildingDef(BuildingType type) const
bool GameWorldView::isValidPlacement(BuildingType type, QPoint anchor, bool GameWorldView::isValidPlacement(BuildingType type, QPoint anchor,
Rotation rot) const Rotation rot) const
{ {
// Terrain and world-bounds validity are owned by the simulation
// (REQ-BLD-PLACE-VALID); the presentation layer only adds the occupancy /
// rotate-in-place check.
if (!m_sim->buildings().isPlacementValid(type, anchor, rot))
{
return false;
}
const BuildingDef* def = findBuildingDef(type); const BuildingDef* def = findBuildingDef(type);
if (!def) { return false; } if (!def) { return false; }
const ParsedSurfaceMask parsed = parseSurfaceMask(def->surfaceMask, rot); const ParsedSurfaceMask parsed = parseSurfaceMask(def->surfaceMask, rot);
bool anyOccupied = false; bool anyOccupied = false;
for (const QPoint& relCell : parsed.bodyCells) for (const QPoint& relCell : parsed.bodyCells)
{ {
const QPoint worldCell = anchor + relCell; if (m_sim->buildings().isTileOccupied(anchor + relCell))
// Terrain check: S cells must be space (x >= 0), A cells must be asteroid (x < 0)
bool isShipDock = false;
for (const QPoint& dock : parsed.shipDockCells)
{ {
if (dock == relCell) anyOccupied = true;
{
isShipDock = true;
break; break;
} }
} }
if (isShipDock && worldCell.x() < 0) { return false; }
if (!isShipDock && worldCell.x() >= 0) { return false; }
if (m_sim->buildings().isTileOccupied(worldCell)) { anyOccupied = true; }
}
if (anyOccupied) if (anyOccupied)
{ {