5
\$\begingroup\$

I'm pretty new to JavaScript, and I'm trying to created a text-based game. I've written a few scenarios and based on the player's decision I have different scenarios written out. As of now I have the code written and it works but I feel like I can be more efficient with an if else statement in my js.

Here's what I have so far in terms of player decisions.

 $(document).ready(function(){
        
        $(".sit_1, .sit_2, .sit_3a, .sit_3b, .sit_4a").hide();
        $(".choice_1, .choice_2a, .choice_2b, .choice_3a, .choice_3b, .choice_4a, .choice_4b").hide();
        $(".sit_1").fadeIn(2000);
        $(".choice_1").fadeIn(4000);
        
        
        $(".choice_1").click(function(){
        	$(".sit_1").hide();
        	$(".choice_1").hide();
        	$(".sit_2").fadeIn(4000);
        	$(".choice_2a").fadeIn(8000);
        	$(".choice_2b").fadeIn(8000);
        
        
        	});
        
        
        $(".choice_2a").click(function(){
        	$(".sit_2").hide();
        	$(".choice_2a").hide();
        	$(".choice_2b").hide();
        	$(".sit_3a").fadeIn(4000);
        	
        
        
        
        	});
        
        
        $(".choice_2b").click(function(){
        	$(".sit_2").hide();
        	$(".choice_2a").hide();
        	$(".choice_2b").hide();
        	$(".sit_3b").fadeIn(4000);
        	$(".choice_3a").fadeIn(4000);
        	$(".choice_3b").fadeIn(4000);
        
        
        
        	});
        
        
        $(".choice_3a").click(function(){
        	$(".sit_3b").hide();
        	$(".choice_3a").hide();
        	$(".choice_3b").hide();
        	$(".sit_4a").fadeIn(3000);
        	$(".choice_4a").fadeIn(4000);
        	$(".choice_4b").fadeIn(4000);
        
        
        
        	});
        
        
        });
    <html>
    <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
    <div id="S1">
    
    				<div class="sit_1">
    					You are lost in the woods.
    				</div>
    		
    				<div class="sit_2">
    					You find a cave. Do you go inside or go look for food? <!-- choices are choice_2a and choice_2b -->
    				</div>
    		
    				<div class="sit_3a">
    					You walk into the cave.<!--no more choices for this one -->
    				</div>
    		
    				<div class="sit_3b">
    					You walk away from the cave, to search for food. You find berries. Do you eat them or not? 
    				</div> <!-- choice_3a and choice_3b -->

                    <div class="sit_4a">
					    You eat the berries.
				</div>
        </div>
        
        <div id="a1">

					<input type="button" class="choice_1" value="Continue on...">

					<input type="button" class="choice_2a"value="Go inside">

					<input type="button" class="choice_2b" value="Look for food">

					<input type="button" class="choice_3a" value="Eat the berries">

					<input type="button" class="choice_3b" value="Don't eat the berries">

			</div>


       
        </html>

Not sure how clear this is but I basically want all my situations (sit_) to be hidden along with the player choices (choice_) until the player presses a button or makes a choice and then the old situations along with the old choices will disappear to make way for the new situations and choices.

As I said this code does work but I feel like I can make it more efficient with an if/else statement but I'm not sure how to go about it. Any help would be greatly appreciated.

Also, should I make the choices into an array to make it easier?

\$\endgroup\$
  • 2
    \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB Aug 1 '16 at 15:44
6
\$\begingroup\$

You are falling into a conceptual trap of trying to define javascript behaviors for each element on the page by numbering each and every one with a different id or class name (i.e. choice_*). Any time you see this pattern in your coding, this should be a red flag that there is probably a better approach to your problem (which is why I think you are asking for this review).

With jQuery (or really javascript in general), you should be thinking in terms of what the behavior of an element is supposed to be using classes to identify elements which need to exhibit similar behavior, much in the same way you use classes to define elements that have common CSS stylings. For example, all of your choice buttons fundamentally have the same behavior. That is, they hide other options and expose a new situation. Because this basic behavior is common, there is no reason that you need to duplicate code for each of these buttons.

So the question then becomes how do you generalize your code to have a single onclick handler for all of the choice buttons?

To do this, you have to have an understanding of which buttons expose which situations - something you are now implementing via choice_* class naming.

In jQuery/javascript, there are probably two main strategies to do this. One is to tie the elements together via their hierarchy in the DOM and to traverse the DOM hierarchy to hide/show related elements based on their location relative to each other on the DOM. For example, when hiding/showing elements, you might hide all the siblings in same level of DOM as the one you are trying to show.

The other main approach is to use data attributes to describe how elements should interact with one another.

I will focus on the data attribute approach here as I think this makes the most sense for your use case. You have a potentially complex set of interactions in a "Choose Your Own Adventure" style of game where you ultimately may have multiple story branches each which could lead to different situations via different paths. This would likely be hard to model via HTML hierarchy alone once you move beyond just a trivial proof of concept. Data attributes are also a powerful tool for a budding javascript developer to learn.

I would propose some changes to your HTML to make each "situation" its own self-contained set of HTML elements, like the concept of a page. You could then hide/show each situation (including choices) as a single entity. I also think in this case it makes your HTML much easier to read and maintain.

<html>
    <div class="situation default_situation" data-situation="start">
        <div class="situation_text">
            You are lost in the woods.
        </div>
        <input type="button" class="situation_choice"
            value="Continue on..."
            data-situation-target="found_cave">
    </div>
    <div class="situation" data-situation="found_cave">
        <div class="situation_text">
            You find a cave. Do you go inside or go look for food? 
        </div>
        <input type="button" class="situation_choice"
            value="Go inside"
            data-situation-target="enter_cave">
        <input type="button" class="situation_choice"
            value="Look for food"
            data-situation-target="food_search">
    </div>
    <div class="situation" data-situation="enter_cave">
        <div class="situation_text">
            You walk into the cave.
        </div>
    </div>
    <div class="situation" data-situation="food_search">
        <div class="situation_text">
            You walk away from the cave, to search for food. You find berries. Do you eat them or not? 
        </div>
        <input type="button" class="situation_choice"
            value="Eat the berries"
            data-situation-target="eat_berries">
        <input type="button" class="situation_choice"
            value="Don't eat the berries"
            data-situation-target="ignore_berries">
    </div>
    <div class="situation" data-situation="eat_berries">
        <div class="situation_text">
            You eat the berries.
        </div>
    </div>
    <div class="situation" data-situation="ignore_berries">
        <div class="situation_text">
            You ignore the berries. You are still hungry.
        </div>
    </div>
</html>

Note that we apply a data attribute called situation on the container of each situation "page" to uniquely identify it amongst all situations. We also have data attributes on each option specifying the situation-target that option relates to (i.e. which situation will be shown when the button is clicked).

I have added another class situation_default to show which situation(s) are visible for display on initial page load.

Finally, I have also introduced a class for situation_text here that is not used in the javascript at all, but could perhaps give you a handle for applying CSS styling.

You use these data attributes within your javascipt to implement the logic as to what situation is shown when an option is clicked.

$(document).ready(function() {
    // Create jQuery handle to all situations.
    // Having this outside click handler allows you to access it while
    // preventing you from having to query the DOM every time you want to
    // act on this selection of elements as a whole.
    var $allSituations = $('.situation');

    // Get selector for default situation(s) which will show on page load
    var $defaultSituation = $('.default_situation');

    // Show default situation(s)
    $allSituations.hide();
    $defaultSituation.fadeIn(2000);

    $('.situation_choice').on('click', function() {
        // Find target situation element for this choice.
        // We use the 'situation-target' data attribute for the
        // clicked element, this, within the handler to determine
        // the target.
        var targetName = $(this).data('situation-target');
        var $target = $('.situation[data-situation="' + targetName + '"]')

        // if we find a single target element, we can now show it
        if($target.length === 1) {
            // hide all situations
            $allSituations.hide();
            // show target element
            $target.fadeIn(4000);
        }
    });
});
\$\endgroup\$
  • \$\begingroup\$ brilliant use of data-family attributes. I was going to write one but yours suffice \$\endgroup\$ – Siobhan Aug 1 '16 at 17:06
  • 3
    \$\begingroup\$ This is a pretty didactic answer, especially useful for the OP as he claimed he's a beginner. So I totally agree with your whole presentation. I would just add, for the OP's sake, that all of that can also be done without jQuery, in pure Javascript. \$\endgroup\$ – cFreed Aug 1 '16 at 20:42
2
\$\begingroup\$

Nice job in general but there are few things you could improve on

  • Use of id : your inner div elements have the class attribute . Use the id selector for a specific element rather class as class is used for multiple elements. so this line can be refactored
<div id="S1">

                    <div id="sit_1">
                        You are lost in the woods.
                    </div>

                    <div id="sit_2">
                        You find a cave. Do you go inside or go look for food? <!-- choices are choice_2a and choice_2b -->
                    </div>

                    <div id="sit_3a">
                        You walk into the cave.<!--no more choices for this one -->
                    </div>

                    <div id="sit_3b">
                        You walk away from the cave, to search for food. You find berries. Do you eat them or not? 
                    </div> <!-- choice_3a and choice_3b -->

                    <div id="sit_4a">
                        You eat the berries.
                </div>
        </div>

Your JQuery can be modified to use id e.g

 $("#sit_1, #sit_2, #sit_3a, #sit_3b, #sit_4a").hide();
 $("#choice_1, #choice_2a, #choice_2b, #choice_3a, #choice_3b, #choice_4a, #choice_4b").hide();
  • Tranverse the div Elements: Rather than calling each of the id for instance
    $("#sit_1, #sit_2, #sit_3a, #sit_3b, #sit_4a").hide();

do this
$("#S1").children().hide();

\$\endgroup\$
  • \$\begingroup\$ @striderno9 Note that the last line of code here in this example of using DOM hierarchy to act against a set of related (in this case children elements) as noted in my answer. My answer used data attributes, but I think it wise to understand both approaches as you will need both if you continue to work in jQuery/javascript. \$\endgroup\$ – Mike Brant Aug 1 '16 at 16:52
1
\$\begingroup\$

I especially like the point of @Mike, namely that you should generalize your code. I would suggest taking this idea one step further, by having the situations and the choices in some data structure (e.g. JSON) and having only a single div, where to render the text and the corresponding choices (as opposed to one div per situation).

To be more concrete, I am thinking about something like this:

HTML:

<div>

<div id="gametext" ></div>
<div id="buttons"></div>

</div>

JS:

var gameMap = {
    "sit1":  {
                "text":  "You are lost in the woods.",
                "choices": {
                                   "continue": "sit2"
                  }
        },
       "sit2":  {
                 "text": "You find a cave. Do you go inside or go look for food?",
                 "choices": {
                      "go inside": "sit3", "look for food": "sit4"
                 }
         },
       "sit3":  {
                 "text": "You walk into the cave.",
                 "choices": {}
         },
       "sit4":  {
                 "text": "You walk away from the cave, to search for food. You findfind berries. Do you eat them or not?",
                 "choices": {}
         }
}

var situationId = "sit1";

function showSituation() {
       var situation = gameMap[situationId];
       var choices = situation.choices;
       var text = situation. text;
       $("#gametext").text(text);



      $("#buttons").empty();

       for (var choice in choices) {
               if (choices.hasOwnProperty(choice)) {
                        var $button = $('<input type="button" value="' + choice  + '"/>');
                        $button.click({"nextsit": choices[choice]}, function(evt) {
                              situationId = evt.data.nextsit;
                              showSituation();
                    });
                    $("#buttons").append($button);
            }
    }

}

showSituation();

Discussion: first of all, please note that the above code is rather a proof of concept and not a full solution. Besides the missing effects (which you can add if you choose to extend this example ;) ), it can be improved in many ways (e.g. not putting game state and functions into the global namespace, but rather enclose them in an object).

That said, let me point out the advantages of this solution:

  • it decouples the event handling logic from the game structure (similar to the answer of Mike)
  • it also disconnects the view from the game structure.

Why is this last point advantageous? Because you can easily add more situations, without the need to copy-paste the gui elements (wrapping divs). Besides that, it is also possible to quickly change the gui implementation. (E.g., if you decide to have divs instead of buttons, you do not have to change every one of them.)

I created a fiddle, where you can try the concept. (Unluckily, the built-in fiddle of SO is not available from tablets...)

\$\endgroup\$
  • \$\begingroup\$ Good thoughts on better decoupling application logic from view, although with this particular implementation example, you actually start putting part of the view into the javascript when you hard code input element for example. It is interesting for OP to note that this sort of approach is also invaluable when you are talking about having a more modern web-based application where you are going to populate content via AJAX. In that sort of case you might be doing something very similar to what is shown here, only getting your data via a web call vs. having it configured in javascript. \$\endgroup\$ – Mike Brant Aug 2 '16 at 14:07

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.