Artifact collision in plane module

Here's something I tried putting together as I'm learning. Critiques on anything are welcome. There's also a logic bug in the Plane module I can't identify.

The long and the short are that it takes the URI "some float/some float/some float/some float", and makes the first 2 (x,y) coord where something is, and the second 2 (x,y) coord where it wants to go. If there are no things overlapping the desired space, it will return the coords of the new location. If there is an overlap, it will return the original coords.

Extra points if you can find the logic bug I've been trying to find where it's thinking all coords are a collision with something.

plane.hs:

``
module Plane where

type X = Float
type Y = Float
type Direction = Float
type Location = (X, Y)
type Size = (Float, Float)
type TopLeftCorner = Location
type TopRightCorner = Location
type BottomLeftCorner = Location
type BottomRightCorner = Location

data Shape = Rectangle deriving (Eq, Show)

data Corner = RectangleCorners {
topLeftCorner :: TopLeftCorner,
topRightCorner :: TopRightCorner,
bottomRightCorner :: BottomRightCorner,
bottomLeftCorner :: BottomLeftCorner}

data Artifact = Artifact {
shape :: Shape,
location :: Location,
size :: Size } deriving (Eq, Show)

type Plane = [Artifact]

moveArtifact :: Plane -> Artifact -> Location -> Artifact
moveArtifact plane originalArtifact (moveToX, moveToY)
| artifactCanGoToLoc = Artifact Rectangle (moveToX, moveToY) $ size originalArtifact
| otherwise = originalArtifact
where artifactCorners = corners originalArtifact
artifactCanGoToLoc = not $
topLeftCorner artifactCorners inside plane ||
topRightCorner artifactCorners inside plane ||
bottomRightCorner artifactCorners inside plane ||
bottomLeftCorner artifactCorners inside` plane

corners :: Artifact -> Corner
corners (Artifact Rectangle (artifactX, artifactY) (artifactW,artifactH)) =
RectangleCorners

Here's the simplest rewrite I could come up with for your artifact collision detection algorithm:

module Plane where

type Location = (Float, Float)
type Size = (Float, Float)
data Artifact = Artifact { location :: Location, size :: Size }

xMin (Artifact (x, y) (w, h)) = x - w / 2
xMax (Artifact (x, y) (w, h)) = x + w / 2
yMin (Artifact (x, y) (w, h)) = y - h / 2
yMax (Artifact (x, y) (w, h)) = y + h / 2

type Plane = [Artifact]

moveArtifact :: Plane -> Location -> Artifact -> Maybe Artifact
moveArtifact plane newLoc oldArtifact =
   if newArtifact `collidesWith` plane then Nothing else Just newArtifact
  where newArtifact = oldArtifact { location = newLoc }

(locX, locY) `isInsideOf` a = xMin a < locX && locX < xMax a
                           && yMin a < locY && locY < yMax a

mobileArtifact `collidesWith` plane = not . or $ do
    x <- [xMin, xMax]
    y <- [yMin, yMax]
    let location = (x mobileArtifact, y mobileArtifact)
    existingArtifact <- plane
    return $ location `isInsideOf` existingArtifact

Some comments:

Your collision detection had two bugs. One was that you were mixing up X and Y coordinates:

lowerLeftX = (-) artifactY $ artifactH / 2
lowerLeftY = (-) artifactX $ artifactW / 2

I think you meant to switch those.

The second bug was that your insideAcc function always returned True when it hit the empty list, regardless of what Bool value it currently had stored. This is why your collision detection always registered a collision.

Your artifact movement also had a bug, in that you were checking the original position of the artifact for collisions and not the new position.

Your insideAcc function was more complicated than it needed to be. A much simpler version is to use the any or or versions from the Prelude:

any :: (a -> Bool) -> [a] -> Bool

any p returns true if the predicate p evaluate to True for any value in the list.

or :: [Bool] -> Bool
or = any id

or just evaluates a list of boolean values and returns True if at least one is true.

In my rewrite, I used the or function to see of the list of returned Bools had any Trues.

I rewrote moveArtifact to return a Maybe Artifact, otherwise you'd have to use floating point equality to tell if your Artifact moved, which would work but would be kind of weird. You can always recover your original behavior by using the fromMaybe function which extracts a value from a Maybe, providing a default value (i.e. your original artifact) if it is a Nothing.

The most important trick I used when rewriting your code was the list monad (i.e. list comprehensions). This is a very useful trick when you need to do something on various permutations of certain values. The collision checking function checks every permutation of the three lists (i.e. [xMin, xMax], [yMin, yMax], and plane) for collisions.

StackExchange Code Review Q#13035, answer score: 5