Roll20 uses cookies to improve your experience on our site. Cookies enable you to enjoy certain features, social sharing functionality, and tailor message and display ads to your interests on our site and others. They also help us understand how our site is being used. By continuing to use our site, you consent to our use of cookies. Update your cookie preferences .
×
Create a free account

First piece of functioning code, looking for constructive criticism and ways to improve it!

Okay! So I have been working on this for 2 days and I actually have my first working bit of code! No matter what number you put in to modify the number of dice in your pool, you get the minimum of 0, a value from 1 to 5, or the maximum of 6! This may not be much in the grand scheme of things, but damn did I have to fight for it! I'm open to constructive criticism, I want to get better at this. I'm specifically looking for ways I could save the values in token health bars or GM notes. That or to find a way to make the code more concise.  var dicePool = 0; function addDice(diceMod){     ++dicePool;     changeResolve(--diceMod); } function removeDice(diceMod){     --dicePool;     changeResolve(++diceMod); } function changeResolve(diceMod){     if (diceMod == 0){         if (dicePool <= 0){             dicePool -= dicePool;             sendChat("Resolve","Your Resolve Pool is empty.");         }         else if (dicePool >= 6){             dicePool -= dicePool;             dicePool += 6;             sendChat("Resolve","You feel full of Resolve!");         }         else {             sendChat("Resolve","You have " + dicePool + " Resolve.");         }     }     else if (diceMod > 0) {         addDice(diceMod);     }     else if (diceMod < 0) {         removeDice(diceMod);     } } on("chat:message",function(msg){     if(msg.type == "api" && msg.content.indexOf("!resolve") == 0){         var args = msg.content.split(/\s+/);         var diceMod = args[1]         changeResolve(diceMod);     } });
Hey Wiggy, nice job on your first bit of code! Good job making something that works for what you want it to do. Currently you have your changeResolve function which can call the addDice and removeDice functions in some cases, but they in turn call changeResolve while modifying the size of the diceMod in the recursive call to changeResolve. A wee bit overkill for what you're trying to do when you boil down the problem to its roots. Try thinking about what each function should do and try to encapsulate that one bit of functionality. At the moment your changeResolve function is acting as both your verification if the resolve can be changed, as well as changing the resolve. If there were more going on I might separate out the verification that a change needs to happen from the change happening, but in this case I didn't think that was needed. You could simplify a lot of what's happening here in a function like this var dicePool = 0; //verify that a pool change can/needs to occur function simpleResolveChange(diceMod){          //simply perform the addition without recursive calls     let newPool = dicePool + diceMod;     //store msg so you don't have to write "sendChat()" all over     let resolveMsg = "";          //check if the entire change puts the pool below 0, and if it is set the pool to 0 and notify empty     if (newPool <= 0 ){         dicePool = 0;         resolveMsg = "Your Resolve Pool is Empty";     }     //if the entire change puts the pool above 6, simply set the pool itself to 6 and notify full     else if(newPool >= 6){                 dicePool = 6;         resolveMsg = "You feel full of Resolve!";     }     //otherwise set the pool to the size of the newPool and notify the current size     else{         dicePool = newPool;         resolveMsg = `You have ${dicePool} Resolve!`;     }     //notify     sendChat("Resolve",resolveMsg); } on("chat:message",function(msg){ if(msg.type == "api" && msg.content.indexOf("!resolve") == 0){         var args = msg.content.split(/\s+/);         var diceMod = args[1]         simpleResolveChange(diceMod);     } }); the modification here removes the need for any circular/recursive function calls, and handles any number without needing the "changeResolve" function to receive a mod of 0 to notify. If you know a max/min you can set those as their own variables and use those to directly modify the pool's size, vs the += and -= you're currently using to get them back to 0. While it does work to do it that way, its harder to read which you'll hit yourself for in the future, believe me! if you get stuck, try writing in psudocode using comments to figure out what needs to be done at each step, it sounds dumb but it can really help. I think this was a good start and without testing your code I think it would function, you just went a bit overkill on a problem that in the end is really just adding or subtracting 2 numbers from eachother and returning a bounded number. Look into Max/Min functions as well in order to potentially simplify even more! good work