A dice roller in node.js

Question

I knocked out a dice roller in 2 days. I think this is the first coding project I've ever actually completed to a degree where I'm not embarrassed to post it.

This is my first time working with objects, sets, and recursion. It took a lot of trial and error and it's not the most stable (bad syntax can crash it), but I'm pretty proud.

When the bot hears roll in chat, it parses the syntax of the the message, rolls the dice, applies some special rules (pertinent to the Exalted RPG system), and responds back to chat with formatted text.

I'd love to hear how experts could improve my code, with the proviso that I'm new to coding and this was my first time using objects, sets, and recursion so my vocabulary and comprehension are still improving.

var Discord = require("discord.js");
var mybot = new Discord.Client();
credentials = require("./token.js");
mybot.loginWithToken(credentials.token);
// Look for messages starting with roll
// To-do: change to .roll
mybot.on("message", function(message) {
 if (message.content.startsWith("roll")) {
 mybot.reply(message, parseMessage(message));
 }
});
//
// SYNTAX GUIDE:
// Handle: target number, double successes (single and #+),
// rerolls (single and cascading), autosuccess
//
// .roll/tn6/
// tn: single target number, values >= to this will count as a success. Default: 7
// db: double x's. 7 double's 7 only, 7+ is 7 and up. Default: 10
// re: reroll #
// as: adds a flat number of successes
//
function Roll(numDice) {
 var roll = function(numDice) {
 var rolls = [];
 for (var i = 0; i < numDice; i++) {
 rolls.push(rolld10());
 }
 return rolls;
 };
 this.doubleSet = new Set([10]);
 this.rerollSet = new Set();
 this.rolls = roll(numDice);
 this.target = 7;
 this.autosuccesses = 0;
}
// This is called first within Roll Object and sometimes during rerolls
// Should it live here?
function rolld10() {
 return Math.floor(Math.random() * 10 + 1);
}
function parseMessage(message) {
 message = message.toString();
 var parsed = message.split(" ");
 // log parsed message for debugging:
 // console.log("parsed message: " + parsed);
 // If there's a number of dice at the end of the roll message...
 if (parsed[1].match(/^\d+/g)) {
 // get digits at beginning of string
 // I'm fairly sure this could be improved upon...
 var numDice = parsed[1].match(/^\d+/g);
 numDice = numDice[0];
 // Create a new Roll Object
 var theRoll = new Roll(numDice);
 // Parse roll options and pass to theRoll
 // To-do: test if empty array causes error
 var options = parsed[0].split("/");
 console.log("options: " + options);
 for (var i in options) {
 // set target number
 if (options[i].startsWith("tn")) {
 var target = options[i].match(/\d+/g);
 theRoll.target = parseInt(target, 10);
 }
 // set doubles
 // To-do: add code to not double 10's
 // To-do: add code for double 7+ (doub;les 7,8,9,and 10)
 if (options[i].startsWith("db")) {
 var double = options[i].match(/10|\d/g);
 double.forEach(function(item) {
 theRoll.doubleSet.add(parseInt(item, 10));
 })
 }
 // set rerolls
 if (options[i].startsWith("re")) {
 var reroll = options[i].match(/10|\d/g);
 reroll.forEach(function(item) {
 theRoll.rerollSet.add(parseInt(item, 10));
 })
 }
 // set autosuccesses
 if (options[i].startsWith("as")) {
 var autosuccesses = options[i].match(/\d+/g);
 theRoll.autosuccesses = parseInt(autosuccesses, 10);
 }
 }
 checkForRerolls(theRoll.rolls, theRoll.rerollSet);
 console.log(theRoll);
 // Pass theRoll through countSuccessesAndDisplayResults
 return countSuccessesAndDisplayResults(theRoll);
 } else {
 // Bad syntax handling
 // To-do: add better support here
 return "I can't find any numbers after roll. Syntax: roll/tn#/db#s/re#s/as# 8d10";
 }
}
// Check whether any of our roll values are contained in our rerollSet
// If so, initiate a cascade
function checkForRerolls(rolls, rerollSet) {
 for (var i in rolls) {
 if (rerollSet.has(rolls[i])) {
 cascade(rolls, rerollSet);
 }
 }
}
// Make a new roll, add it to our roll array. If this new value is
// also a reroll, run cascade again
function cascade(rolls, rerollSet) {
 roll = rolld10();
 rolls.push(roll);
 if (rerollSet.has(roll)) {
 cascade(rolls, rerollSet);
 }
}
function countSuccessesAndDisplayResults(theRoll) {
 // Sort dice rolls
 theRoll.rolls = theRoll.rolls.sort(function(a, b){return a-b});
 // Count successes and format results
 var successes = theRoll.autosuccesses;
 for (var i in theRoll.rolls) {
 if (theRoll.rolls[i] >= theRoll.target && theRoll.doubleSet.has(theRoll.rolls[i]) && theRoll.rerollSet.has(theRoll.rolls[i])) {
 successes+=2;
 theRoll.rolls[i] = "~~__**"+theRoll.rolls[i]+"**__~~";
 } else if (theRoll.rolls[i] >= theRoll.target && theRoll.doubleSet.has(theRoll.rolls[i])) {
 successes+=2;
 theRoll.rolls[i] = "__**"+theRoll.rolls[i]+"**__";
 } else if (theRoll.rolls[i] >= theRoll.target) {
 successes+=1;
 theRoll.rolls[i] = "**"+theRoll.rolls[i]+"**";
 } else if (theRoll.rerollSet.has(theRoll.rolls[i])) {
 theRoll.rolls[i] = "~~"+theRoll.rolls[i]+"~~";
 }
 }
 console.log(theRoll.rolls);
 return "you rolled " + theRoll.rolls + " for a total of **" + successes + "** successes";
}

• konijn: What gaming systems uses such complex dice rolls?
• mcdustin: Exalted. Most of the time you just count 1 success for each dice over 7 and double successes on 10s, but there are various powers which add rerolls, more doubles, or change the target number.

Answer 1

You might want to experiment instead of Roll as the main object entity, try Dice. Generally speaking, in object oriented programming, we want to have the class that preferably maps to a specific object or in another word, prefer nouns for our classes over verbs. I do realize Roll can also be a noun but one can argue that it's just easily an action being taken place.

The reason why I suggest this is that this will likely help organize your functions and think about your objects more concretely. For example, if I'm thinking in term of a "Dice", what can I configure a dice with? Such as.. how many sides of a dice do I want? maybe, what are the faces of this dice? Another question would be, what type of actions can I do with a single dice? In your case, primarily we can "roll" a dice. Methods in a class is more intuitive with verbs or actions that the object can take. Also, our dice object can have interactions with other objects in the future, maybe we eventually want to build our chatbot to allow playing crabs or monopoly. We should theoretically be able to use the same Dice object as long as we keep its responsibilities very specific to what a dice can do.

Here is an example of how you may want to rearrange and hopefully, we will gain clarity and reusability:

/* I encourage using ES6 classes, by the way you should prefer function
 expressions over function declarations as there are hoisting properties
 that may catch you by surprise and honestly it's rare to see.
*/
class Dice {
 constructor(numOfFaces) {
 this.numOfFaces = numOfFaces;
 this.faceUpValue = null;
 }
 roll() {
 this.faceUpValue = Math.floor(Math.random() * this.numOfFaces + 1);
 return this.faceUpValue; // this is arbitrary, I prefer this method to not return face up value;
 }
 value() {
 return this.faceUpValue;
 }
}
// I'm going to introduce this Roll class just to show how we may organize this code.
class Roll {
 constructor(numOfDice) {
 this.numOfDice = numOfDice;
 this.dices = [];
 const numOfFaces = 10;
 for(let i = 0; i < numOfDice; i++) {
 // notice that we can create dices with different faces if we wanted
 const newDice = new Dice(numOfFaces);
 this.dices.push(newDice);
 }
 }
 doIt() {
 this.dices.forEach((dice) => {
 dice.roll();
 }
 }
 getDiceValues() {
 this.dices.map((dice) => {
 return dice.value();
 }
 }
}
// sample code, putting it all together
const numOfDices = 6;
const roller = new Roll(numOfDices);
roller.doIt();
console.log(roller.getDiceValues());
// reroll
roller.doIt();
console.log(roller.getDiceValues());

This illustration mostly just show how we can improve clarity and help us figure out where our code might go. In your original snippet, there is a lot functions just living on the global scope, we want to generally try to avoid those as much as possible. Generic functions generally are bad organization as it doesn't give any context to the function and the pattern definitely won't scale as we add more functionalities. Sometimes the trick is just figure out what new entity to create and code will often naturally find its place to where it belongs.