5
\$\begingroup\$

I'm working on a little game where you press a button and a new image appears. There's a glitch every time you ask a new image to display and it occurred to me that I should preload the necessary images. However, I'm not sure how to do this efficiently as I'm not using a variable to display the images. It's also been pointed out to me that I should look into storing reference for the elements, but I don't know how to go about doing so. This is what I'm working on. Any suggestions?

HTML:

<body onLoad="setup()">
<div id="wrapper">
        <div id="jajo"></div><!--this is where jajo will be displayed-->
        <div id="directions"></div><!--directions for how to interact with jajo-->
    </div><!--wrapper-->
</body>

JavaScript:

// Calls the loadJajo function and passes the image URL
// Initiates directionSlide function
function setup() {
loadJajo('jajo.png');
elem = document.getElementById('directions');
directionSlide();
}

//Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
var main = document.getElementById('jajo'); // Creates an variable to represent the "main" division
var defaultJajo = document.createElement('img'); // Creates a new image object (default Jajo image)
defaultJajo.src = jajoSRC; // adds the source file name to the defaultJajo image object
main.appendChild(defaultJajo); //puts the defaultJajo object inside the "main" division
}

// Listen for key pressed events.
document.onkeydown = function(event) {
var keyPress = String.fromCharCode(event.keyCode); // Assigns value of key pressed to variable.

if(keyPress == "W") { // If 'W' is pressed, display Jajo waving.
    document.getElementById("jajo").innerHTML= "<img alt='car' src='jajo_wave.png'>";
    document.onkeyup = function(event) { // If 'W' is released, display default Jajo.
        document.getElementById("jajo").innerHTML= "<img alt='Jajo' src='jajo.png'>";
    }
} else if(keyPress == "A") { // If 'A' is pressed, display Jajo winking.
    document.getElementById("jajo").innerHTML= "<img alt='car' src='jajo_wink.png'>";
    document.onkeyup = function(event) { // If 'A' is released, display default Jajo.
        document.getElementById("jajo").innerHTML= "<img alt='Jajo' src='jajo.png'>";
    }
} else if(keyPress == "S") { // If 'S' is pressed, display transparent Jajo.
    document.getElementById("jajo").innerHTML= "<img alt='car' src='jajo_invisible.png'>";
    document.onkeyup = function(event) { // If 'S' is released, display default Jajo.
        document.getElementById("jajo").innerHTML= "<img alt='Jajo' src='jajo.png'>";
    }
} else if(keyPress == "D") { // If 'D' is pressed, display purple Jajo.
    document.getElementById("jajo").innerHTML= "<img alt='car' src='jajo_purple.png'>";
    document.onkeyup = function(event) { // If 'D' is released, display default Jajo.
        document.getElementById("jajo").innerHTML= "<img alt='Jajo' src='jajo.png'>";
    }
} else if(keyPress == "E") { // If 'E' is pressed, display Jajo eating a carrot.
    document.getElementById("jajo").innerHTML= "<img alt='car' src='jajo_carrot.png'>";
    document.onkeyup = function(event) { // If 'E' is released, display default Jajo.
        document.getElementById("jajo").innerHTML= "<img alt='Jajo' src='jajo.png'>";
    }
}
}

var elem;
var i = 0; // Counter variable.
// Array with directions for interacting with Jajo.
var directionArray = ["This is Jajo, your new pet monster!",
             "Jajo wants to say 'Hi!'<br>Press and hold 'W'",
             "Jajo has some special skills.<br>Press and hold 'D' to see one!",
             "Jajo is hungry for a healthy snack.<br>Press and hold 'E'",
             "Jajo wants to show you his secret power.<br>Press and hold 'S'",
             "That secret is just between you and Jajo!<br>Press and hold 'A'"];

// Transitions one direction to the next.
function nextDirection() {
i++; // Continuously add 1 to i.
elem.style.opacity = 0; // Directions opacity at 0%.
if(i > (directionArray.length - 1)) { // Resets counter to 0 when it reaches the end of the array.
    i = 0;
}
setTimeout(directionSlide,1000); // Set time delay for transition between directions.
}

// Displays direction one at a time.
// http://www.developphp.com/view.php?tid=1380
function directionSlide() {
elem.innerHTML = directionArray[i]; // Displays direction based on position of counter variable.
elem.style.opacity = 1; // Direction opacity at 100%.
setTimeout(nextDirection,5000); // Set time delay for display of directions.
}
\$\endgroup\$
1
  • \$\begingroup\$ Do all the images need to be individual? you'd most likely be better of using a sprite sheet loading that. stackoverflow.com/questions/8050152/… \$\endgroup\$
    – atmd
    Commented Mar 24, 2014 at 11:06

1 Answer 1

Reset to default
3
\$\begingroup\$

From a once over:

  • Please indent your functions, bad:

    function setup() {
loadJajo('jajo.png');
elem = document.getElementById('directions');
directionSlide();
}

    better:

    function setup() {
    loadJajo('jajo.png');
    elem = document.getElementById('directions');
    directionSlide();
}
  • Consider having only 1 comma separated var statement on the top of your code

  • Lighten up on the comments, you are overdoing it, this:

    //Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
var main = document.getElementById('jajo'); // Creates an variable to represent the "main" division
var defaultJajo = document.createElement('img'); // Creates a new image object (default Jajo image)
defaultJajo.src = jajoSRC; // adds the source file name to the defaultJajo image object
main.appendChild(defaultJajo); //puts the defaultJajo object inside the "main" division
}

    could be this:

    //Creates a new image object (Jajo) and writes it to the page.
function loadJajo(jajoSRC) {
  var main = document.getElementById('jajo'), 
      defaultJajo = document.createElement('img'); 
  defaultJajo.src = jajoSRC; 
  main.appendChild(defaultJajo); 
}

  • You are actually storing a reference to an element here:

    elem = document.getElementById('directions');

    However, you should call elem differently, probably directions. Also, you should consider caching document.getElementById('jajo') since you use it a ton of times.

  • There is a clear mapping in your code between key presses and images, you could write your keydown handler like this:

    var keyImageMap = 
{
  'W' : 'jajo_wave.png',      // If 'W' is released, display default Jajo.
  'A' : 'jajo_wink.png',      // If 'A' is pressed, display Jajo winking.
  'S' : 'jajo_invisible.png', // If 'S' is pressed, display transparent Jajo.
  'D' : 'jajo_purple.png',    // If 'D' is pressed, display purple Jajo.
  'E' : 'jajo_carrot.png'     // If 'E' is pressed, display Jajo eating a carrot.
}

// Listen for key pressed events.
document.onkeydown = function(event) {
  // Assigns value of key pressed to variable.
  var keyPress = String.fromCharCode(event.keyCode); 

  if( keyImageMap[ keyPress ] )
  {
    document.getElementById('jajo').innerHTML = '<img alt="car" src="' + keyImageMap[ keyPress ] + '">';
  }
}
  • In the previous sample code you should of course have used a reference to jajo instead of calling document.getElementById. Also you should have deep thoughts on rewriting that image tag every time, perhaps it would be better to have a reference to the image and then keep changing src
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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