jonascarpay / apecs

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

Why is there no getAll / members? #87

Closed bingis-khan closed 1 year ago

bingis-khan commented 3 years ago

I can't seem to find any info about why getAll was removed except the CHANGELOG.md, which states "Removed: getAll and count, which were made redundant by cfold.", which is simply untrue. "members", which was mentioned here (https://github.com/jonascarpay/apecs/blob/master/examples/Shmup.md) and seems to serve the same function, but does not actually exist. How, in "modern" apecs, would I go about accessing the Player's position (like here: https://aas.sh/blog/making-a-game-with-haskell-and-apecs/) monadically?

dpwiz commented 3 years ago

I tend to regularly reimplement/copypaste this couple:

collect
  :: forall components w m a
  .  ( Apecs.Get w m components
     , Apecs.Members w m components
     )
  => (components -> a)
  -> Apecs.SystemT w m [a]
collect f = Apecs.cfold (\acc comps -> f comps : acc) []

collectMaybe
  :: forall components w m a
  .  ( Apecs.Get w m components
     , Apecs.Members w m components
     )
  => (components -> Maybe a)
  -> Apecs.SystemT w m [a]
collectMaybe f = Apecs.cfold (\acc -> maybe acc (: acc) . f) []
dpwiz commented 3 years ago

And that tutorial needs to be updated to use (a, b) <- get global or cmapM_ instead.

jonascarpay commented 3 years ago

"members", which was mentioned here (https://github.com/jonascarpay/apecs/blob/master/examples/Shmup.md) and seems to serve the same function, but does not actually exist

Oh, that's not good. I think we should add members back in, but in the haddocks explain that it exists only for completeness' sake as a wrapper for the explMembers primitive function, and you usually should create a bespoke fold using cfold. Let's leave this issue open until that's addressed.

Also, if you say you regularly copy/paste it, maybe collect is worth adding as well?

dpwiz commented 3 years ago

Having all three is kinda redundant.

Perhaps there should be a single collectMaybe, renamed to just collect.

dpwiz commented 3 years ago

Also, I feel members is often an antipattern "get everything indiscriminately, sort it out later".

The shmup tutorial says you can implement cmap via members, but don't use members directly.

jonascarpay commented 3 years ago

Perhaps there should be a single collectMaybe, renamed to just collect.

Let's go with that

Also, I feel members is often an antipattern "get everything indiscriminately, sort it out later". The shmup tutorial says you can implement cmap via members, but don't use members directly.

Either way, it could do with some rewording

jonascarpay commented 3 years ago

I looked at the shmup example, and there's more things that are a little outdated. My priority right now is to get #86/#88 done, but after that I'll do a rewrite there.

Ashe commented 2 years ago

I'm flattered that people are using one of my posts as a beginner's tutorial and I do apologise if there's some confusion because of getAll and that it's an old post.

I've been away from the haskell gamedev space trying to get a job in-industry (I wrote that while still a student), but I'm re-entering the space now and start something when I have an idea about some of the new tools in the ecosystem.

If someone has managed to fix that snippet in that post, I'll gladly edit it so that things aren't confusing for people!

ParetoOptimalDev commented 1 year ago

Even after reading the above and playying with the collect function above, it's not obvious to me how this function in the old blog post would be implemented:

fireWeapon :: V2 Double -> Double -> System' ()
fireWeapon (V2 i j) -> speed = do
  [(Player, p :: Position, Weapon ammo, e :: Entity)] <- getAll
  when (ammo > 0) $ void $ do
    let ammoLeft = max 0 $ ammo - 1
    set e (Weapon ammoLeft)
    newEntity (Velocity (i * speed) (j * speed), p)

I tried replacing getAll with collect but collect takes (components -> a)... hmm so maybe the something closer to collect (\(Player, p :: Position, Weapon ...)? That's obviously not valid syntax, but still trying to puzzle this one out.

Ashe commented 1 year ago

The code in this example is outdated. getAll isn't something you should be leaning on like I did in that post and I'd even go as far to say that I'm sorry for leading you in this direction.

You should just be using cmap, cmapM and cmapM_ (or cfold). The code above only worked because we expected 1, and only 1, entity to be in the list retrieved by getAll. This isn't really safe and in cases where there's lots of entities it might also be bad for performance creating a list with a lot of objects just to traverse them etc.

I'm not on my dev machine so there may be errors, but try this:

fireWeapon :: V2 Double -> Double -> System' ()
fireWeapon (V2 i j) speed = do
  cmapM \(Player, p :: Position, Weapon ammo) -> 
    if (ammo > 0)  then do
      let ammoLeft = max 0 $ ammo - 1
      newEntity (Velocity (i * speed) (j * speed), p)
      pure (Weapon ammoLeft)
  else pure ()

Unsure if cmapM will work with returning pure () or if you'd have to do pure (Weapon ammo), you'll have to play around.

The most important thing here is rather than getting a list of entities and pattern matching on the the list, we instead just map a function onto every entity with those components. The output of cmapM (via pure) is the component(s) we want to write to the current entity, in this case we will write the weapon component to have 1 less ammo. I'm thinking that if you don't want to make changes, maybe pure () will suffice but unsure.

Since only one entity has the Player component (I expect), this function will only run on your player entity.

I hope this helps!

ParetoOptimalDev commented 1 year ago

The code in this example is outdated. getAll isn't something you should be leaning on like I did in that post and I'd even go as far to say that I'm sorry for leading you in this direction.

No no, thanks for your content and especially your updated post. I had a feeling I should use cmap or cmapM but couldn't puzzle it out, so thanks for the quick response.

I'll try this after work and let you know!

jonascarpay commented 1 year ago

Closed in #99