Issues/234 player item interaction

[jessesnyder]

Description

Features added here

• Items not defined as crossable cannot be crossed over by players
• Details (the name, for now) of the item on the same square as the current player are displayed to the player
• Available transitions on the current square are displayed (crudely, for now)

Non-public changes

• Use a different delimiter between actor and target transition elements, so allow using friendly strings in item/transition config
• Don't persist position on Items on the clients side. Instead use the GridItems wrapper as the single point of truth on grid position for items (avoids possibility of Item.position and position registered in position map conflicting)
• Remove a bunch of dead variables from JS code

Demo

https://app.screencast.com/wrJQmlLXDFutN

[alecpm]

Love this! The separation of the grid items into its own class that manages the position mapping is a nice improvement. I think there are a couple of small inconsistencies in how you're using gridItems.remove(), but otherwise this looks great.

[alecpm]

I think I'd rather see a ➡️ or a → than a = here.

[jessesnyder]

Natalia's mockup used the =, so I went with that, but I also prefer the arrow. I'm going to go with that. I think there's likely to be a lot more change in this presentation anyway.

[alecpm]

player.getTransition() should return null not undefined. I think it would be more semantically appropriate if gridItems.atPosition(...) returned null as well.

[alecpm]

It seems like we're not setting position on Item anymore, won't that be a problem here?

[jessesnyder]

Fixed in related PR

[alecpm]

gridItems.remove() seems to take an item, not a position, though perhaps this would be better.

[jessesnyder]

Looks like I passed an Item in one case and a position in another. I agree position is better.

[alecpm]

I think this should explicitly return a null (e.g. || null) if nothing is found, if only because undefined seems like kind of a weird value for this to return.