225 lines
14 KiB
Markdown
225 lines
14 KiB
Markdown
|
# Jack Attack
|
||
|
|
||
|
* Your comments are very thorough! Have you, however, consider the special
|
||
|
"haddock" syntax for your documentation? For instance, in your `Object` record,
|
||
|
you clearly describe what each field does. If you used the "haddock" syntax
|
||
|
(maybe something like `--^ comment goes here`), you'd be able to generate
|
||
|
static documentation pages much like those you see on hackage or stackage.
|
||
|
Those are useful because they allow the user to browse the documentation
|
||
|
for your package from the web before deciding to install and use it. And,
|
||
|
of course, I think it's nicer to look at a web page than comments!
|
||
|
|
||
|
* There's a lot of duplicate functionality in your `doTimedTransform`
|
||
|
function compared to your `doTransform` function. It seems almost
|
||
|
as though there should be _one_ function that takes care of both
|
||
|
the interpolation and the transformation; perhaps `doTransform`
|
||
|
can take an argument `0.0..1.0` of "progress", and go from there.
|
||
|
This will save you the trouble of unpacking and repacking your
|
||
|
`Transformation` objects. Even if you don't go for that approach,
|
||
|
however, there's still an improvement you can make. Rather than making
|
||
|
your `doTimedTransform` function do _everything_, you may want to define
|
||
|
a function
|
||
|
|
||
|
```Haskell
|
||
|
tween :: Object -> Float -> Transformation -> Transformation
|
||
|
```
|
||
|
|
||
|
This function would do _kind of_ like what your `doTimeTransform` does:
|
||
|
it would interpolate the transformation given the current object and
|
||
|
the "progress" within the transformation. The difference, though,
|
||
|
is that it will not actually _apply_ the transformation, but create
|
||
|
a new one (much like you're already doing in `doTimedTransform` anyway).
|
||
|
You would then be able to feed this transformation into `doTransform`,
|
||
|
and it would work as expected. The advantage here is that you won't have
|
||
|
to be writing duplicate code for stuff like:
|
||
|
|
||
|
```
|
||
|
doTransform (Combine xs) $ doTransform x obj
|
||
|
doTimedTransform (seconds, x) (doTimedTransform (seconds, Combine xs) obj elapsed) elapsed
|
||
|
```
|
||
|
|
||
|
There's a fairly clear similarity (and consequently duplication) here, and
|
||
|
that's what I think my approach would suggest.
|
||
|
|
||
|
And there's one more thing: does `tween` _really_ need access to the object
|
||
|
being animated? It sure seems to if your transformations are "absolute",
|
||
|
like "move to (1,2)". This "absoluteness" of transformations forces you to
|
||
|
use `obj` in `doTimedTransform`, and would also force you to use it in
|
||
|
`tween`. But if your transformations were _delta based_ ("move left 1, up t2"), your `tween` could be only a function of the current "progress" `Float`:
|
||
|
|
||
|
```
|
||
|
tween :: Float -> Transformation -> Transformation
|
||
|
```
|
||
|
|
||
|
This may help clean up some of the code, and it also allows for an
|
||
|
interesting experiment: what if you compose `tween` with various
|
||
|
functions `Float -> Float`? For instance, you could compose it with
|
||
|
`\x -> x * x`, which I think would make your animation start out slow
|
||
|
and speed up at the end. You may be able to use this to play around
|
||
|
with various ways of animating your objects that are used in "actual"
|
||
|
animation: from what I know, linear transformations are not as common
|
||
|
as, say, sigmoid ones (animation starts slow, gets fast in the middle,
|
||
|
and slows down again at the end).
|
||
|
|
||
|
* I would try to factor out the "seconds" part of your calculations as soon
|
||
|
as possible. In my opinion, it would be clearer to pass around a "percentage
|
||
|
completed" float, especially if you don't have any other dependence on
|
||
|
the current time.
|
||
|
|
||
|
* Cody is right, you do have a couple of places that can be used by folds
|
||
|
or other higher order functions. For instance, `getAniLength` is just
|
||
|
`sum . map fst`, and the `Combine` case of `doTransform` is something
|
||
|
like `foldr (flip doTransform) obj (x:xs)`.
|
||
|
|
||
|
* Are transformations a monoid? I suspect that they are, with `mempty = Wait`,
|
||
|
`mappend x y = Combine [x,y]`, and `mconcat = Combine`. I don't know
|
||
|
if this will be useful, but it's always nice to have access to more
|
||
|
standard library functions!
|
||
|
|
||
|
* I'm not sure I follow the `AST` vs `Lib` modules. They both seem
|
||
|
to have _something_ in mind, but I can't quite figure out how they work
|
||
|
together. Does the `AST` eventually produce a `Transformation`? I just
|
||
|
don't really know what to make of it!
|
||
|
|
||
|
About your design questions:
|
||
|
|
||
|
* Q1: It would be cool if you had a slider that lets you play
|
||
|
the animation back and forward (or at an arbitrary point in time).
|
||
|
I think it may help in debugging:
|
||
|
|
||
|
> Why did my object just make a 360 and walk away?
|
||
|
|
||
|
It would be even better if you could label your transformations
|
||
|
with the function / piece of code that induced them, so that
|
||
|
at any point, you can kind of see what part of your code is
|
||
|
causing the current transformation.
|
||
|
|
||
|
* Q2: At the implementation level, you may need to replace the
|
||
|
`Float` argument of `doTransform` with an `Env` argument,
|
||
|
which may be a record-like thing that contains the current
|
||
|
mouse position, the current time, and so on. You'd then
|
||
|
thread it as before.
|
||
|
|
||
|
At a language level, things are more interesting. Have you
|
||
|
looked into [functional reactive programming](https://en.wikipedia.org/wiki/Functional_reactive_programming)?
|
||
|
You may be interested in encoding transformations declaratively, given, say
|
||
|
the current mouse position, time, etc. as input. Your programs would then
|
||
|
be descriptions of how to convert these inputs into figures on the screen.
|
||
|
|
||
|
[Elm](elm-lang.org) is a language that implements quasi-FRP. It doesn't deal with
|
||
|
continuous inputs like you would be, but it encodes programs as transition
|
||
|
functions (`Event -> State -> State`). You may be interested in adapting
|
||
|
a similar approach - it's a nice way to deal with "imperative" environments
|
||
|
from a functional language.
|
||
|
|
||
|
* Q3: This is a tough one! I think you can come up with a textual functional
|
||
|
language that can be used for your domain, but I'd be thrilled to see how
|
||
|
you may incorporate visual (scratch-like) elements into your project. At this
|
||
|
point, though, I can't help but wonder: would you have time to implement something
|
||
|
like Scratch-like blocks? We're in week 8, after all. That would be a very graphical
|
||
|
type of user interface, and I can't personally imagine defining a UI from scratch,
|
||
|
in Haskell, using only graphics library primitives. Of course, you're probably
|
||
|
more versed than I am in Haskell's graphics tooling, but it seems to me like
|
||
|
a fairly daunting task.
|
||
|
|
||
|
# Escape
|
||
|
|
||
|
* I played with your game for a bit, pretty cool! I think you did a nice job with the textual
|
||
|
users interface; it feels pretty much like I'd expect a command line interface to work.
|
||
|
I did run into an error on my first playthrough, and decided not to try again: `Prelude.head: empty list`.
|
||
|
This happened because I typed `search` without any arguments. Now, I understand that
|
||
|
what I did was an incorrect command, but crashing the game doesn't seem like the right thing to do!
|
||
|
`head` is a dangerous function; I think that if Haskell were redesigned today, from the ground
|
||
|
up, this function would no longer have type `[a] -> a`. In general, exceptions in Haskell are
|
||
|
a bit of an antipattern, because they're not indicated by the types (how could you know `head` throws
|
||
|
an exception), but also because they're cumbersome to work with!
|
||
|
|
||
|
Languages defined after Haskell actually define `head` with a `Maybe`. Here's the [Idris version](https://github.com/idris-lang/Idris-dev/blob/a13caeb4e50d0c096d34506f2ebf6b9d140a07aa/libs/prelude/Prelude/List.idr#L128-L130),
|
||
|
and here's the [Elm version](https://package.elm-lang.org/packages/elm/core/latest/List#head). You
|
||
|
may want to define your own head function, `safeHead :: [a] -> Maybe a`! You can then use pattern
|
||
|
matching or the `maybe` function to properly handle the error that occurs when the user doesn't give
|
||
|
the command enough arguments. You could, for instance, print "Search what?" when a user types in "search".
|
||
|
|
||
|
* This is a minor nitpick, but I'm pretty sure `snake_case` is not the preferred standard
|
||
|
for Haskell variable names. Your records (and module names!) should be in `camelCase`.
|
||
|
In fact, my linter plugin _covers_ the screen with warnings about style when it sees your code!
|
||
|
* `Funcfunctor` is a funny name for a typeclass. It seems to imply that `t` is a functor; however,
|
||
|
`Functor` is a type constructor class, whereas your code treats `t` as just a type, not a type
|
||
|
constructor! In fact, I'm really not sure what to make of the word `Funcfunctor`. What does
|
||
|
it mean?
|
||
|
* On the other hand, I think that this is a good application of type classes. If you want to
|
||
|
print a variety of different objects on the screen in your command line game, defining a common
|
||
|
type class with the various properties of these objects is a great idea. You can certainly
|
||
|
get a very consistent interface this way!
|
||
|
* In `Escape_funs`, a lot of your pattern matching code can be significantly simplified! You always write
|
||
|
out stuff like `Object {object_id=id, object_name=...}`, but use only one of these variables (for instance, `id`).
|
||
|
Instead, you can _only_ write `Object {object_id=id}` -- that's it! You don't need to manually enumerate every single
|
||
|
key in your records to pattern match on them.
|
||
|
|
||
|
A cute fun fact of this notation: it's "preferred style" according to Haskell's linter to rewrite code like `(Constructor _ _ _ _)`
|
||
|
(pattern matching on a data type and ignoring all its fields) as `Constructor{}`.
|
||
|
* `foldr (||) False (map (==(get_key obj)) list)` is good use of a fold a map, but it also
|
||
|
corresponds to the Haskell function [`any`](http://zvon.org/other/haskell/Outputprelude/any_f.html):
|
||
|
`any (==(get_key obj)) list`.
|
||
|
* Notice how your `run_code` is a bit fragile: your `help` function prints out all the commands available
|
||
|
to the user, while `run_code` actually implements these commands. It must be very easy to
|
||
|
edit `run_code` to add a new command, while forgetting to update `help`. You may be able
|
||
|
to work around this by defining a `Command` data structure: maybe it'll have a `description`
|
||
|
field that you print to the user, and another `action` filed that's maybe some monad like `IO ()`
|
||
|
that actually performs the actions that your command does. Then, you can have a global
|
||
|
list of commands `commands :: Map String Command`, which contains all the allowed commands
|
||
|
in your game. Your `help` function would go through this `commands` variable and print
|
||
|
out the `description` field of every available command; on the other hand, the `run_code`
|
||
|
function would read a command from the user, look it up in `commands`, and try to execute
|
||
|
the `action` field.
|
||
|
* Notice that you're explicitly handling your application's state: you will need to pass
|
||
|
the correct house to `run_code` every time, which means that for each command, you need
|
||
|
a separate call to `run_code` with whatever house that command puts your player in.
|
||
|
|
||
|
An alternative approach is to handle state _implicitly_. You won't need to take a `house`
|
||
|
argument explicitly, and you therefore won't have to explicitly pass it to your recursive
|
||
|
calls to `run_code`. In your case, this would be best done with a `State` monad, since
|
||
|
the current location of the player is, well, a piece of state.
|
||
|
|
||
|
The question, though, becomes: how the heck do you incorporate a state monad into your
|
||
|
`IO`-based code? The answer to that is the `StateT` _monad transformer_. We haven't learned
|
||
|
about these in class just yet, but we will soon, and I think it would serve you well! Instead
|
||
|
of making your code of type `IO ()` (what it currently is), you'd use something like
|
||
|
`StateT [Room] IO ()`. Your calls to `putStrLn` would have to go through a `liftIO`,
|
||
|
but you'd also gain access to functions `get` and `put`, which are the functions of the
|
||
|
State monad that we have covered in class. Then, whenever your user goes to a different
|
||
|
house, you'd run `put new_house`, and whenever you need to check the current location
|
||
|
of the player, you'd run `get`.
|
||
|
|
||
|
You may then incorporate other monad transformers into this implementation; I think
|
||
|
of interest to you would be `ExceptT` (which is __not the same as Haskell's exceptions
|
||
|
that I mentioned in my first bullet point!__), which might help you exit out of your
|
||
|
code when your player types "exit".
|
||
|
* By the way, you should probably explicitly type out the signatures of your function;
|
||
|
I think that's considered good style. For instance, for `run_code`, you'd write:
|
||
|
|
||
|
```
|
||
|
run_code :: Player -> [Room] -> IO ()
|
||
|
run_code plauer house = ...
|
||
|
```
|
||
|
|
||
|
Your questions:
|
||
|
|
||
|
* Q1: Hey, I brought up `StateT` and `ExceptT` and monad transformers in general earlier!
|
||
|
Check them out. I think you may find them useful for what you're doing. The only difficulty
|
||
|
is that you really need to see a few examples of code before-and-after the use of State
|
||
|
and Exception monads to be comfortable in applying them (at least for me! It took me a while
|
||
|
to properly understand monads, and more time to understand monad transformers).
|
||
|
|
||
|
* Q2: Haskell's `IO` monad has `readFile` and `writeFile` methods, but these may be a bit
|
||
|
basic for your taste. You may be interested in various formats like [INI](https://hackage.haskell.org/package/ini-0.4.1/docs/Data-Ini.html)
|
||
|
or even JSON (for which you can use something like [Aeson](https://hackage.haskell.org/package/aeson-1.5.6.0/docs/Data-Aeson.html),
|
||
|
especially its `encodeFile` and `decodeFile` methods).
|
||
|
|
||
|
|
||
|
I'm running out of steam writing this comment, but I may post a follow up if I think of anything else!
|
||
|
|
||
|
__Eric, if you're reading this:__ have I been evangelizing monad transformers too much? I feel like
|
||
|
I have been. It would be good to know if I'm being overzealous; everything starts to look like a nail
|
||
|
when all you have is a hammer!
|