More SVG questions (and Javascript) - Interactivity [moving a rectangle]

The following code is supposed to make the rectangle move when pressing the WASD keys on the keyboard. It doesn't work. The rectangle does not move, but no errors are generated. How can I make this work?

<?xml version="1.0" encoding="UTF-8" standalone="no"?>

<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN"
"http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">

<svg width="90%" height="90%" version="1.1"
xmlns="http://www.w3.org/2000/svg">

<script type="text/javascript">
    <![CDATA[
    var x;
    var y;
    function move()
    {
        x = new Number(document.getElementById("player").x);
        y = new Number(document.getElementById("player").y);
        switch (event.keyCode)
        {
            case 119:
            y--;
            document.getElementById("player").y=y;
            break;
            case 115:
            y++;
            document.getElementById("player").y=y;
            br开发者_如何学Ceak;
            case 97:
            x--;
            document.getElementById("player").x=x;
            break;
            case 100:
            x++;
            document.getElementById("player").x=x;
            break;
            default:
        }
    }
    document.documentElement.addEventListener("keypress", move, true);
    ]]>
</script>

<rect x="0" y="0" height="100%" width="100%" style="stroke-width:5; stroke:black; fill:white"></rect>
<rect x="250" y="250" id="player" height="50" width="50" style="stroke-width:1; stroke:red; fill:red"></rect>

</svg>

Your code above does not work because the x and y properties of an SVGRectElement are SVGAnimatedLength objects, representing values that can change over time.

The simplest changes to make your code 'work' are:

1. Change
   x = new Number(document.getElementById("player").x);
   to
   x = new Number(document.getElementById("player").x.baseVal.value);
   (and similarly for y).

2. Change
   document.getElementById("player").x=x;
   to
   document.getElementById("player").x.baseVal.value=x;
   (and similarly for y).

This will cause your code to work as desired. However, your code could generally be improved per my answer to your previous question.

Here is how I would write it (if you really only want to move one unit at a time):

<?xml version="1.0" encoding="UTF-8" standalone="no"?>
<svg width="90%" height="90%" version="1.1"
 xmlns="http://www.w3.org/2000/svg">
<style>
    #player { stroke-width:1; stroke:red; fill:red }
</style>
<rect x="250" y="250" id="player" height="50" width="50" />
<script type="text/javascript"><![CDATA[
  var KEY = { w:87, a:65, s:83, d:68 };
  var rect = document.getElementById('player');
  var x = rect.getAttribute('x')*1,
      y = rect.getAttribute('y')*1;
  document.documentElement.addEventListener("keydown", function(evt){
    switch (evt.keyCode){
      case KEY.w: rect.y.baseVal.value = --y; break;
      case KEY.s: rect.y.baseVal.value = ++y; break;
      case KEY.a: rect.x.baseVal.value = --x; break;
      case KEY.d: rect.x.baseVal.value = ++x; break;
    }
  }, false);
]]></script>
</svg>

Note that I've moved the <rect> above the <script> block so that a) the user sees the rectangle before the script starts to load, but more importantly b) so that the rectangle exists before calling getElementById(). If your script block occurs before your elements have been defined, you must instead set up your code references in response to an onload or similar event handler.