1
\$\begingroup\$

I'm currently creating a text based game where the user navigates through the game via buttons and i'm trying to improve on my code because i'm pretty sure it could be alot better, the following is some code which i wrote to give the user some options (via html buttons) and then check what their selection is in order to execute the correct function.

class Room {
    
    constructor(description, options) {
        
        this.description = description;
        this.options = options;
        
    }
}

 var f1hallway = new Room("You see two rooms", ["Room 1", "Room 2"]);

 var f1room1 = new Room("You find a letter on a table", ["Read it", "Exit"]);
    
 var f1room2 = new Room("Nothing of interest here", ["Exit"]);

    whichArea = function(areaSelected) {
                 
                selectedArea = areaSelected;
                areaSelected();
                
              }
              
    function presentChoices(options) {
        
              choices.innerHTML = "";
            
         for (var x = 0; x < options.length; x++) {
                  
            choiceButton = document.createElement("button");
            choiceButton.setAttribute("class", "choice-button");
            choices.appendChild(choiceButton);
            choiceButton.innerHTML = options[x];
            selectedChoiceIndex = x;
            choiceButton.setAttribute("id", x);
            
             choiceButton.onclick = function() {
        
                selectedChoice = this.innerHTML;
                chosenOptionId = this.id;
                whichArea(selectedArea);
                
             }
             
             
        }
    
    }

function f1hallwayFunc() {

        mainText.innerHTML = f1hallway.description;
        choices.innerHTML = "";
        presentChoices(f1hallway.options);
        
        if (chosenOptionId == 0) {
     
            chosenOptionId = null;
            whichArea(f1roomOneFunc);
          
        
        }
        
        else if (chosenOptionId == 1) {
            
            chosenOptionId = null;
            whichArea(f1roomTwoFunc);
            
        }

function f1roomOneFunc() {

    mainText.innerHTML = f1room1.description;
    presentChoices(f1room1.options);
    
     if (chosenOptionId == 0) {
        
        chosenOptionId = null;
        mainTextWrap.innerHTML = "You start reading the letter";
        
    }
    
    else if (chosenOptionId == 1) {
        
        chosenOptionId = null;
        whichArea(f1hallwayFunc);
        mainText.innerHTML = "You exit the room and return to the hallway";
        
    }

}

function f1roomTwoFunc() {
    
    mainText.innerHTML = f1room2.description;
    presentChoices(f1room2.options);
    
     if (chosenOptionId == 0) {

        chosenOptionId = null;
        whichArea(f1hallwayFunc);
        mainText.innerHTML = "You exit the room and return to the hallway";
        
    }
    
}

The code works fine but seems like a mess at the same time, each area that the user ends up in goes through this same process, can someone please tell me how this could be written better and any best practices i'm missing? I'm still quite new to programming and would appreciate any help, thanks!

\$\endgroup\$
0
\$\begingroup\$

Programming rules of thumb.

  • If you find yourself prefixing many names with the same prefix it is a sign that you should be creating an object to hold these variables.

  • Many functions doing similar things is a sign that one function with arguments defining things can do the same.

  • Always separate data from code. You should not need to change code when you change the data and you should design the data so that changes to the code does not effect unrelated data.

    Creating games is a very creative and iterative process. Having to change code each time you change some aspect of the game impedes the creative process, the end result is a so so game. Separating data from code make it much easier to create a great game.

Encapsulate

A program is created to manage abstractions. For example your game has the abstract entities Rooms, Room, Option, etc..

You should encapsulate abstract concepts withing Objects so that it becomes much easier to manage and work with these abstract objects rather than a long list of loosely related functions.

Use maps

When performance is not critical use named references via maps, rather than indexes to locate data. We humans are much better at associating names to items, than numbers.

A map can be an object with named properties. Eg

rooms = {};
rooms["room1"] = new Room(...);
rooms["room2"] = new Room(...);

Code noise

Code noise is anything that is not absolutely needed to achieve some task. Always strive to reduce code noise.

Repeated verbose code

The DOM interface is a rather verbose and ugly thing. You can greatly reduce the amount of noise in your code by creating functions to do these often repeated calls.

The example creates functions to work with the DOM tag, append, listener. Then some higher level functions to help with the game clearOptions, createOptionBtn, showActionText that use the DOM functions to do their thing.

Then when you want to add a choice you can call createOptionBtn rather than a long list of obscure DOM calls.

Rewrite

The rewrite is as an example only, there is no right or wrong way to write this type of code. If you are new to coding it may be too complex, if so take what you can from it. It is better to write code you are comfortable with than struggle with code you don't understand.

The rewrite is from the ground up.

It completely separates the game logic from the game data.

It encapsulates

  • Room A room has a description, and may contain named options.
  • Option An option, has a name and actions. Actions define what the options will do. In the example I did not formalize the actions within an abstract Object as I did not plan out what the game should do.
  • rooms Holds all the rooms by name rooms.list it has a function to add a room, and handles the actions available to an option.

With the code in place the game is created by defining rooms and options.

For example to add a room with two options.

rooms.add("hallway", "You see two rooms",
    Option("Enter Room 1", {goto: "room1"}),
    Option("Enter Room 2", {goto: "room2"})
);

The first room has the letter. The option to read the letter has actions to replace the read letter option with a new option.

rooms.add("room1", "You find a letter on a table",
    Option("Read it", {
        do: "You start reading the letter. 'Something evil lurks in the other room'",
        addOption: {
            room: "room1",
            option: Option("Read it", {do: "You read the letter again"}),
        },
        update: {room: "room1"},
    }),
    Option("Exit", {goto: "hallway", desc: "You exit the room and return to the hallway"}),
);

Though only partly thought out you should be able to see how the game content is independent of the game logic. You could create a huge adventure without needing to add any more code.

If you find that the existing code does not cover all aspects of the game, you should be able to add extra features to the code logic without having to change any unrelated data.

;(()=>{
"use strict";
// General utils
const tag = (tag, props = {}) => Object.assign(document.createElement(tag), props);
const append = (par, ...sibs) => sibs.reduce((p, sib) => (p.appendChild(sib), p), par);
const listener = (el, type, call, opts = {}) => (el.addEventListener(type, call, opts), el);

// Game utils
const clearOptions = () => optionsContainer.innerHTML = "";
const createOptionBtn = option => listener(
    tag("button", {className: "choiceButton", textContent: option.name}),
    "click", option.clickEvent, {once: true}
);
const showActionText = text => {
    append(textContainer,  tag("div", {textContent: text}));
    textContainer.scrollTo(0, 2000000);
};

// game object
function Option(name, actions) {
    return  {
        get name() { return name } ,
        get actions() { return actions } ,
    };
}
function Room(description, ...options) {
    const optionSelectClick = option => event => {
        for (const action of Object.keys(option.actions)) {
            rooms.actions[action] && rooms.actions[action](option);
        }
    };

    const optionMap = {};
    const addOption = option => {
        optionMap[option.name] = option;
        option.clickEvent = optionSelectClick(option);
    };
    options.forEach(addOption);
    return {
        enter() {
            showActionText(description);
            clearOptions();
            append(optionsContainer, ...Object.values(optionMap).map(createOptionBtn));
        },
        addOption,
        removeOption(name) { delete optionMap[name] },
        update(desc = description) {
            description = desc,
            clearOptions();
            append(optionsContainer, ...Object.values(optionMap).map(createOptionBtn));
        },
    };
}
const rooms = {
    list: {},
    add(name, desc, ...options) { rooms.list[name] = Room(desc, ...options) },
    actions: {
        goto(option) {
            option.actions.desc && showActionText(option.actions.desc);
            rooms.list[option.actions.goto].enter();
        },
        do (option) { showActionText(option.actions.do) },
        addOption(option) {
            const add = option.actions.addOption;
            rooms.list[add.room].addOption(add.option);
        },
        removeOption(option) {
            const remove = option.actions.removeOption;
            rooms.list[remove.room].removeOption(remove.name);
        },
        update(option) { 
            const update = option.actions.update ;
            rooms.list[update.room].update(update.desc) 
        },
    }
};


// Game data
rooms.add("hallway", "You see two rooms",
    Option("Room 1", {goto: "room1", desc: "You enter the first room."}),
    Option("Room 2", {goto: "room2", desc: "You enter the second room." })
);

rooms.add("room1", "You find a letter on a table",
    Option("Read it", {
        do: "You start reading the letter. 'Something evil lurks in the other room'",
        addOption: {
            room: "room1",
            option: Option("Read it", {
                 do: "Reading the letter again you notice a drawing of what looks like bug spray",
                 addOption: {
                     room: "room2",
                     option: Option("Get bug spray", {
                          do: "You pickup the can of bug spray",
                          removeOption: {room: "room2", name: "Get bug spray"},
                          addOption: {
                              room: "room2",
                              option: Option("Spray bug", {
                                  do: "You spray the bug. It reacts with a hiss and disapares down a rather large hole in the floor boards",
                                  removeOption: {room: "room2", name: "Spray bug"},
                                  addOption: {
                                      room: "room2",
                                      option: Option("Inspect bug", {
                                          do: "The bug is gone",
                                          removeOption: {room: "room2", name: "Inspect bug"},
                                          update: {room: "room2"}
                                      })
                                  },
                                  update: {room: "room2", desc: "You enter an empty room"}
                             }),
                          },
                          update: {room: "room2"},
                     })
                 },
                 removeOption: {
                     room: "room1",
                     name: "Read it",
                 },
                 update: {room: "room1"}      
             }),
        },
        update: {room: "room1"},
    }),
    Option("Exit", {goto: "hallway", desc: "You exit the room and return to the hallway"}),
);

rooms.add("room2", "You find a huge bug",
    Option("Inspect bug", {
        do: "The bug is as big as you and looks hungry",
        addOption: {
            room: "room2",
            option: Option("Inspect bug", {
                goto: "endGame", desc:"The gigantic bug looms over and devours you" }),
        },
        update: {room: "room2"},
    }),
    Option("Exit", {goto: "hallway", desc: "You exit the room and return to the hallway"})
);

rooms.add("endGame", "Game over you die");


// game start
rooms.actions.goto({actions:{goto:"hallway", desc: "You are in a hallway, you hear a noise somewhere down the hallway."}});


})();
.choiceButton {
}

#textContainer {
  height: 140px;
  max-height: 140px;
  overflow-y: scroll;
}
<div id="textContainer"> 
</div>
<div id="optionsContainer"> 
</div>

\$\endgroup\$
0
\$\begingroup\$

I think a good way to approach this would be to integrate all the data (descriptions, options, and additional room choices) into an object indexed by room number or step name (since multiple things can be done in a single room). Starting at the first room, parse the data for that room to display the description and choices appropriately, and continue recursively whenever a new choice is made. Doing it this way means that, to set up the rooms, you only need to change around the object room numbers and strings instead of writing convoluted JavaScript logic.

For example, for the hallway and 2 rooms, you could have:

const steps = {
  hallway: {
    desc: 'You see two rooms',
    choices: ['Room 1', 'Room 2']
  },
  'Room 2': {
    desc: 'Nothing of interest here',
    // when the choice description and JS step name are not the same,
    // separate them
    choices: [{ desc: 'Exit', target: 'hallway' }]
  },
  'Room 1': {
    desc: 'You find a letter on the table',
    choices: [
      { desc: 'Read it', target: 'readLetter' },
      { desc: 'Exit', target: 'hallway', onChoiceDesc: 'You exit the room and return to the hallway' }
    ]
  },
  readLetter: {
    // ...
  },
}; 

Whenever a new step runs, if the choice that was just chosen had a onChoiceDesc property, display that. Otherwise, display the desc of the new step onto the page. Iterate through each choice to add a button, a button description, and a click handler.

To conditionally display a choice depending on program state, add a function to the choice object. Eg, if, after reading the letter, something becomes available in the hallway, you could add the following:

choices: ['Room 1', 'Room 2', { desc: 'Room 3', target: 'Room 3', showIf: () => hasReadLetter }]
readLetter: {
  desc: 'You start reading the letter. The letter informs you that Room 3 exists',
  choices: [
    { desc: 'Exit', target: 'hallway', onChoiceFn: () => hasReadLetter = true }
  ]
}

where onChoice gets invoked when the button is clicked (in addition to the default behavior of re-rendering the hallway step). (Make sure to have declared a hasReadLetter variable outside.)

That's the general idea - leave the manipulation of the DOM to the other code that parses through the steps object, while writing the meat of the game inside the steps object.


Although you'll probably be ditching most of it, some suggestions regarding your current code:

Classes are good for tying together data with methods related to the data. If all you have is plain data, a class doesn't help you. In your case, unless you were planning on adding methods later, using a plain object would make more sense for the Rooms.

Use consistent indentation so that you (and other readers of the code) can pick up on the { } blocks at a glance. Ideally, use an IDE that indents code properly and automatically. For example, this:

}
 var f1hallway = new Room("You see two rooms", ["Room 1", "Room 2"]);
    whichArea = function(areaSelected) {
                selectedArea = areaSelected;

should be

}
var f1hallway = new Room("You see two rooms", ["Room 1", "Room 2"]);
whichArea = function(areaSelected) {
    selectedArea = areaSelected;

Declare variables before using them - if you don't, you'll either be implicitly assigning to the global object (which is inelegant), or an error will be thrown if running in strict mode (and strict mode is recommended). Not declaring a variable before using it is a not-uncommon source of bugs when one finds that the variable isn't scoped as one expected it to be. The whichArea = should be let whichArea =.

Use modern syntax If you're going to write in ES6+ (which you are with the class, and as you should), never use var, it has too many gotchas to be worth using nowadays - use const instead when declaring variables, or use let when you have to reassign the variable.

Prefer IDL properties over setAttribute when feasible - IDL properties are usually more concise and easier to read and write. These two:

choiceButton.setAttribute("class", "choice-button");
choiceButton.setAttribute("id", x);

can be

choiceButton.className = "choice-button";
choiceButton.id = x;

(though, neither of those should be necessary at all: to target the button with CSS, you should be able to do .choices > button, by giving the parent a class name instead. Dynamic IDs are quite a code smell too - to get the index inside a click handler, either check the child's index in an array of all siblings, or use a data attribute, or a Map - but not an ID. Or, even better, in this case, just use the closure in the click handler over the index being iterated over.)

Prefer .textContent over .innerHTML unless you're deliberately setting text content; .textContent is faster, safer, and more semantically appropriate.

Prefer strict equality - loose equality with == and != has a whole bunch of weird coercion rules that neither you nor others who happen to read the code should have to have memorized to understand what's going on. Use === and !== instead.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.