jonascarpay / apecs

a fast, extensible, type driven Haskell ECS framework for games
392 stars 43 forks source link

cmapMIf and cmapM_If #71

Closed jeyj0 closed 4 years ago

jeyj0 commented 4 years ago

Is there a reason cmapMIf and cmapM_If are not implemented (other than: they didn't seem necessary, or nobody has needed them yet)? I just tried implementing cmapM_If myself, but maybe they should actually be included in the library itself? What do you think?


Here's my attempt at an implementation for cmapM_If (It doesn't compile though, and I'm too much of a haskell beginner to fix it...):

cmapM_If :: forall w m cp c.
  ( Get w m cp
  , Get w m c
  , Members w m c
  )
  => (cp -> Bool)
  -> (c -> SystemT w m ())
  -> SystemT w m ()
cmapM_If cond sys = do
  sp :: Storage cp <- getStore
  s :: Storage c <- getStore
  sl <- lift $ explMembers (s, sp)
  U.forM_ sl $ \ety -> do
    p <- explGet sp ety
    when (cond p) $ do
      x <- lift $ explGet s ety
      sys x
jonascarpay commented 4 years ago

Hey, thanks for your PR.

It doesn't compile though

The issue is that you need to lift both explGet calls.

Is there a reason cmapMIf and cmapM_If are not implemented (other than: they didn't seem necessary, or nobody has needed them yet)?

There are too many possible variants to include every single one. The space of cmap-like functions contains at least the following axes:

It doesn't really make sense to include all possible versions, so we kind of have to draw a line somewhere. Currently, that line is that I just write what I need and then people add what they need in PR's, and that works well enough. There used to be more of these functions, but they tend to get lost in rewrites and once #72 lands it should be even easier to write your own.

In short; I'd happily accept this as a PR (please call it cmapIf_), but it might get lost once #72 hits :)

jeyj0 commented 4 years ago

Thanks a lot! My use of apecs has sadly had to be reduced due to less time for the side-project I'm using it in. If I start working on it again and have another place where I could use cmapIf_ I'll send a PR for sure!