xat-server 0.1.0 candidate#48
Open
huumanoid wants to merge 50 commits into
Open
Conversation
…ers in same pool
… goes to @chat.onPool as String
Conflicts: src/services/client.coffee src/workers/chat.coffee
houndci-bot
reviewed
Oct 12, 2016
| database.exec('INSERT INTO messages (id, uid, message, name, registered, avatar, time, pool) values (?, ?, ?, ?, ?, ?, ?, ?)', [ @user.chat, @user.id, message, @user.nickname, @user.username || 'unregistered', @user.avatar, math.time(), @chat.onPool ]).then((data) -> | ||
| packet = messageBuilder.buildNewMain( | ||
| message: message | ||
| client: @ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduction
This pull request introduces a bunch of features which may lead xat-server to becase 0.1.0, according to issue list. Let's discuss all this stuff.
Changes
Thanks
Thanks @iiegor for his feedback, i was glad to see you are interested in my collaboration.
#1. Pools
Behavior
When user joins pool, it appears online only for users with same pool.
When user changes pool from 0 to 1, users in 0 receive
<l>, users in 1 receive<u>.User sends messages only for users with same pool.
User receives messages only from users with same pool.
Implementation
new method
changePooladded tochat. This method emit<l>, changes client'sonPooland performsjoinRoom. Later, while implementing banpool, we should usechangePoolto put user to banpool. While implementing rankpool, we should add checks tochangePool.Pools tests are into
test/pools.coffee.Issues
Currently, database scheme for
chatstable doesn't containsonPoolfield, but it seems thatonPoollies intopoolfield. As for now, it's ok.TODO
Dynamic pools
Original xat server creates additional pools when amount of online users exceed 50, and connects new users to random additional pools.
#2. Ranks
Ranks handling added:
<j2 r="pass"... />is handled.Behavior
When moderator changes user's rank:
<c>, if user is on chat.<m t="/m" p="STR_RANK" .. />.<i>and<gp>, if user is on chat (chat packet builder for<i>and<gp>introduced).<u>with actual user's rank, if user is on chat.Restrictions:
Implementation
Ranks stored in
rankstable of db, ranks are fetched inchat.joinChat, making handled bychat.makeUsermethod. Current code assume that only rank data stored inranks.f. It means, if we want to add more significant bits toranks.f, we should change at least this line.Rankdata structure added tosrc/structures/rank.coffee. Implements rank conversions from/to string/number, rank comparison.Discuss
ranks.fright? Some insertions intoranksin sql script contains strange values off(eg 5).client.user.fandclient.chat.rankare dependent fields, but they are parts of plain objects. Probably, we should implement class that would contain some logic based onfcomplex nature.Commander? Is it a tool for chat administrator or server administrator? In first case, we only need to add rank checking to solve Triage important tasks #7.TODO
Fix messaging between connected users #3. Compilation
Compilation task added to
package.json.Output folder is
lib.Binary that runs compiled server:
bin/xat-productionTo run compiled server, execute:
Tests uses compiled version of chat. To switch to coffee version instead, change symlink
bin/xat-test, which points to server's binary used to tests.#4. Packet builders
Most of this section content copied from #45
Also, packet builders for chat added.
I've investigated for packet creation methods, which have duplications. I've found, that creation of 'u' packets and 'm' packets for main chat messages are duplicated.
packet-builders/userandpacket-builders/messagehave been created and hardcoded packet composings have been replaced.I think this stuff will evolve as project grows. Some refactoring needed. For example: may be we should impelement separate method for expanding of 'response-to-locate' packet. May be not.
Discuss
We should implement separate methods for online and offline users.
At first sight, it's convenient to create buildUser method, which can recognize, is user online or not, and make fetch user from db in the second case. However, it requires buildUser to be async. Returning promise from buildUser or even using callback may seems fine, but check out this lines. Here, buildUser should be called in cycle. Making an array of promises and passing them to
Promise.allor making recursive calls of callbacks is not the case. That's why sync version of buildUser() have been created.I think, we can implement unified buildUser method even if there is C# async/await syntax in coffee-script (see ES7 async/await for coffeescript)
#5. Other
@houndci-bot "@ must not be used stand alone"
I propose to ignore this warn. According to coffee-lint docs and coffee-script discussion about standalone @ and this, it's not recommended to use neither standalone
@northis. But we need to.