From 63e8f9dcef06b144e1e60e518b4abc916876d792 Mon Sep 17 00:00:00 2001 From: Karl Ostmo Date: Sun, 15 Sep 2024 23:25:48 -0700 Subject: [PATCH 1/4] add new test --- .../sequential-placement.yaml | 95 +++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml diff --git a/data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml b/data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml new file mode 100644 index 000000000..c007f7c9c --- /dev/null +++ b/data/scenarios/Testing/1780-structure-merge-expansion/sequential-placement.yaml @@ -0,0 +1,95 @@ +version: 1 +name: Flipped structure placement +author: Karl Ostmo +description: | + Sequentially place structures that are larger than the map + with flipped orientation. +robots: + - name: base + dir: north +creative: true +objectives: + - goal: + - Must have 3 of each color visible + condition: | + def countColor = \e. + resonate e ((0, 0), (10, -5)); + end; + + as base { + r <- countColor "pixel (R)"; + g <- countColor "pixel (G)"; + b <- countColor "pixel (B)"; + y <- countColor "gold"; + return $ r == 3 && g == 3 && b == 3 && y == 3; + } +solution: | + noop +known: [boulder, log, pixel (R), pixel (G), pixel (B), gold] +world: + structures: + - name: reddish + structure: + mask: '.' + palette: + 'x': [stone, pixel (R)] + map: | + xx + x. + - name: greenish + structure: + mask: '.' + palette: + 'x': [stone, pixel (G)] + map: | + xx + x. + - name: bluish + structure: + mask: '.' + palette: + 'x': [stone, pixel (B)] + map: | + xx + x. + - name: goldish + structure: + mask: '.' + palette: + 'x': [stone, gold] + map: | + xx + x. + - name: block + structure: + mask: '.' + palette: + 'x': [ice, log] + placements: + - src: greenish + orient: + flip: true + offset: [-3, 2] + - src: reddish + offset: [-6, 0] + - src: goldish + orient: + flip: true + offset: [3, -1] + - src: bluish + offset: [0, 1] + map: | + xxx + xx. + x.. + palette: + 'Ω': [grass, erase, base] + mask: '.' + placements: + - src: block + offset: [0, -1] + upperleft: [0, 0] + dsl: | + {grass} + map: | + Ω From b14368ceae32a2930e4c640d8cf4799d7ec2f504 Mon Sep 17 00:00:00 2001 From: Karl Ostmo Date: Sun, 18 Aug 2024 15:33:24 -0700 Subject: [PATCH 2/4] fix positioning bug with negative coordinates --- .../00-ORDER.txt | 3 +- .../circle-and-crosses.yaml | 2 +- .../Scenario/Topography/Structure/Assembly.hs | 23 +++++--- .../Scenario/Topography/Structure/Overlay.hs | 7 +-- test/integration/Main.hs | 7 +++ test/standalone-topography/src/Lib.hs | 11 ++-- test/standalone-topography/src/Main.hs | 59 +++++++++++-------- 7 files changed, 69 insertions(+), 43 deletions(-) diff --git a/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt b/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt index 8580fa46e..ed9116ef4 100644 --- a/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt +++ b/data/scenarios/Testing/1780-structure-merge-expansion/00-ORDER.txt @@ -1,3 +1,4 @@ nonoverlapping-structure-merge.yaml root-map-expansion.yaml -structure-composition.yaml \ No newline at end of file +structure-composition.yaml +sequential-placement.yaml \ No newline at end of file diff --git a/data/test/standalone-topography/circle-and-crosses.yaml b/data/test/standalone-topography/circle-and-crosses.yaml index e5f650bd1..dada1adfd 100644 --- a/data/test/standalone-topography/circle-and-crosses.yaml +++ b/data/test/standalone-topography/circle-and-crosses.yaml @@ -19,7 +19,7 @@ structures: fff placements: - src: beam - offset: [0, 3] + offset: [0, 0] - src: beam offset: [-3, -3] orient: diff --git a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs index 06a6e12ac..c78f1398c 100644 --- a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs +++ b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs @@ -22,7 +22,6 @@ import Data.Text qualified as T import Linear.Affine import Swarm.Game.Location import Swarm.Game.Scenario.Topography.Area -import Swarm.Game.Scenario.Topography.Grid import Swarm.Game.Scenario.Topography.Navigation.Waypoint import Swarm.Game.Scenario.Topography.Placement import Swarm.Game.Scenario.Topography.Structure @@ -42,14 +41,14 @@ overlaySingleStructure :: Either Text (MergedStructure (Maybe a)) overlaySingleStructure inheritedStrucDefs - (Placed p@(Placement _ pose@(Pose loc orientation)) ns) + (Placed p@(Placement _sName pose@(Pose loc orientation)) ns) (MergedStructure inputArea inputPlacements inputWaypoints) = do MergedStructure overlayArea overlayPlacements overlayWaypoints <- mergeStructures inheritedStrucDefs (WithParent p) $ structure ns let mergedWaypoints = inputWaypoints <> map (fmap $ placeOnArea overlayArea) overlayWaypoints mergedPlacements = inputPlacements <> map (placeOnArea overlayArea) overlayPlacements - mergedArea = overlayGridExpanded (gridContent inputArea) pose overlayArea + mergedArea = overlayGridExpanded inputArea pose overlayArea return $ MergedStructure mergedArea mergedPlacements mergedWaypoints where @@ -81,6 +80,8 @@ mergeStructures inheritedStrucDefs parentPlacement (Structure origArea subStruct map wrapPlacement $ filter (\(Placed _ ns) -> isRecognizable ns) overlays + -- NOTE: Each successive overlay may alter the coordinate origin. + -- We make sure this new origin is propagated to subsequent sibling placements. foldlM (flip $ overlaySingleStructure structureMap) (MergedStructure origArea wrappedOverlays originatedWaypoints) @@ -97,18 +98,22 @@ mergeStructures inheritedStrucDefs parentPlacement (Structure origArea subStruct -- * Grid manipulation overlayGridExpanded :: - Grid (Maybe a) -> + PositionedGrid (Maybe a) -> Pose -> PositionedGrid (Maybe a) -> PositionedGrid (Maybe a) overlayGridExpanded - inputGrid - (Pose loc orientation) - (PositionedGrid _ overlayArea) = - PositionedGrid origin inputGrid <> positionedOverlay + baseGrid + (Pose yamlPlacementOffset orientation) + -- NOTE: The '_childAdjustedOrigin' is the sum of origin adjustments + -- to completely assemble some substructure. However, we discard + -- this when we place a substructure into a new base grid. + (PositionedGrid _childAdjustedOrigin overlayArea) = + baseGrid <> positionedOverlay where reorientedOverlayCells = applyOrientationTransform orientation overlayArea - positionedOverlay = PositionedGrid loc reorientedOverlayCells + placementAdjustedByOrigin = gridPosition baseGrid .+^ (yamlPlacementOffset .-. origin) + positionedOverlay = PositionedGrid placementAdjustedByOrigin reorientedOverlayCells -- * Validation diff --git a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs index 86444ed5a..f666b7198 100644 --- a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs +++ b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs @@ -101,12 +101,11 @@ instance (Alternative f) => Semigroup (PositionedGrid (f a)) where mergedSize = computeMergedArea $ OverlayPair a1 a2 combinedGrid = zipGridRows mergedSize paddedOverlayPair - -- We subtract the base origin from the - -- overlay position, such that the displacement vector - -- will have: + -- We create a vector from the overlay position, + -- such that the displacement vector will have: -- \* negative X component if the origin must be shifted east -- \* positive Y component if the origin must be shifted south - originDelta@(V2 deltaX deltaY) = overlayLoc .-. baseLoc + originDelta@(V2 deltaX deltaY) = overlayLoc .-. origin -- Note that the adjustment vector will only ever have -- a non-negative X component (i.e. loc of upper-left corner must be shifted east) and -- a non-positive Y component (i.e. loc of upper-left corner must be shifted south). diff --git a/test/integration/Main.hs b/test/integration/Main.hs index 9f0fa9c8c..b9e200b3e 100644 --- a/test/integration/Main.hs +++ b/test/integration/Main.hs @@ -440,6 +440,13 @@ testScenarioSolutions rs ui key = [ testSolution Default "Testing/1535-ping/1535-in-range" , testSolution Default "Testing/1535-ping/1535-out-of-range" ] + , testGroup + "Structure placement (#1780)" + [ testSolution Default "Testing/1780-structure-merge-expansion/sequential-placement" + -- , testSolution Default "Testing/1780-structure-merge-expansion/nonoverlapping-structure-merge" + -- , testSolution Default "Testing/1780-structure-merge-expansion/root-map-expansion" + -- , testSolution Default "Testing/1780-structure-merge-expansion/structure-composition" + ] , testGroup "Structure recognition (#1575)" [ testSolution Default "Testing/1575-structure-recognizer/1575-browse-structures" diff --git a/test/standalone-topography/src/Lib.hs b/test/standalone-topography/src/Lib.hs index 1fa47d2c5..c73991d8e 100644 --- a/test/standalone-topography/src/Lib.hs +++ b/test/standalone-topography/src/Lib.hs @@ -32,8 +32,12 @@ parseStructures dataDir baseFilename = do dataDir "test/standalone-topography" baseFilename return $ forceEither $ left prettyPrintParseException eitherResult -compareToReferenceImage :: FilePath -> Assertion -compareToReferenceImage fileStem = do +compareToReferenceImage :: + -- | set this to update the golden tests + Bool -> + FilePath -> + Assertion +compareToReferenceImage refreshReferenceImage fileStem = do dataDir <- getDataDir parentStruct <- parseStructures dataDir $ fileStem <.> "yaml" let MergedStructure overlayArea _ _ = forceEither $ mergeStructures mempty Root parentStruct @@ -44,6 +48,3 @@ compareToReferenceImage fileStem = do else do decodedImg <- LBS.readFile referenceFilepath assertEqual "Generated image must equal reference image!" decodedImg encodedImgBytestring - where - -- Manually toggle this to update the golden tests - refreshReferenceImage = False diff --git a/test/standalone-topography/src/Main.hs b/test/standalone-topography/src/Main.hs index cd8438307..399d21a04 100644 --- a/test/standalone-topography/src/Main.hs +++ b/test/standalone-topography/src/Main.hs @@ -4,33 +4,46 @@ -- SPDX-License-Identifier: BSD-3-Clause module Main where -import Test.Tasty (defaultMain, testGroup) +import Data.Proxy +import Data.Typeable (Typeable) +import Lib (compareToReferenceImage) +import Test.Tasty import Test.Tasty.HUnit (testCase) +import Test.Tasty.Options -import Lib +newtype UpdateGoldenTests = UpdateGoldenTests Bool + deriving (Eq, Ord, Typeable) + +instance IsOption UpdateGoldenTests where + parseValue = fmap UpdateGoldenTests . safeRead + defaultValue = UpdateGoldenTests False + optionName = return "refresh" + optionHelp = return "Should overwrite the golden test images" + optionCLParser = mkFlagCLParser mempty (UpdateGoldenTests True) main :: IO () main = do - defaultMain $ - testGroup - "Test structure assembly" - [ mkGroup - "Black and white" - [ "circle-and-crosses" - , "checkerboard" - ] - , mkGroup - "Color" - [ "rainbow" + defaultMainWithIngredients ingredients $ askOption $ \(UpdateGoldenTests shouldRefreshTests) -> + let doTest stem = + testCase (unwords ["Image equality:", stem]) $ + compareToReferenceImage shouldRefreshTests stem + + mkGroup title members = + testGroup title $ + map + doTest + members + in testGroup + "Test structure assembly" + [ mkGroup + "Black and white" + [ "circle-and-crosses" + , "checkerboard" + ] + , mkGroup + "Color" + [ "rainbow" + ] ] - ] where - doTest stem = - testCase (unwords ["Image equality:", stem]) $ - compareToReferenceImage stem - - mkGroup title members = - testGroup title $ - map - doTest - members + ingredients = includingOptions [Option (Proxy :: Proxy UpdateGoldenTests)] : defaultIngredients From 5455c996a61265921e08efc2536acfc89866ecc2 Mon Sep 17 00:00:00 2001 From: Karl Ostmo Date: Sun, 15 Sep 2024 23:20:43 -0700 Subject: [PATCH 3/4] helper function --- .../Swarm/Game/Scenario/Topography/Navigation/Portal.hs | 2 +- src/swarm-topography/Swarm/Game/Location.hs | 5 +++++ .../Swarm/Game/Scenario/Topography/Structure/Assembly.hs | 2 +- .../Swarm/Game/Scenario/Topography/Structure/Overlay.hs | 2 +- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/swarm-scenario/Swarm/Game/Scenario/Topography/Navigation/Portal.hs b/src/swarm-scenario/Swarm/Game/Scenario/Topography/Navigation/Portal.hs index 96e6ed114..5db64fc72 100644 --- a/src/swarm-scenario/Swarm/Game/Scenario/Topography/Navigation/Portal.hs +++ b/src/swarm-scenario/Swarm/Game/Scenario/Topography/Navigation/Portal.hs @@ -179,7 +179,7 @@ validatePartialNavigation currentSubworldName upperLeft unmergedWaypoints portal correctedWaypoints = binTuples $ map - (\x -> (wpName $ wpConfig $ value x, fmap (offsetLoc $ upperLeft .-. origin) x)) + (\x -> (wpName $ wpConfig $ value x, fmap (offsetLoc $ asVector upperLeft) x)) unmergedWaypoints bareWaypoints = M.map (NE.map extractLoc) correctedWaypoints waypointsWithUniqueFlag = M.filter (any $ wpUnique . wpConfig . value) correctedWaypoints diff --git a/src/swarm-topography/Swarm/Game/Location.hs b/src/swarm-topography/Swarm/Game/Location.hs index f1a80ee01..884705733 100644 --- a/src/swarm-topography/Swarm/Game/Location.hs +++ b/src/swarm-topography/Swarm/Game/Location.hs @@ -29,6 +29,7 @@ module Swarm.Game.Location ( -- ** Utility functions manhattan, euclidean, + asVector, getLocsInArea, getElemsInArea, @@ -199,6 +200,10 @@ manhattan (Location x1 y1) (Location x2 y2) = abs (x1 - x2) + abs (y1 - y2) euclidean :: Location -> Location -> Double euclidean p1 p2 = norm (fromIntegral <$> (p2 .-. p1)) +-- | Converts a 'Point' to a vector offset from the 'origin'. +asVector :: Location -> V2 Int32 +asVector loc = loc .-. origin + -- | Get all the locations that are within a certain manhattan -- distance from a given location. -- diff --git a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs index c78f1398c..3c620ef99 100644 --- a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs +++ b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Assembly.hs @@ -112,7 +112,7 @@ overlayGridExpanded baseGrid <> positionedOverlay where reorientedOverlayCells = applyOrientationTransform orientation overlayArea - placementAdjustedByOrigin = gridPosition baseGrid .+^ (yamlPlacementOffset .-. origin) + placementAdjustedByOrigin = gridPosition baseGrid .+^ asVector yamlPlacementOffset positionedOverlay = PositionedGrid placementAdjustedByOrigin reorientedOverlayCells -- * Validation diff --git a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs index f666b7198..cb95e82a1 100644 --- a/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs +++ b/src/swarm-topography/Swarm/Game/Scenario/Topography/Structure/Overlay.hs @@ -105,7 +105,7 @@ instance (Alternative f) => Semigroup (PositionedGrid (f a)) where -- such that the displacement vector will have: -- \* negative X component if the origin must be shifted east -- \* positive Y component if the origin must be shifted south - originDelta@(V2 deltaX deltaY) = overlayLoc .-. origin + originDelta@(V2 deltaX deltaY) = asVector overlayLoc -- Note that the adjustment vector will only ever have -- a non-negative X component (i.e. loc of upper-left corner must be shifted east) and -- a non-positive Y component (i.e. loc of upper-left corner must be shifted south). From ffaf710800f1b71bca4b4daa44ea8aefc9d0c1ce Mon Sep 17 00:00:00 2001 From: Karl Ostmo Date: Mon, 16 Sep 2024 12:43:12 -0700 Subject: [PATCH 4/4] add todo reference --- scripts/validate/issues-for-todos.sh | 2 +- test/integration/Main.hs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/validate/issues-for-todos.sh b/scripts/validate/issues-for-todos.sh index 55e9153e1..41de14ac1 100755 --- a/scripts/validate/issues-for-todos.sh +++ b/scripts/validate/issues-for-todos.sh @@ -3,7 +3,7 @@ cd $(git rev-parse --show-toplevel) -if grep --line-number --include \*.hs -riP '(TODO|FIXME|XXX)\b' src app 2>&1 | grep -vP '#\d+'; then +if grep --line-number --include \*.hs -riP '(TODO|FIXME|XXX)\b' src app test 2>&1 | grep -vP '#\d+'; then echo "Please add a link to Issue, for example: TODO: #123" exit 1 else diff --git a/test/integration/Main.hs b/test/integration/Main.hs index b9e200b3e..a2c72b1ea 100644 --- a/test/integration/Main.hs +++ b/test/integration/Main.hs @@ -443,6 +443,7 @@ testScenarioSolutions rs ui key = , testGroup "Structure placement (#1780)" [ testSolution Default "Testing/1780-structure-merge-expansion/sequential-placement" + -- TODO(#2148) define goal conditions or convert to image fixtures -- , testSolution Default "Testing/1780-structure-merge-expansion/nonoverlapping-structure-merge" -- , testSolution Default "Testing/1780-structure-merge-expansion/root-map-expansion" -- , testSolution Default "Testing/1780-structure-merge-expansion/structure-composition"