a problem with actor representation #34

Closed
Mikolaj opened this Issue Mar 5, 2011 · 6 comments

Comments

1 participant
Owner

Mikolaj commented Mar 5, 2011

See commit 6572e30:

TODO: This function and the next show the shortcomings of the actor
mechanism, since here we need to query and modify not only actors.
but also monsters/heroes that are passive targets of the actions.
To fix this, we should either consider everybody an actor all the time
(add permanent monster number and add (AHero n) actors)
or forgo actors alltogether and just pass around and modify
values of the Monster type and, when done, overwrite with them
their old copy stored on a level (as we currently do with target heroes,
without the need for their actor representation).

Owner

Mikolaj commented Mar 5, 2011

Actually, even in functions that do not involve passive target, but just a single active actor, we sometimes don't adhere to the actor style. I mean, we don't build a function and then apply it to an actor with updateActor, but instead, we copy the actor's content with "getActor state actor", compute various things based on the old values and only then update the actor. It's not as bad as could be, since we never just overwrite a whole actor (by updating with a "const" function, as happens a lot with, e.g., level map), but still, if the code was concurrent, many things could change between the query and the update, so the values with which we update may be out of sync with the rest of the actor. We should have some policy of not using accessors and updates of the same state component in the same function. Best if the types could ensure that.

Of course, if we pass around whole monster bodies instead of actors, it'll be even worse and the risk that we use different copies of an actor for different parts of the computation increases, even in the absence of concurrency (modifying and passing around state is enough). Yet another possible solution is pass around bodies, but each time remove them from the state and reinsert modified. For heroes it's cheap and simple, since they have unique numbers and are stored in intmaps. Already, when the player starts controlling a hero, the hero is removed from the pool of level's heroes. For monsters, where we have a tiny bit of that approach too, it requires some changes to be comfortable (I'd advocate the same approach as with heroes, it has the same asymptotic complexity as the time-sorted list of monsters).

The actor approach has the advantage over removing components of the state before modification, that it indicates with types that state components are not copied. However, currently the indication can be ignored, while with the removal method, it results in a runtime error, as it should. Perhaps we could do both, inside functions. E.g., getActor could also remove the actor component of the state, or make it inaccessible, except to updateActor. Reminds me of linear types.

Owner

Mikolaj commented Mar 5, 2011

If actor APlayer is changed to (AHero n), I'd put Actor instead of Int in the target type, so that heroes can target also heroes and monsters also monsters. I'd also put Actor at the splayer position of state, so that the player can control a monster (via a scroll, say, and until the monster gets tired). To lead a monster from a level to a level, monsters have to be numbered uniquely throughout the dungeon (how?) instead of throughout only the level (easy, just take maxKeys + 1, if monsters are on an intmap).

Owner

Mikolaj commented Mar 8, 2011

use AHero to avoid duplicating state in combat; closed by d8f271a

TODO: many minor points described in #34.

Edit: Ha! Reopening issues works now on github!

Owner

Mikolaj commented Mar 10, 2011

the splayer field of state is now an actor; closed by 49e12bd (again)

TODO: simplification and clean-up

Edit: reopened again, but only a little remains to do before it can be closed for good.

Owner

Mikolaj commented Mar 13, 2011

target is now an actor, not a number; closed by 8937a50 (yet again)

Edit: reopening (for the last time).

Owner

Mikolaj commented Mar 13, 2011

monsters stored on intmap, not time-sorted list; closed by 6097568 (for the last time)

Asymptotic complexity is the same: finding first monster to move is linear,
but previously inserting the monster back into the time-sorted list was linear.

Mikolaj added a commit that referenced this issue Aug 16, 2011

use AHero to avoid duplicating state in combat; close #34
TODO: many minor points described in #34.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment