JavaScript text-based RPG

Question

I'm making a text-based RPG in JavaScript. It works but I'd like to know how to improve it, like code-wise. Also, I want to know how to make it run on anything other than Internet Explorer.

var canvas = document.createElement("canvas");
    canvas.id = "canvas";
    var ctx = canvas.getContext("2d");

    canvas.width = 1280;
    canvas.height = 720;

    document.body.appendChild(canvas);
	
var bgReady = false;
var bgImage = new Image();

bgImage.onload = function () {
	bgReady = true;
};
bgImage.src = "images/background.bmp";

var inventory = new Array();
var map = new Array();
var T = 404;

var events = ["nothing","enemy","shop"];
var gamestate = "";
var lines = new Array();
var text = "";
var textposx =32;
var textposy = 33;

var playerx = 50;
var playery = 0;
var dead = false;
var weapon = new Array();
var armor = new Array();
var shield = new Array();

var mapsizex = 100;
var mapsizey = 100;
var finished = false;
var enemytypes = new Array();

player = new Object()
player.health = 300;
player.gold = 0;
player.xp = 0;
player.level = 0;

// weapon object
function item(I,wname,acc,dmg,cat,b1,b2) {
          I = I || {};
		  I.type = cat; // type 1 = weapon, 2 = armor, 3 = healing item, 4 = stat enchancements
		  I.name = wname;
		  I.catergory = cat;
		  I.rating = acc;
		  I.damage = dmg;
		  I.bonus = new Array(b1,b2); //defence bonus,armor bonus
		  return I;
		  }
		  
weapon.push(item(0,"short sword",100,60,"weapon",0,0));
armor.push(item(0,"leather armor",0,0,"armor",30,3));

//after adding a new stat, dont forget to edit the spawnMonster() function!
function enemy(I,ename,hp,acc,dmg,def,arm,exp,lvl){
  I = I || {};
  I.name = ename;
  I.health = hp;
  I.rating = acc;
  I.damage = dmg;
  I.defence = def;
  I.armor = arm;
  I.level = lvl;
  I.xp = exp;
  I.type = 1;
  return I;
}
//enemy database
//level 1 
enemytypes.push(enemy(0,"Thief",200,65,15,8,0,250,1));
enemytypes.push(enemy(0,"Zombie",250,60,10,15,0,300,1));
enemytypes.push(enemy(0,"Fallen",180,80,15,20,3,350,1));
enemytypes.push(enemy(0,"Yeti",325,70,10,10,0,450,1));
enemytypes.push(enemy(0,"Skeleton",150,60,10,10,0,200,1));
enemytypes.push(enemy(0,"Lion",125,120,25,10,0,300,1));


function randString()
{
    var rtext = "";
    var possible = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";

    for( var i=0; i < 5; i++ )
        rtext += possible.charAt(Math.floor(Math.random() * possible.length));

    return rtext;
}

function spawnMonster(place)
{
map[place]=new Array();
var x = Math.round(place/100);
var y = place%mapsizey;
var numgenmons = 1+Math.round(Math.random()*5);
for (var i = 0; i<numgenmons; i++){
var num = Math.round(Math.random()*5);
map[place].push(enemy(0,enemytypes[num].name,enemytypes[num].health,enemytypes[num].rating,enemytypes[num].damage,enemytypes[num].defence,enemytypes[num].armor,enemytypes[num].xp,enemytypes[num].level)); 
}
//name,hp,acc,dmg,def,arm,lvl
}

generate = function(){
for (var i = 0; i < (mapsizex*mapsizey); i++){
var r = Math.random();
if(r<0.5){
spawnMonster(i);
//50 % chance of encountering a monster/enemy
}

}
finished = true;


}
generate();

function eventCheck(){
var tempname = map[playerx*mapsizex+playery][0].name;
if(tempname!=undefined){
text = "you encountered a "+tempname+" !";
write();
gamestate = "enemy"; 
}

}

doShit = function(){

if(gamestate=="enemy"&&dead==false){
fight();
}
}

function targetCheck(){
T = 404;
for (var i=0;i<10;i++){
if(map[playerx*mapsizex+playery][i]!=undefined){
T = i;
return;
}
}

}

function fight(){

targetCheck();
if(T==404||map[playerx*mapsizex+playery][T].health<0){
alert("Target not found/dead!");
}

// player attacks enemy
var c = Math.random()*weapon[0].rating;
var d =  map[playerx*mapsizex+playery][T].defence;
//comparing your attack rating to enemies' defence rating
if(c>d){
map[playerx*mapsizex+playery][T].health-=(weapon[0].damage-map[playerx*mapsizex+playery][T].armor);
text = "You succesfully attacked the enemy with "+(weapon[0].damage-map[playerx*mapsizex+playery][T].armor)+".";
write();
}else{
text = "You tried to attack the enemy, but failed to inflict any damage.";
write();
//alert(c+" : "+d);
}
if(map[playerx*mapsizex+playery][T].health <= 0){
text = "You killed the "+map[playerx*mapsizex+playery][T].name+" !";
write();
player.xp+=map[playerx*mapsizex+playery][T].xp;
if(player.xp>Math.pow((player.level+1)*10,2)){
player.level++;
}
//50% chance of loot drop
if(Math.random()<0.5){
dropLoot(map[playerx*mapsizex+playery][T].level);
}
delete map[playerx*mapsizex+playery][T];
targetCheck();
if(T==404){
text = "There are no enemies left. You won the battle!";
write();
gamestate="";
}else{
text = "You encountered a "+map[playerx*mapsizex+playery][T].name+".";
write();
}
return;
}

//enemy attacks player
var c2 = Math.random()*map[playerx*mapsizex+playery][T].rating;
var d2 =  armor[0].bonus[0];
//comparing your defence rating to enemies' attack rating
if(c2>d2){
player.health-=(map[playerx*mapsizex+playery][T].damage-armor[0].bonus[1]);
text = "The enemy succesfully attacked you with "+(map[playerx*mapsizex+playery][T].damage-armor[0].bonus[1])+" damage.";
write();
if(player.health <= 1){dead = true,text="You died!",write()};
}else{
text = "The enemy failed to attack you.";
write();
//alert(c+" : "+d);
}

}

function dropLoot(level)
{
var kaas = Math.round(Math.pow((Math.random()*level*30),1.2));
player.gold+=kaas;
text = "You found "+kaas+" gold!";
write();
}

move = function(n){
if(gamestate==""&&dead==false){

if(n==3){
if(playerx<mapsizex){
playerx++;
text = "You moved east!";
}
}

if(n==2){
if(playerx>0){
playerx--;
text = "You moved west!";
}
}

if(n==1){
if(playery<mapsizey){
playery++;
text = "You moved north!";
}

}

write();
eventCheck();
}
}


function switchWeapon(){

}

write = function(){
//error was ~= instead of !=

if(textposy<650&text!=""){
lines.push(text);
}

}
function updateText()
{
ctx.fillText("\n \n",textposx, textposy); 
ctx.fillStyle = "rgb(0, 0, 0)";
ctx.font = "24px Helvetica";
ctx.textAlign = "left";
ctx.textBaseline = "top"; //top
	
textposy = 10;
textposx = 33;
if(lines.length<30){var scroll = 0}else{var scroll = lines.length-30};
for (var i=scroll; i<lines.length; i++)
{
textposy += 20;
ctx.fillText(lines[i],textposx,textposy); 
}
ctx.fillText("Your health: "+player.health,850,20); 
ctx.fillText("Your gold: "+player.gold,850,50); 
ctx.fillText("Your experience: "+player.xp + " / "+Math.pow((player.level+1)*10,2),850,80); 
ctx.fillText("Your level: "+player.level,850,110); 
}
setup = function() {
  
 
ctx.clearRect(300,0,1000,1000);
if (bgReady&&finished) {
		ctx.drawImage(bgImage,0,0); //draw background first
	}  
  	updateText(); // after that, draw text
//alert(map[400][0].name);
 
}

//setInterval(setup,250);
//setTimeout(setup,1500);
setInterval(setup,250);

.button {
    position: fixed;
    bottom: 245px;
    right: 1200px;
}
.switch{
    position: fixed;
    bottom: 780px;
    right: 710px;
}

<!DOCTYPE html>
<html lang="en">

<link rel="stylesheet" href="css/style.css" type="text/css">
	<head>
		<meta charset="utf-8">
		<title>rpg game</title>
	</head>
	<body>
<script src="javascript/diablo2.js"></script>
<div class="button">
<button onclick="move(3)">Go east</button>
<button onclick="move(2)">Go west</button>
<button onclick="move(1)">Go north</button>
<button onclick="doShit()">Action</button>
</div>
<div class = "switch">
<button onclick="switchWeapon()">Switch weapon</button>
</div>

	</body>
</html>

Answer 1

• You have a lot of inconsistent indentation. (See link at the bottom for an easy way to fix this)

• Inconsistent positioning of {, are you going to place it on it's own line (common C# style) or on the previous line (common Java style)? The JavaScript conventions is to use the same conventions like Java for this.

• Put your global variables (playerx, playery, weapon, armor, shield, mapsizex, finished...) into an object, such as game. Perhaps weapon, armor and shield should be a part of the player object?

• Unclear variable names: I, acc, arm (there's also nothing wrong in using damage instead of dmg) Use variable names that are not too long (theDamageOfTheEnemy would be too long) but still long enough to be descriptive. (damage is fine, or sometimes enemyDamage)

• This comment indicates a code smell:

  //after adding a new stat, dont forget to edit the spawnMonster() function!

  When you have code that needs to be edited on multiple places in order to work, something is wrong. Instead consider using a enemy(enemytype) method that creates an enemy from an object. Instead of using nine (!!) parameters you can use one object that contains nine properties. Then you won't have to keep track of the order of the parameters. enemy({ hp: 20, damage: 40, experience: 23, level: 15 })

• Perhaps my JavaScript knowledge is a bit rusty, but I = I || {}; looks strange to me. Instead, consider using if-else to handle that logic to make it more clear what you are doing. (Disregard this one, apparently it is very common and actually seems very useful. However, see tomdemuyt's answer for a better solution)

• This method:

  generate = function(){
  for (var i = 0; i < (mapsizex*mapsizey); i++){
  var r = Math.random();
  if(r<0.5){
  spawnMonster(i);
  //50 % chance of encountering a monster/enemy
  }
  
  }
  finished = true;
  
  
  }
  

  The readability is almost zero, use proper indentation and spacing and get rid of some empty lines:

  generate = function() {
      for (var i = 0; i < (mapsizex * mapsizey); i++) {
          var r = Math.random();
          if (r < 0.5) {
              spawnMonster(i);
              //50 % chance of encountering a monster/enemy
          }
      }
      finished = true;
  }
  

  And it suddenly becomes so much more readable! This should be applied to all your code.

• 1+Math.round(Math.random()*5); that's not the proper way to generate a number. You've used the correct way at one place, Math.floor(Math.random() * ...). Why is this? Because the first number (0) and the last number(5) will have slightly lower probability than the others. Math.random() is guaranteed to return a number from 0 (inclusive) to 1 (exclusive). Therefore, to generate one of the numbers 1 2 3 4 5 6, you should use: Math.floor(Math.random() * 6) + 1. This will give equal probabilities to all of the numbers.

Honestly, there are more things that needs to be cleaned up in your code, this is just a start. @tomdemuyt provided more very important things.

Consider using an Online JavaScript beautifier for your code every now and then. This tool doesn't take care of refactoring your code though, such as renaming variables and organizing variables into objects, and rewriting methods to use one parameters instead of nine, but it is a handy tool to clean up some things at least.

Answer 2

General

One of the best ways to learn a language is to write an RPG in it, because you can write working code with just basic knowledge, and then you keep polishing for a really long time.

Since your game has a respectable 300 lines which is definitely going to grow, I would advise you to organize your code into objects.

Approach

I gave your code a read through and comment on things as I see them.

UI

You should have a UI object with an init function which will build the UI.

var UI =
{
  init : function()
  {
    var canvas = document.createElement("canvas");
        canvas.id = "canvas";
        this.ctx = canvas.getContext("2d");

        canvas.width = 1280;
        canvas.height = 720;

        document.body.appendChild(canvas);

    var bgReady = false;
    var bgImage = new Image();

    bgImage.onload = function () {
        bgReady = true;
    };
    bgImage.src = "images/background.bmp";
  },
  write : function
  {
    ....
  },
  update: function
  {
    ....
  }
}

On a side note, you probably want those magical constants 1280 and 720 placed on top of your code in constants. Furthermore, you are loading a bmp, this might be the reason that this only works in IE, considering using a png or jpg file instead.

Rogue Variables

Your code then has a number of variables that you should try to place inside an object.

These could belong to a game object:

var events = ["nothing","enemy","shop"];
var gamestate = "";
var finished = false;

Note that you should try to stick to 1 naming convention, ideally lowerCamelCase, so you would use gameState instead of gamestate or game_state.

These could belong to the UI object:

var lines = new Array();
var text = "";
var textposx =32;
var textposy = 33;

These could be long to the player object that you have already

var playerx = 50;
var playery = 0;
var dead = false;
var enemytypes = [];
var weapon = [];
var armor = [];
var shield = [];

Notice how I replaced with new Array(); with [], which is considered more appropriate.

These could belong to a dungeonMap object:

var map = new Array();
var mapsizex = 100;
var mapsizey = 100;

you could even call the property size and assign { x : 100 , y : 100 } to it. A map, in essence, is a 2d array, you should really consider using that instead of a single array, it will make things much easier.

Also, I would advise you to use a different way to init the player object:

player = 
{
  health : 300,
  gold : 0,
  xp : 0,
  level : 0,
  x : 50,
  y : 0,
  dead :false,
  inventory : []  
}

instead of generating the object and assign properties to it (DRY).

Items

On to item/weapon/armor/shield :

// weapon object
function item(I,wname,acc,dmg,cat,b1,b2)
{
  I = I || {};
  I.type = cat; // type 1 = weapon, 2 = armor, 3 = healing item, 4 = stat enchancements
  I.name = wname;
  I.catergory = cat;
  I.rating = acc;
  I.damage = dmg;
  I.bonus = new Array(b1,b2); //defence bonus,armor bonus
  return I;
}

You have realized that armor/weapon/shield is close enough that you can use the same object, plus this will give you the option to create weapons with defense bonuses and armor with attack bonuses etc. I would change this function to be a constructor though and hence drop the I argument.

function Item( name, rating, damage, type, db, ab )
{
  this.name   = name;
  this.rating = rating;
  this.damage = damage;
  this.type = type; 
  this.bonus = { defense : db , armor : ab };
}

I dropped catergory(sic) since you store that in type.

You should have constants to represent the type, you could store them in item

Item.prototype.types = 
{
  WEAPON = 1,
  ARMOR = 2,
  HEALING = 3,
  STATBONUS = 4,
}

weapon.push(item(0,"short sword",100,60,"weapon",0,0));
armor.push(item(0,"leather armor",0,0,"armor",30,3));

could then become

weapons.push(new Item("short sword",100,60,Item.types.WEAPON,0,0));
armors.push(new Item("leather armor",0,0,Item.types.ARMOR,30,3));

Enemies

See Items

Functions

randString

• randString is interesting, I am not sure what you would do with this, since you do not seem to use it anywhere, you might as well drop it.

spawnMonster

• This function should be part of the dungeonMap object.
• It could really use some indenting
• var x = Math.round(place/100); -> you dont use x
• var y = place%mapsizey -> you dont use y
• numgenmons could be called monsterCount
• 1+Math.round(Math.random()*5); -> You should scour your code and put all Math.random related code into an RNG object. ( Random Number Generator, all praise him, for he giveth and taketh away ).
• To push a new monster, I would use the built in JSON to clone the monster map[place].push( JSON.parse( JSON.stringify( enemytypes[num] ) ) );

generate

• Not sure why you chose generate = function(){ instead of function generate(){
• So much more could be done there, think walls, treasures, traps, quests, NPC's etc.

eventCheck

• You should replace direct map access with a call to the dungeonMap object
• The code does not seem to check what happens if you come back to a cell where all monsters were defeated, bug ?
• Indenting!
• gamestate = "enemy"; -> "enemy" should be a constant, declared at the top or as part of the game class

targetCheck

• 404 is cute
• since the monster are added randomly to the array, just pick the first instead of a random one, then you can just check .length() of the cell and maybe you wont even need this function anymore, since it would be a one liner

fight

Combat, the crux of RPG!

• Whenever you write alert(something), consider console.log( something ) or write( something), this way only developers will see those messages
• text should be a parameter of write, globals are evil in general, but especially here
• 'c' and 'd' , could be better named
• From a design perspective, 'd' should have a random factor as well I feel
• [playerx*mapsizex+playery][T] & weapon[0]
  • You could just var monster = [playerx*mapsizex+playery][T]
  • You could just var weapon = player.weapon[0]
  • Then you can write

if( attackScore > defenseScore ){
  var damage = weapon.damage - monster.armor
  monster.health-= damage;
  write( 'You succesfully attacked the enemy for ' + damage + '.' );
}

• Indent your code!
• The code where player attacks monster, and the code where monster counter attacks should really be in 2 different functions
• Consider to give the monster a bonus if there are several monsters in the same place ?
• Finally, player/monster level should probably increase the attackScore ?

dropLoot

• Just had to say I love kaas as well

move

• INDENT!
• consider putting the directions into an object

directions = 
{
  1 : { x : +0 , y : +1 , s : 'north' },
  2 : { x : -1 , y : +0 , s : 'west'  },
  3 : { x : +1 , y : +0 , s : 'east'  },
  4 : { x : +0 , y : -1 , s : 'south' } /* Did you omit south on purpose ? */
};

• with that object you could check directions[ n ] to check for a valid direction and modify player.x and player.y and build a string to show where the player is going

updateText

• This should be part of a UI object
• indent..
• There are a ton of magical constants that you should tidy up
• Use newlines, dont be tempted to do this : if(lines.length<30){var scroll = 0}else{var scroll = lines.length-30};
• fillStyle, font, textAlign, and textBaseline should be part of initialization, they dont change

Conclusion

Much work to be done, please come back after you have implemented some/all of the code review answers, it was fun to read.

Answer 3

As for running this code locally, your browser will be able to run it locally so long as all the resources are present and all links to those resources are relative. Yours links are already relative. It runs (for me) locally in firefox using this directory structure:

cr37672/
├── 37672.html
├── css
│   └── style.css
├── images
│   └── background.bmp
└── javascript
    └── diablo2.js

If I open the file 37672.html in my browser of choice game renders and appears to function.

This can be a useful setup especially if you have an IDE that provides you a 'live view' function where changes made in the editor cause the browser to automatically refresh.

Make life easier and structure your code.

Make your life easier and write tests -- it'll inform your design.