0

I just started a CS class at school, so please excuse my total lack of basic knowledge. This JS only works if I put it after the HTML code, not if I put it in the headtag. Shouldn't the window.onload take care of that? Can someone please explain what's wrong? Thanks in advance!

<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<title>Document</title>
<style>
    #field {
        width: 600;
        height: 600;
        background-color: black;
        position: relative;
    }

    #player {
        width: 50;
        height: 50;
        background-color: red;
        position: absolute;
        left: 0px;
        top: 0px;
    }
</style>
<script>
    var player = document.getElementById("player");
    var playerLeft = 0;
    var PlayerTop = 0;

    function move(e) {
        if (e.keyCode == 68) {
            playerLeft += 10
            player.style.left = playerLeft + "px";
        }
        if (e.keyCode == 65) {
            playerLeft -= 10
            player.style.left = playerLeft + "px";
        }
        if (e.keyCode == 87) {
            PlayerTop -= 10
            player.style.top = PlayerTop + "px";
        }
        if (e.keyCode == 83) {
            PlayerTop += 10
            player.style.top = PlayerTop + "px";
        }
    }


    function loadEvents() {
        document.onkeydown = move;
    }
    window.onload = loadEvents;
</script>
</head>
<body>

<div id="field">
    <div id="player">
    </div>
</div>

Anna Leo
  • 37
  • 3
  • Possible duplicate of [Why window.onload works and onload="" does not work](http://stackoverflow.com/questions/27140026/why-window-onload-works-and-onload-does-not-work) – Tim B Nov 06 '16 at 10:54
  • 5
    Your assignment of the player needs to ALSO be inside the onload: `var player; function loadEvents() { player = document.getElementById("player"); document.onkeydown=move; }` var – mplungjan Nov 06 '16 at 10:57
  • @TimB - wrong duplicate! – mplungjan Nov 06 '16 at 11:00

2 Answers2

6

The problem is that your want to get an element which doesn't exist yet

var player = document.getElementById("player");

Put this line in the loadEvents() function which is called when the window is loaded.

Note: Avoid errors (if #player element doesn't exist) adding if (player) { ... }

<script>
  var player = null;
  var playerLeft = 0;
  var playerTop = 0;

  function move(e) {
    if (e.keyCode == 68) {
        playerLeft += 10
        player.style.left = playerLeft + "px";
    }
    if (e.keyCode == 65) {
        playerLeft -= 10
        player.style.left = playerLeft + "px";
    }
    if (e.keyCode == 87) {
        playerTop -= 10
        player.style.top = playerTop + "px";
    }
    if (e.keyCode == 83) {
        playerTop += 10
        player.style.top = playerTop + "px";
    }
  }


  function loadEvents() {
    player = document.getElementById("player");
    if (player) {
      document.onkeydown = move;
    }
  }
  window.onload = loadEvents;
</script>

Edit

For @tmslnz

var player = null is somewhat redundant, since getElementById returns null if no element is found.

From the ECMAScript2015 spec

4.3.10 undefined value

primitive value used when a variable has not been assigned a value

4.3.12 null value

primitive value that represents the intentional absence of any object value

See this thread and this answer

Community
  • 1
  • 1
Robiseb
  • 1,576
  • 2
  • 14
  • 17
  • In addition, `strict mode` would invalidate this. – roberrrt-s Nov 06 '16 at 11:01
  • @mplungjan Yes you right, I just pasted the code too quickly without reading myselft after. Thanks guys – Robiseb Nov 06 '16 at 11:07
  • I am hair-splitting, but for it's own sake: `var player = null` is somewhat redundant, since `getElementById` returns `null` if no element is found. – tmslnz Nov 06 '16 at 11:25
0

This runs before the document is loaded. AKA before the browser has any clue of your markup.

var player = document.getElementById("player");
…

Any time you are querying the DOM (think of it as the HTML you wrote as seen by the browser), you need to wait for it be ready.

Your script would probably be OK if you placed it at the very end of your document, before the closing </body> tag, because by then the browser would have had a chance to spot #player.

What you need to do is find a way to structure your scripts so that they don't try to access the DOM before it's ready.

The easy way is to put everything inside onload in inside $(function(){ … }) if you are using jQuery.

Alternatively if you prefer a bit more control, you can use something like an init function, which you use to "start the engine after the tank is filled" (to use a crappy metaphor…)

For example:

var player;
var playerLeft = 0;
var playerTop = 0;

function init () {
    player = document.getElementById("player");
    loadEvents();
}

function move(e) {
    if (e.keyCode == 68) {
        playerLeft += 10
        player.style.left = playerLeft + "px";
    }
    if (e.keyCode == 65) {
        playerLeft -= 10
        player.style.left = playerLeft + "px";
    }
    if (e.keyCode == 87) {
        playerTop -= 10
        player.style.top = playerTop + "px";
    }
    if (e.keyCode == 83) {
        playerTop += 10
        player.style.top = playerTop + "px";
    }
}

function loadEvents() {
    document.onkeydown = move;
}

window.onload = init;
tmslnz
  • 1,801
  • 2
  • 15
  • 24