@janos already covered this a little bit, but if you want to use the built-in random(x, y) used in Processing.JS, I'd do something like this:
var numberOfDots = random(1, 6);
But, we need to do one more thing. Since the random(x, y) function in Processing.JS can return non-integer results, we need to use the floor function to round the returned value down, and then add one. This means that your calculation simply becomes this:
var numberOfDots = floor(random(1, 6)) + 1;
Now you should be able to generate random numbers from 1-6 in a cleaner, and more readable manner.
I've also noticed that you're repeating a lot of code. @Caridorc already touched on this a little bit, but I'm going to expand on it a bit further. Rather than keeping it in the mouseClicked override, you can simply put it at the top of your code like this:
fill(245, 242, 242);
rect(100, 100, 200, 200);
mouseClicked = function() {
...
}
This is a good thing to remember when you're doing graphics programming. If there's no need to re-draw something every frame, or click, then don't. In this case, you don't need to re-draw and fill the rectangle, so it can be moved entirely out of the mouseClicked override. In the case of this program though, I felt that the rectangle wasn't needed, so I just removed it altogether.
Expanding on the subject of code repetition, you're also repeating a lot of ellipse calls. I'd highly suggest implementing a function that automatically draws x amount of circles for you. This is what I came up with:
var drawDiceDots = function(dotPositions, dotDiameter) {
for(var i = 0; i <= dotPositions.length - 1; ++i) {
ellipse(dotPositions[i][0], dotPositions[i][1], dotDiameter, dotDiameter);
}
};
To use this function, you supply a list of x-y positions in the format [[x, y], ...]. For example, in order to render the dots on the 6-dot side of a die, you'd do this:
drawDiceDots([
[150, 150],
[150, 250],
[250, 250],
[250, 150],
[250, 200],
[150, 200]
], 45);
Finally, rather than using a chain of if statements, I'd recommend using a switch/case construct. A typical switch/case construct looks something like this:
switch(someVariable) {
case someValue:
...
break;
...
}
But, we can get better than a chain of if statements. We can either use a JavaScript object or a list. In this case, I'm going to use a list. To do this, you'd first create a list that looks like this:
var possibleDiceFaces = [
[100, [200, 200]],
[70, [150, 150], [250, 250]],
[60, [150, 150], [200, 200], [250, 250]],
[60, [150, 150], [150, 250], [250, 250],[250, 150]],
[60, [150, 150], [150, 250], [250, 250], [250, 150], [200, 200]],
[45, [150, 150], [150, 250], [250, 250], [250, 150], [250, 200], [150, 200]]
];
The first value of each list is the diameter of the circles, and the rest are positions. The nice thing about this is that it's easily expandable, rather than having to add a new case or if statement each time you want to add a "side".
In addition, thanks to the tips from @Jonah, I'd also recommend the following things:
- Separate responsibilities, e.g, simply call a dice drawing function in the
mouseClick override, rather than having a huge pile of case statements.
- Use more domain-oriented names for things, rather than implementation-oriented.
After making all these changes, plus a few more, I ended up with the following, improved (hopefully) code:
/**
* Draw x amount of dots given a list of positions
* for a dice.
* @param {list[][]} dotPositions - A list of dot positions in the form [x, y].
* @param {number} dotDiameter - The diameter of the dots.
*/
var drawDiceDots = function(dotPositions, dotDiameter) {
for(var i = 1; i <= dotPositions.length - 1; ++i) {
fill(0, 0, 0);
ellipse(dotPositions[i][0], dotPositions[i][1], dotDiameter, dotDiameter);
}
};
/**
* Draw the fact of a dice. By default, there are 6 different faces,
* but this number can be changed to add more.
*/
var drawDiceFace = function(rolledNumber) {
var possibleDiceFaces = [
[100, [200, 200]],
[70, [150, 150], [250, 250]],
[60, [150, 150], [200, 200], [250, 250]],
[60, [150, 150], [150, 250], [250, 250],[250, 150]],
[60, [150, 150], [150, 250], [250, 250], [250, 150], [200, 200]],
[45, [150, 150], [150, 250], [250, 250], [250, 150], [250, 200], [150, 200]]
];
if(rolledNumber in possibleDiceFaces) {
drawDiceDots(possibleDiceFaces[rolledNumber], possibleDiceFaces[rolledNumber][0]);
}
else {
throw "The rolled number specified is not valid.";
}
};
mouseClicked = function() {
background(255, 255, 255);
var rolledNumber = floor(random(1, 6)) + 1;
drawDiceFace(rolledNumber);
};
Finally, I'd just like to nitpick a few things:
- Generally you should write braces in JavaScript on the same line as the declaration, due to evil things like automatic semicolon insertion.
- You should properly indent everything, including comments.
- Don't use comments when your code is self-explanatory.
Other than that, nice work! :)
