search

mikelax / forgingadventures

0 stars 0 forks source link

closes #181 - Create Lounge Message when GM updates player status #183

Closed nazar closed 6 years ago

mikelax commented 6 years ago

@nazar There is currently a logic bug with the "Accepted" player implementation. The issue is that the GM is the one performing the operation to "accept" the player, so the new message that gets created in game_lounges has a userid of the GM, not the player. What this means is that when the Message is rendered the "meta" message uses the GM name in the text instead of the player.

I think the (simple) fix is to have the userId be that of the player instead of the GM. It does feel like the GM though should own the message, but this seems like it would be harder to implement as then we would need more info in the meta column, to store the player user id.

EDIT - I know that the API server is what adds the userId in the mutation before creating the record.... hmm still thinking of the best solution that will be easy to maintain and make sense.

nazar commented 6 years ago

I think the (simple) fix is to have the userId be that of the player instead of the GM. It does feel like the GM though should own the message, but this seems like it would be harder to implement as then we would need more info in the meta column, to store the player user id.

Yeah that's an interesting but slightly tricky one to get right without adding too much into the meta column.

There could be a 1/2 solution: the admin still owns the message (which kind of makes sense) but the accepted message is a special case where the target user's name is recorded in game_lounge.message. Funnily enough, the current code does this but the accepted message with the user's name isn't surfaced when rendering the game lounge message.

I'll push a tiny tweak that does that - maybe that should be enough for now?

mikelax commented 6 years ago

@nazar I saw the message field contains a full message as well as the meta.

I thought about just displaying the message field for the accepted meta type in addition to join.

Do you think maybe we should make the meta column a Json? Then we could store attributes we would need like this in the same col.

mikelax commented 6 years ago

@nazar I just got to the office and took a full look at the last change. I see what you are doing, basically you are treating the message field as an "attribute" if the meta type is accepted.

I think that is fine for now, as I do agree that the GM should own this message.