12
\$\begingroup\$

This is part 2 of my re-writing of a Angular client to vanilla JS. Part 1 is here:

This animated GIF illustrates the basic functionality of the lobby. I realize it's kind of ugly, I'll make it look nicer later, I wanted to be sure it works before I spend time on styling.

In particular, lobbyController.js is really large and I want to make sure it's organized decently.

html-client-chat-lobby

This page has a good amount of sending and listening for WebSocket messages from the game server, and it was more complicated to put together. I'm looking for a review of all aspects, but in particular, anti-patterns that would be better written in other ways. The whole repository can be found here on GitHub.

sections/lobby/lobby.html

<div id="lobby" class="table lobby">
    <!-- ROW 1 - Headers -->
    <div id="lobby_headers" class="tableHeading lobbyHeader">
        <div id="lobby_title" class="tableCell lobbyTitle">
            Lobby
        </div>
        <div id="lobby_deck_builder" class="tableCell lobbyDeckBuilder">
            <input id="lobby_deck_builder_btn" type="button" value="Deck Builder" class="btn btn-navbar csh-button" />
        </div>
    </div>
    <!-- ROW 2 - Only show when getting invite request -->
    <div id="lobby_invite_request" style="display: none;" class="tableHeading lobbyInviteRequest">
        <!-- td colspan 2 -->
        <div id="lobby_invite_request_colspan" class="tableCell">
            <div id="lobby_invite">
                <!-- TODO this should be filled in dynamically -->
                Game invite from NAME to play GAME_TYPE!
                <input id="lobby_invite_accept" type="button" value="Accept" class="btn btn-success" />
                <input id="lobby_invite_decline" type="button" value="Decline" class="btn btn-warning" />
                <audio id="invite_ping">
                    <source src="../../sounds/ping_sound.mp3" />
                </audio>
            </div>
        </div>
    </div>
    <!-- ROW 3 - Subheaders for Messages and Users -->
    <div id="lobby_list_headers" class="tableRow lobbyListHeaders">
        <div id="lobby_message_list_header" class="tableCell lobbyMessageListHeader">
            Messages
        </div>
        <div id="lobby_user_list_header" class="tableCell lobbyUsersListHeaders">
            Users online
        </div>
    </div>    
    <!-- ROW 4 -->
    <div id="lobby_lists" class="tableRow lobbyLists">
        <div id="lobby_message_list" class="tableCell lobbyMessageList">
            <form id="lobby_chat_messages" class="lobbyChatMessages">
                <!-- Append <li> for chat messages to this element -->
            </form>
        </div>
        <div id="lobby_user_list" class="tableCell lobbyUsersList">
            <ul id="lobby_users" class="lobbyUsers">
                <!-- Append <li>/<input> for users, logic needs to ensure a user can't invite themselves to a game -->
            </ul>
        </div>
    </div>
    <!-- ROW 5 -->
    <div id="lobby_input" class="tableRow">
        <div id="lobby_message" class="tableCell lobbyMessage">
            <!-- User types chat text here -->
            <textarea id="lobby_chat_text_area" class="lobbyTextArea" rows="1" cols="75" wrap="hard" placeholder="Type message, press Enter to send."></textarea>
        </div>
        <div id="lobby_inviter" class="tableCell lobbyInviter">
            <input id="lobby_invite_button" type="button" value="Invite to game" class="btn btn-sm btn-navbar csh-button inviteButton" />
        </div>
    </div>
    <!-- ROW 6 -->
    <div id="lobby_mods" class="tableRow lobbyMods">
        <!-- colspan 2 -->
        <!-- Available mods here -->
        <form id="lobby_mod_selection" class="tableCell lobbyModSelection">
            <!-- Mod selector; https://github.com/Cardshifter/HTML-Client/blob/be991f41c2630c1f46f40d2f8f232bbfad71b2a8/src/lobby/lobby.html#L54 -->
        </form>
    </div>
</div>

sections/lobby/lobbyController.js

/* global CardshifterServerAPI, dynamicHtmlController */

"use strict";

let addChatMessage;

const lobbyController = function() {
    const currentUser = localStorage.getItem("username");
    const onlineUsers = [];
    const invite = {
        id: null,
        username: null,
        mod: null
    };

    const userDisplay = document.getElementById("lobby_users");
    const chatInput = document.getElementById("lobby_chat_text_area");
    const chatSendButton = document.getElementById("lobby_chat_message_send");
    const chatMessageList = document.getElementById("lobby_chat_messages");

    /**
     * Adds a user to the onlineUsers list.
     * @param {Object} user - The user object
     * @returns {undefined}
     */
    const addToGlobalUserList = function(user) {
        if (!userExists(user)) {
            onlineUsers.push(user);
            onlineUsers.sort();
        }
        renderUserList();
    };

    /**
     * Removes a user from the onlineUsers list.
     * @param {Object} user - The user object
     * @returns {undefined}
     */
    const removeFromGlobalUserList = function(user) {
        if (userExists(user)) {
            for (let i = 0; i < onlineUsers.length; i++) {
                if (onlineUsers[i].name === user.name) {
                    onlineUsers.splice(i, 1);
                }
            }
            onlineUsers.sort();
            renderUserList();
        }
    };

    /**
     * Checks whether the user exists in onlineUsers.
     * @param {Object} user
     * @returns {Boolean} - Whether the user exists
     */
    const userExists = function(user) {
        const username = user.name;
        for (let i = 0; i < onlineUsers.length; i++) {
            if (onlineUsers[i].name === username) {
                return true;
            }
        }
        return false;
    };

    /**
     * Renders the user list on the page based on the content of onlineUsers.
     * @returns {undefined}
     */
    const renderUserList = function() {
        if (userDisplay) {
            userDisplay.innerHTML = "";
        }
        for (let i = 0; i < onlineUsers.length; i++) {
            const usernameContainer = document.createElement("div");
            usernameContainer.className = "lobbyUser";
            const username = onlineUsers[i].name;
            const userNum = `user${i}`;
            const usernameSelect = document.createElement("input");
            usernameSelect.type = "radio";
            usernameSelect.id = userNum;
            usernameSelect.name = "select_username";
            usernameSelect.value = username;
            if (username === currentUser) {
                usernameSelect.disabled = true;
            }
            usernameSelect.onclick = function() {
                localStorage.setItem("selectedUsername", username);
            };
            const usernameLabel = document.createElement("label");
            usernameLabel.for = userNum;
            usernameLabel.innerHTML = username;
            usernameContainer.appendChild(usernameSelect);
            usernameContainer.appendChild(usernameLabel);
            if (userDisplay) {
                userDisplay.appendChild(usernameContainer);
            }            
        }
    };

    /**
     * Displays a game invite near the top of the lobby.
     * @returns {undefined}
     */
    const renderInvite = function() {
        const inviteRequestContainer = document.getElementById("lobby_invite_request");
        inviteRequestContainer.style.display = "block";
        const lobbyInvite = document.getElementById("lobby_invite");
        lobbyInvite.innerHTML = `Game invite<br/>From: ${invite.username}<br/>Mod: ${invite.mod}!<br/>`;
        const acceptBtn = document.createElement("input");
        acceptBtn.type = "button";
        acceptBtn.id = "lobby_invite_accept";
        acceptBtn.value ="Accept";
        acceptBtn.className = "btn btn-success";
        acceptBtn.style.marginRight = "5px";
        acceptBtn.onclick = function() {
            const acceptMsg = new CardshifterServerAPI.messageTypes.InviteResponse(invite.id, true);
            logDebugMessage(`Sent invite accept message: ${JSON.stringify(acceptMsg)}`);
            CardshifterServerAPI.sendMessage(acceptMsg);
            inviteRequestContainer.style.display = "none";
        };
        const declineBtn = document.createElement("input");
        declineBtn.type = "button";
        declineBtn.id = "lobby_invite_decline";
        declineBtn.value ="Decline";
        declineBtn.className = "btn btn-warning";
        declineBtn.style.marginLeft = "5px";
        declineBtn.onclick = function() {
            const declineMsg = new CardshifterServerAPI.messageTypes.InviteResponse(invite.id, false);
            logDebugMessage(`Sent invite decline message: ${JSON.stringify(declineMsg)}`);
            CardshifterServerAPI.sendMessage(declineMsg);
            inviteRequestContainer.style.display = "none";
        };
        // TODO find out why this doesn't load in Sources in the browser.
        //const pingSound = new Audio("../../sounds/ping_sound.mp3");
        //pingSound.play();
        lobbyInvite.appendChild(acceptBtn);
        lobbyInvite.appendChild(declineBtn);
    };

    /**
     * Renders the available mods list.
     * @returns {undefined}
     */
    const renderAvailableMods = function() {
        const mods = document.getElementById("lobby_mod_selection");
        for (let i = 0; i < global.availableMods.length; i++) {
            const modContainer = document.createElement("span");
            modContainer.className = "lobbyMod";
            const modName = global.availableMods[i];
            const modNum = `mod${i}`;
            const modSelect = document.createElement("input");
            modSelect.type = "radio";
            modSelect.id = modNum;
            modSelect.name = "select_mod";
            modSelect.value = modName;
            modSelect.onclick = function() {
                localStorage.setItem("selectedMod", modName);
            };
            const modLabel = document.createElement("label");
            modLabel.for = modNum;
            modLabel.innerHTML = modName;
            modContainer.appendChild(modSelect);
            modContainer.appendChild(modLabel);
            if (mods) {
                mods.appendChild(modContainer);
            }
        }
    };

    /**
     * Handles interactions between the browser client and the game server.
     * @returns {undefined}
     */
    const handleWebSocketConnection = function() {
        const CHAT_FEED_LIMIT = 10;
        const ENTER_KEY = 13;
        const MESSAGE_DELAY = 3000;

        let getUsers = new CardshifterServerAPI.messageTypes.ServerQueryMessage("USERS", "");
        CardshifterServerAPI.sendMessage(getUsers);

        CardshifterServerAPI.setMessageListener(function(wsMsg) {
            updateUserList(wsMsg);
            addChatMessage(wsMsg);
            receiveInvite(wsMsg);
            startGame(wsMsg);
        });

        /**
         * Updates the onlineUsers list based on `userstatus` messages from game server.
         * @param {Object} wsMsg - WebSocket message
         * @returns {undefined}
         * @example message - {command: "userstatus", userId: 2, status: "ONLINE", name: "AI Loser"}
         */
        const updateUserList = function(wsMsg) {
            if (wsMsg.command === "userstatus") {
                logDebugMessage(`SERVER userstatus message: ${JSON.stringify(wsMsg)}`);
                const user = {
                    id: wsMsg.userId,
                    name: wsMsg.name
                };
                if (wsMsg.status === "ONLINE") {
                    addToGlobalUserList(user);
                }
                else if (wsMsg.status === "OFFLINE") {
                    removeFromGlobalUserList(user);
                    /**
                     * This condition is for circumventing an apparent server-side bug, see:
                     * https://github.com/Cardshifter/Cardshifter/issues/443
                     */
                    if (wsMsg.name) {
                        addChatMessage({
                            chatId: 1,
                            message: `${wsMsg.name} is now offline.`,
                            from: "Server Chat",
                            command: "chat"
                        });
                    }
                }
            }
        };

        /**
         * Adds chat message to the lobby on `chat` messages from game server.
         * @param {Object} wsMsg - WebSocket message
         * @returns {undefined}
         * @example {"command":"chat","chatId":1,"message":"Hello","from":"Phrancis"}
         */
        addChatMessage = function(wsMsg) {
            if (wsMsg.command === "chat") {
                logDebugMessage(`SERVER chat message: ${JSON.stringify(wsMsg)}`);
                const now = new Date();
                const timeStamp = formatDate(now, "dd-MMM hh:mm");
                const msgText = `${timeStamp} | ${wsMsg.from}: ${wsMsg.message}`;
                const msgElem = document.createElement("li");
                if (msgElem) {
                    msgElem.innerHTML = msgText;
                    msgElem.className = "lobbyChatMessages lobbyChatMessage";
                }                
                if (chatMessageList) {
                    chatMessageList.appendChild(msgElem);
                }
            }
        };

        /**
         * Fires rendering of invite requests on the page when an invite is received.
         * @param {OObject} wsMsg - WebSocket message
         * @returns {undefined}
         * @example {"command":"inviteRequest","id":1,"name":"HelloWorld","gameType":"Mythos"}
         */
        const receiveInvite = function(wsMsg) {
            if (wsMsg.command === "inviteRequest") {
                logDebugMessage(`SERVER inviteRequest message: ${JSON.stringify(wsMsg)}`);
                invite.id = wsMsg.id;
                invite.username = wsMsg.name;
                invite.mod = wsMsg.gameType;
                renderInvite();
            }
        };

        /**
         * Load up deck builder when invite accepted and game starts
         * @param {type} wsMsg - WebSocket message
         * @returns {undefined}
         * @example {"command":"newgame","gameId":26,"playerIndex":1}
         */
        const startGame = function(wsMsg) {
            if (wsMsg.command === "newgame") {
                logDebugMessage(`SERVER newgame message: ${JSON.stringify(wsMsg)}`);
                dynamicHtmlController.unloadHtmlById("lobby");
                dynamicHtmlController.loadHtmlFromFile("deckBuilder", "sections/deck_builder/deck_builder.html")
                .then(function() {
                    deckBuilderController();
                });
            }
        };
    };

    /**
     * Handles the usage of the user chat textarea and send button. 
     * @returns {undefined}
     */
    const handleUserChatInput = function() {
        const enterKeyCode = 13;
        const newlineRegex = /\r?\n|\r/g;
        const postMessage = function() {
            const msg = chatInput.value.replace(newlineRegex, "");
            if (msg) {
                chatInput.value = null;
                sendChatMessage(msg);     
            }
        };
        if (chatInput) {
            chatInput.addEventListener("keyup", function(evt) {
                const code = evt.keyCode;
                if (code === enterKeyCode) {
                    postMessage();
                }
            });
        }

    };


    /**
     * Sends a chat message to the server.
     * @param {string} message
     * @returns {undefined}
     */
    const sendChatMessage = function(message) {
        const chatMessage = new CardshifterServerAPI.messageTypes.ChatMessage(message);
        logDebugMessage(`sendChatMessage: ${chatMessage}`);
        CardshifterServerAPI.sendMessage(chatMessage);
    };

    const activateInviteButon = function() {
        const lobbyInviteButton = document.getElementById("lobby_invite_button");
        if (lobbyInviteButton) {
            lobbyInviteButton.addEventListener("click", sendInvite);
        }

    };

    /**
     * Sends an invite to play to another user.
     * @returns {undefined}
     * @example {"command":"inviteRequest","id":15,"name":"HelloWorld","gameType":"Mythos"}
     */
    const sendInvite = function() {
        logDebugMessage("sendInvite called");
        const selectedUser = localStorage.getItem("selectedUsername");
        const selectedMod = localStorage.getItem("selectedMod");
        if (selectedUser === "null") {
            const msg = "Client error: You must select a user to be your opponent to invite them to a game.";
            addChatMessage({
                chatId: 1,
                message: msg,
                from: "NOTIFICATION",
                command: "chat"
            });
            logDebugMessage(msg);
        }
        else if (selectedMod === "null") {
            const msg = "Client error: You must select a mod to play with the opponent.";
            addChatMessage({
                chatId: 1,
                message: msg,
                from: "NOTIFICATION",
                command: "chat"
            });
            logDebugMessage(msg);
        }
        else {
            let selectedUsedId = null;
            for (let i = 0; i < onlineUsers.length; i++) {
                if (onlineUsers[i].name === selectedUser) {
                    selectedUsedId = onlineUsers[i].id;
                }
            }
            const inviteMsg = new CardshifterServerAPI.messageTypes.StartGameRequest(selectedUsedId, selectedMod);
            CardshifterServerAPI.sendMessage(inviteMsg);
        }
    };


    /**
     * IIFE to control the lobby.
     * @type undefined
     */
    const runLobbyController = function() {
        logDebugMessage("lobbyController called");
        localStorage.setItem("selectedUsername", null);
        localStorage.setItem("selectedMod", null);
        handleWebSocketConnection();
        handleUserChatInput();
        renderAvailableMods();
        activateInviteButon();
    }();
};

styles/lobby.css

/* WHOLE LOBBY */

.lobby {
    width: 80%;
    padding-left: 20px;
}

/* TABLE HEADERS */

.lobbyHeader {
    font-family: Verdana, Geneva, sans-serif;
    text-align: center;
    color: #DDDDDD;
    background-color: #000000;
}

.lobbyTitle {
    font-size: 1.5em;
    font-weight: bold;
    padding: 5px;
}

.lobbyDeckBuilder {
    width: 20%;
    padding: 5px;
}

/* Game invite accept dialog */
.lobbyInviteRequest {
    font-family: Verdana, Geneva, sans-serif;
    font-size: 1.6em;
    text-align: center;
    background-color: #0033CC;
    color: #EEEEEE;
    border-top-color: #FFFFFF;
    vertical-align: middle;
}

/* SECTION HEADERS */

.lobbyListHeaders {
    font-family: Verdana, Geneva, sans-serif;
    font-size: 1.4em;
    text-align: center;
}

.lobbyMessageListHeader {}

.lobbyUsersListHeaders {}

/* MAIN MESSAGE & USERS SECTIONS */

.lobbyLists {
    vertical-align: text-top;
    height: 400px;
}

.lobbyMessageList {
    font-size: 0.9em !important;
}
/* List of all messages */
.lobbyChatMessages {
    list-style-type: none;
    padding-left: 0;
}

.lobbyChatMessages:nth-child(even) {
    background-color: #FFFFFF;
}
.lobbyChatMessages:nth-child(odd) {
    background-color: #EEEEEE;
}

/* Each individual message line */
.lobbyChatMessage {
}

.lobbyUsersList {
    font-size: 0.9em;
    font-family: Verdana, Geneva, sans-serif;
}
/* List of all users */
.lobbyUsers {
    list-style-type: none;
    padding-left: 0;
}
/* Each individual user line */
.lobbyUser {
    font-weight: normal;
}

/* FOOTER SECTIONS */

.lobbyMessage {
    background-color: #000000;
}
/* TEXT AREA FOR TYPING CHAT MESSAGES*/
.lobbyTextArea {
    outline: none;
    overflow: auto;
    vertical-align: middle;
    margin-left: 5px;
    padding: 3px;
}

.inviteButton {
    margin: 5px;
}

.lobbyInviter {
    background-color: #000000;
    text-align: center;
}

.lobbyMods {}

.lobbyModSelection {
    text-align: center;
    padding-top: 5px;
    padding-bottom: 5px;
}

.lobbyMod {
    padding: 10px;
}
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
6
\$\begingroup\$

Quick skimming

let addChatMessage;

Why is this outside your lobbyController?

const addToGlobalUserList = function(user) {
    if (!userExists(user)) {
        onlineUsers.push(user);
        onlineUsers.sort();
    }
    renderUserList();
};

The function is named addToGlobalUserList yet it pushes to a list called onlineUsers... Why not name this something with online?

The same applies for remove.

        for (let i = 0; i < onlineUsers.length; i++) {
            if (onlineUsers[i].name === user.name) {
                onlineUsers.splice(i, 1);
            }
        }
        onlineUsers.sort();

It should not be necessary to sort the onlineUsers after removing an element. Only additions can destroy sorting of arrays.

/**
 * Checks whether the user exists in onlineUsers.
 * @param {Object} user
 * @returns {Boolean} - Whether the user exists
 */
const userExists = function(user) {

Why not name it userOnline? That does more accurately reflect what it means. 

        if (onlineUsers[i].name === username) {
            return true;
        }

This is a somewhat dangerous assumption. Is it guaranteed that usernames are unique?

    for (let i = 0; i < onlineUsers.length; i++) {
        const usernameContainer = document.createElement("div");
        usernameContainer.className = "lobbyUser";
        const username = onlineUsers[i].name;
        const userNum = `user${i}`;
        const usernameSelect = document.createElement("input");
        // [...]
        if (userDisplay) {
            userDisplay.appendChild(usernameContainer);
        }            
    }

You're building HTML in your JS code. That's bound to be wordy and somewhat difficult to follow. HTML 5 introduced the <template> element for exactly that purpose:

<template id="userlist_entry">
    <div class="lobbyUser">
        <input type="radio" name="select_username" />
        <label></label>
    </div>
</template>

---

let userRow = document.getElementById("userlist_entry").content.cloneNode(true);
let select = userRow.querySelector("input");
// update select
let label = userRow.querySelector("label");
// update label
userDisplay.appendChild(userRow );

This separates HTML and JS code and allows you to somewhat adapt each without affecting the other.
In addition you can reuse templates at multiple places in your code, not that it's necessary here.

The same considerations of course also apply for renderInvite and renderAvailableMods. Do note that in the former you should see much more improvement to the code there as opposed to in renderUserList.

    CardshifterServerAPI.setMessageListener(function(wsMsg) {
        updateUserList(wsMsg);
        addChatMessage(wsMsg);
        receiveInvite(wsMsg);
        startGame(wsMsg);
    });

Just to be safe, I checked the functions you have there... Each of them checks for strict equality of wsMsg.command. Instead of indiscriminately calling all these functions, consider the following:

const messageHandlers = { 
    userstatus : updateUserList, 
    chat : addChatMessage,
    inviteRequest : receiveInvite,
    newgame : startGame
};
CardshifterServerAPI.setMessageListener(function (wsMsg) {
    messageHandlers[wsMsg.command](wsMsg);
}

Of course this assumes that messageHandlers[wsMsg.command] never returns undefined, which is guaranteed if the handlers are exhaustive.

if that's not the case, this will work similarly well:

let handler = messageHandlers[wsMsg.command];
if (handler) { handler(wsMsg); }
Improve this answer
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the answer! We do have a duplicate username check on server side, but I added a comment in client to clarify that. I made an edit to your template handling code, correcting a few things I ran into while implementing it. Cheers! \$\endgroup\$
    – Phrancis
    Apr 4, 2018 at 5:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.