leave discovered entities in the inventory forever (#164)

Once an entity has been discovered, it stays in the inventory, even if its count becomes 0.

I think I just deleted the code @fryguybob carefully fixed in #157... =)

Fixes #163.

src/Swarm/Game/Entity.hs

@@ -1,5 +1,3 @@
------------------------------------------------------------------------------
------------------------------------------------------------------------------
 {-# LANGUAGE DeriveAnyClass #-}
 {-# LANGUAGE DeriveGeneric #-}
 {-# LANGUAGE FlexibleInstances #-}
@@ -70,6 +68,7 @@ module Swarm.Game.Entity (
   -- ** Lookup
   lookup,
   lookupByName,
+  countByName,
   contains,
   elems,
 
@@ -86,7 +85,7 @@ import Brick (Widget)
 import Control.Arrow ((&&&))
 import Control.Lens (Getter, Lens', lens, to, view, (^.))
 import Control.Monad.IO.Class
-import Data.Bifunctor (bimap, second)
+import Data.Bifunctor (bimap, first, second)
 import Data.Char (toLower)
 import Data.Function (on)
 import Data.Hashable
@@ -98,7 +97,7 @@ import qualified Data.IntSet as IS
 import Data.List (foldl')
 import Data.Map (Map)
 import qualified Data.Map as M
-import Data.Maybe (isJust)
+import Data.Maybe (fromMaybe, isJust, listToMaybe)
 import Data.Text (Text)
 import qualified Data.Text as T
 import GHC.Generics (Generic)
@@ -485,11 +484,23 @@ lookup :: Entity -> Inventory -> Count
 lookup e (Inventory cs _) = maybe 0 fst $ IM.lookup (e ^. entityHash) cs
 
 -- | Look up an entity by name in an inventory, returning a list of
---   matching entities.
+--   matching entities.  Note, if this returns some entities, it does
+--   *not* mean we necessarily have any in our inventory!  It just
+--   means we *know about* them.  If you want to know whether you have
+--   any, use 'lookup' and see whether the resulting 'Count' is
+--   positive, or just use 'countByName' in the first place.
 lookupByName :: Text -> Inventory -> [Entity]
 lookupByName name (Inventory cs byN) =
   maybe [] (map (snd . (cs IM.!)) . IS.elems) (M.lookup (T.toLower name) byN)
 
+-- | Look up an entity by name and see how many there are in the
+--   inventory.  If there are multiple entities with the same name, it
+--   just picks the first one returned from 'lookupByName'.
+countByName :: Text -> Inventory -> Count
+countByName name inv =
+  fromMaybe 0 $
+    flip lookup inv <$> listToMaybe (lookupByName name inv)
+
 -- | The empty inventory.
 empty :: Inventory
 empty = Inventory IM.empty M.empty
@@ -516,7 +527,7 @@ insertCount cnt e (Inventory cs byN) =
     (IM.insertWith (\(m, _) (n, _) -> (m + n, e)) (e ^. entityHash) (cnt, e) cs)
     (M.insertWith IS.union (T.toLower $ e ^. entityName) (IS.singleton (e ^. entityHash)) byN)
 
--- | Check whether an inventory contains a given entity.
+-- | Check whether an inventory contains at least one of a given entity.
 contains :: Inventory -> Entity -> Bool
 contains inv e = lookup e inv > 0
 
@@ -526,27 +537,20 @@ delete = deleteCount 1
 
 -- | Delete a specified number of copies of an entity from an inventory.
 deleteCount :: Count -> Entity -> Inventory -> Inventory
-deleteCount k e (Inventory cs byN) = Inventory cs' byN'
+deleteCount k e (Inventory cs byN) = Inventory cs' byN
  where
   cs' = IM.alter removeCount (e ^. entityHash) cs
-  newCount = lookup e (Inventory cs' byN)
-
-  byN'
-    | newCount == 0 = M.adjust (IS.delete (e ^. entityHash)) (T.toLower $ e ^. entityName) byN
-    | otherwise = byN
 
   removeCount :: Maybe (Count, a) -> Maybe (Count, a)
   removeCount Nothing = Nothing
-  removeCount (Just (n, a))
-    | k >= n = Nothing
-    | otherwise = Just (n - k, a)
+  removeCount (Just (n, a)) = Just (max 0 (n - k), a)
 
 -- | Delete all copies of a certain entity from an inventory.
 deleteAll :: Entity -> Inventory -> Inventory
 deleteAll e (Inventory cs byN) =
   Inventory
-    (IM.alter (const Nothing) (e ^. entityHash) cs)
-    (M.adjust (IS.delete (e ^. entityHash)) (T.toLower $ e ^. entityName) byN)
+    (IM.adjust (first (const 0)) (e ^. entityHash) cs)
+    byN
 
 -- | Get the entities in an inventory and their associated counts.
 elems :: Inventory -> [(Count, Entity)]

src/Swarm/Game/Step.hs

@@ -595,7 +595,10 @@ execConst c vs k = do
         -- Make sure the robot has the thing in its inventory
         e <-
           listToMaybe (lookupByName s inv)
-            `isJustOr` cmdExn Place ["You don't have", indefinite s, "to place."]
+            `isJustOr` cmdExn Place ["What is", indefinite s, "?"]
+
+        (E.lookup e inv > 0)
+          `holdsOr` cmdExn Place ["You don't have", indefinite s, "to place."]
 
         -- Place the entity and remove it from the inventory
         updateEntityAt loc (const (Just e))
@@ -620,7 +623,10 @@ execConst c vs k = do
         inv <- use robotInventory
         item <-
           (listToMaybe . lookupByName itemName $ inv)
-            `isJustOr` cmdExn Give ["You don't have", indefinite itemName, "to give."]
+            `isJustOr` cmdExn Give ["What is", indefinite itemName, "?"]
+
+        (E.lookup item inv > 0)
+          `holdsOr` cmdExn Give ["You don't have", indefinite itemName, "to give."]
 
         -- Giving something to ourself should be a no-op.  We need
         -- this as a special case since it will not work to modify
@@ -656,7 +662,10 @@ execConst c vs k = do
         inv <- use robotInventory
         item <-
           (listToMaybe . lookupByName itemName $ inv)
-            `isJustOr` cmdExn Install ["You don't have", indefinite itemName, "to install."]
+            `isJustOr` cmdExn Install ["What is", indefinite itemName, "?"]
+
+        (E.lookup item inv > 0)
+          `holdsOr` cmdExn Install ["You don't have", indefinite itemName, "to install."]
 
         myName <- use robotName
         focusedName <- lift . lift $ use focusedRobotName

test/Unit.hs

@@ -3,8 +3,6 @@
 -- | Swarm unit tests
 module Main where
 
-import Control.Lens
-
 import Data.Text (Text)
 import qualified Data.Text as T
 import Test.Tasty
@@ -14,14 +12,11 @@ import Swarm.Language.Pipeline
 import Swarm.Language.Pretty
 import Swarm.Language.Syntax hiding (mkOp)
 
-import Swarm.Game.Display (defaultRobotDisplay)
-import qualified Swarm.Game.Entity as E
-
 main :: IO ()
 main = defaultMain tests
 
 tests :: TestTree
-tests = testGroup "Tests" [parser, prettyConst, inventory]
+tests = testGroup "Tests" [parser, prettyConst]
 
 parser :: TestTree
 parser =
@@ -130,12 +125,3 @@ prettyConst =
  where
   equalPretty :: String -> Term -> Assertion
   equalPretty expected term = assertEqual "" expected . show $ ppr term
-
-inventory :: TestTree
-inventory =
-  testGroup
-    "Inventory"
-    [ testCase "byName case insensitive insert delete" $
-        let e = E.mkEntity defaultRobotDisplay "WaCkYcAsE" [] []
-         in assertEqual "" (E.empty & E.insert e & E.delete e & E.lookupByName (e ^. E.entityName)) []
-    ]