Skip to content

Add 'sendchat' command for sending private chats#1

Open
lieuwex wants to merge 1 commit into
masterfrom
private-chats
Open

Add 'sendchat' command for sending private chats#1
lieuwex wants to merge 1 commit into
masterfrom
private-chats

Conversation

@lieuwex

@lieuwex lieuwex commented Jul 18, 2016

Copy link
Copy Markdown
Member

@tomsmeding How's this? The client doesn't necessarily have to implement this right now. But it's maybe a nice feature to have.

@tomsmeding

Copy link
Copy Markdown
Member

Great idea!
But then players should never have a name that can be/become a roomid; else the client cannot distonguish between them.

@lieuwex

lieuwex commented Jul 20, 2016

Copy link
Copy Markdown
Member Author

Good point, should I add a room id collision check in nicknameExists?

@lieuwex

lieuwex commented Jul 20, 2016

Copy link
Copy Markdown
Member Author

I also added an ID field to players (491ac3a).
We can also move from nicknames to IDs only. But the client has to be updated for that.

@tomsmeding

Copy link
Copy Markdown
Member

Good point, should I add a room id collision check in nicknameExists?

You also need to prevent collisions with any future room id's. I think it's better to either make private messages a different message typ, or add a boolean argument to it indicating whether the message is private.

We can also move from nicknames to IDs only.

Meh

@lieuwex

lieuwex commented Jul 20, 2016

Copy link
Copy Markdown
Member Author

You also need to prevent collisions with any future room id's. I think it's better to either make private messages a different message typ, or add a boolean argument to it indicating whether the message is private.

Good point.

We can also move from nicknames to IDs only.

Meh

I mean behind the scenes, not user facing. Collisions have to be fixed by appending an (n) after the name, like player(2).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants