RPG Stack Exchange chat bot

Recently, I made a small bot to simulate an ASCII-based RPG in a chat room. The other users can enter commands (.left, .right, .up, .down) to move the character.

It's made with JavaScript and everything is in global scope, so I can simply enter it in the console window to create a game.

This version can run outside of a chatroom by entering it to the developer console:



`var target = document.getElementById("chat");

var walkable = [1, 4, 5, 6];
var inCave = false;
var gold = 0;

var obs = new MutationObserver(function(muts) {
if (muts[0].addedNodes[0].className.indexOf("mine") == -1) {
var messages = document.getElementsByClassName("content");
var lastMessage = messages[messages.length - 1].innerHTML;

if (lastMessage.charAt(0) == ".") {
walk(lastMessage.slice(1));
}
}
});

var config = { attributes: true, childList: true, characterData: true};
//obs.observe(target, config);

// Code above this is used to get the last message

var map = [];
var mapSize = 20;
var caveSize = 10;
var currMapSize = mapSize;

for (var y = 0; y = 0 && y >= 0 && x = currMapSize) {
player.x = currMapSize - 1;
}
if (player.y = mapSize) {
player.y = currMapSize - 1;
}

if (map[player.x + player.y * currMapSize] == 4) {
map[player.x + player.y * currMapSize] = 1;
inCave = true;
for (var y = 0; y



To enter a command, you need to write:

walk();

First, I would suggest moving up the functions higher in the code, preferably in the beginning of the code for visibility. It's nice to know functions exist before you see code that actually uses it.

Next is that your functions are trying to do too much. For instance, walk I expect to just update the player's position. However, inside walk, you have gold updates, and other state changes. Break these operations to smaller functions for maintainability. You don't want to be updating several functions for just gold updates.

In addition, this game is turn-based. It's a simple get-input-and-return-output which fits nicely with the functional paradigm. For instance, we could have a move function that accepts the current state and returns the next state. Given the same input, it will always give out the same result. Also, it won't mutate the input. It will give out a new object all the time.

var currentState = {
  direction: 'left',
  mapState: [0,0,0,0,1,G,0,0,0],
  player: { id: 1, gold: 0, hp: 100}
};

var nextState = move(currentState);

// {
//   mapState: [0,0,0,0,0,1,0,0,0],
//   player: { gold: 10, hp: 100 }
// }

if (map[player.x + player.y * currMapSize] == 4)
if (map[player.x + player.y * currMapSize] == 5)
if (map[player.x + player.y * currMapSize] == 6)

Don't make me read the code to know what 4, 5 and 6 are. Put them in variables so they are readable. If they belong to a common set of items, use an object.

var mapItems = {
  treasure: 4,
  arena: 5,
  gold: 6,
}

if (map[player.x + player.y * currMapSize] == mapItems.treasure)
if (map[player.x + player.y * currMapSize] == mapItems.arena)
if (map[player.x + player.y * currMapSize] == mapItems.gold)

Also, map[player.x + player.y * currMapSize] is being repeated. Consider extracting to a function.

StackExchange Code Review Q#132979, answer score: 2