HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Metal King: A JavaScript Game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
kingmetalgamejavascript

Problem

I started to learn programming just a few days ago. And I've written code for a simple original game for practice, using some knowledge of JavaScript that I have gained so far.

Is the following code properly written? I'm especially concerned about the use of "Math.floor", "Math.random", "function", and "if / else".

Incidentally, "Metal King" is the name of a monster in the game, and "Angelo" is a character.

```
alert("Metal Kings (A, B, and C) have appeared!");

// Which Metal King (A, B, or C) will Angelo decide to attack? \
// 33% probability for each.

var MetalKingABC = 1 + Math.floor(Math.random() * 3);

if (MetalKingABC === 1) {
MetalKingABC = "Metal King A";
}
else if (MetalKingABC === 2) {
MetalKingABC = "Metal King B";
}
else {
MetalKingABC = "Metal King C";
}

var WhichOneToAttack = function(MonsterName) {

alert("Angelo is about to attack " + MonsterName + ".");

// Metal King has a 50% chance of fleeing the battle.

var FleeOrFight = 1 + Math.floor(Math.random() * 2);

if (FleeOrFight === 1) {
alert(MonsterName + " flees like lightning!");
// The battle ends here before Angelo attacks.
}

else // Metal King stays, and Angelo attacks.
{
// 70% chance of MISS, 20% of ONE damage, 10% of Critical.

var MissOneCritical = 1 + Math.floor(Math.random() * 10);

if (MissOneCritical <= 7) {
alert(MonsterName + " swiftly evades Angelo's attack.");
}

else if (MissOneCritical <= 9) {
alert("Angelo inflicts 1 point of damage on " + MonsterName + ".");
}

else {
// The damage dealt by a critical hit can range from 197 to 256 points.

var CriticalRange = 197 + Math.floor(Math.random() * 60);

alert("Angelo lands a critical hit on " + MonsterName + ", dealing massive \
damage of " + CriticalRange + " points! Angelo defeats " + MonsterName + ".");

// When defeated, Metal King

Solution

First, your game doesn't have a name! (Which is why I renamed the question like that...)

A few direct things to focus on:

if (MetalKingABC === 1) {
    MetalKingABC = "Metal King A";
}
else if (MetalKingABC === 2) {
    MetalKingABC = "Metal King B";
}
else {
    MetalKingABC = "Metal King C";
}


This is not how you do that.

Instead of using 1 + Math.floor get rid of the 1 as 0 is a valid array index, and is helpful in many situations.

Instead of using 2, alongside an array, use array.length.

Instead of using an if-else loop with duplicate logic (string assignment), use an array instead.

var metalKingTypes = ['A', 'B', 'C'];
var metalKing = metalKingTypes[Math.floor(Math.random() * metalKingTypes.length)];


Instead of using MissOneCritical: use a switch

After you decide who to attack, and then attack them, the game is over! Even if you miss or only do 1 damage!

Set a isActive variable to keep the game going.

A few indirect things:

  • WhichOneToAttack: could be better named: DecideOpponent



  • FleeOrFight: You naming structure should be camelCase where the first word is entirely lowercase.



  • You can use this to get a random true or false value: Math.floor(Math.random() * 2)



Your structure:

You have a bunch of loose functions that aren't linked to one another in any other way than the fact that they call each other.

Use a class structure, or a prototype chain, that way you can avoid passing parameters around:

Taking those changes into consideration would give you something like this:



function Game(){
alert("Metal Kings (A, B, and C) have appeared!");

/* Which Metal King (A, B, or C) will Angelo decide to attack?
* 33% probability for each.
*/


var metalKingTypes = ['A', 'B', 'C'];
this.monster = "Metal King " + metalKingTypes[Math.floor(Math.random() * metalKingTypes.length)];
this.isActive = true;
while (this.isActive) {
this.DecideOpponent();
}
}
Game.prototype.DecideOpponent = function(){
alert("Angelo is about to attack " + this.monster + ".");

// Metal King has a 50% chance of fleeing the battle.

var fleeOrFight = Math.floor(Math.random() * 2);

if (fleeOrFight) {
alert(this.monster + " flees like lightning!");
// The battle ends here before Angelo attacks.
return false;
}
// 70% chance of MISS, 20% of ONE damage, 10% of Critical.
var attackOptions = ['evade', 'normal', 'critical'];

var attack = attackOptions[Math.floor(Math.random() * attackOptions.length)];
switch (attack) {
case 'evade':
alert(this.monster + " swiftly evades Angelo's attack.");
break;
case 'normal':
alert("Angelo inflicts 1 point of damage on " + this.monster + ".");
break;
case 'critical':
var CriticalRange = 197 + Math.floor(Math.random() * 60);

alert("Angelo lands a critical hit on " + this.monster + ", dealing massive " +
"damage of " + CriticalRange + " points! Angelo defeats " + this.monster + ".");

// When defeated, Metal King drops a shield with an 80% probability.

var shieldDrop = Math.floor(Math.random() * 2);

if (shieldDrop) {
alert(this.monster + " drops a shield. Angelo obtains a Diamond Shield!");
}
this.isActive = false;
break;
}
};

// Finally, call the function.

var g = new Game();




Additionally, instead of using alert consider building each line of the game to the console or to a HTML element.

Something like:

function buildToHTML(text){
    var field = document.getElementById('gameField');
    field.text += text;
}

Code Snippets

if (MetalKingABC === 1) {
    MetalKingABC = "Metal King A";
}
else if (MetalKingABC === 2) {
    MetalKingABC = "Metal King B";
}
else {
    MetalKingABC = "Metal King C";
}
var metalKingTypes = ['A', 'B', 'C'];
var metalKing = metalKingTypes[Math.floor(Math.random() * metalKingTypes.length)];
function buildToHTML(text){
    var field = document.getElementById('gameField');
    field.text += text;
}

Context

StackExchange Code Review Q#113317, answer score: 5

Revisions (0)

No revisions yet.