onclick=function() not working for me.

Reaga · 02-01-2013, 12:21 AM

I'm not sure what's wrong, it's my first html file, but it's for a class. I had to create a Beat-The-Buzzer game with 100 buttons, rules were up to me, entirely in notepad2. I thought I'd finished the basic version, but the button to begin the game does nothing.

Code:

<html>
<head>
  <title>Beat the Buttons</title>
  <script language="JavaScript">
  var points = 0;
  var missCount = 0;
  var choice = 0;
  var timer1;
  function GameOver ();
  {
    window.alert('You scored' + points + '. Press Start again to try and beat your score.');
    points = 0;
    missCount = 0;
  }
   function miss ()
  {
    missCount++;
    points = points - 5;
    if (missCount == 5)
    {
     GameOver();
    }
    else
    {
      colorChange();
    }
  }
  function colorChange ()
  {
    choice = Math.floor(100*Math.random()+1);
    var currentButton = document.getElementById(choice);
    var colorChoice = Math.floor(4*Math.random()+1);
    switch (colorChoice)
    {
      case 1:
        currentButton.style.color = "Red";
        timer1 = window.setTimeout ("miss()", 4000);
        break;
      case 2:
        currentButton.style.color = "Blue";
        timer1 = window.setTimeout ("miss()", 3000);
        break;
      case 3:
        currentButton.style.color = "Green";
        timer1 = window.setTimeout ("miss()", 2000);
        break;
      default:
        currentButton.style.color = "Black";
        timer1 = window.setTimeout ("colorChange()", 5000);
    }
  }

  function point ()
  {
    var currentButton = document.getElementById(choice);
    if (currentButton.style.color == "Red")
    {
      window.clearTimeout (timer1);
      points++;
    }
    else if (currentButton.style.color == "Blue")
    {
      window.clearTimeout (timer1);
      points = points + 3;
    }
    else if (currentButton.style.color == "Green")
    {
      window.clearTimeout (timer1);
      points = points + 5;
    }
    else
    {
      window.clearTimeout (timer1);
      miss();
    }
  }
  </script>
</head>
<body bgcolor="#F8F8FF" onload="window.alert('Score as high as possible.\n1. Missing loses you 5 points\n2. Missing 5 means Game Over\n3. Hitting Black counts as a Miss\n4. Red is 1\n5. Blue is 3\n6. Green is 5')">
<center><p><br />
<h2>Beat the Buttons!</h2>
<br />
<table border="1" cellspacing="5" cellpadding="5">
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=1 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=2 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=3 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=4 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=5 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=6 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=7 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=8 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=9 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=10 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=11 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=12 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=13 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=14 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=15 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=16 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=17 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=18 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=19 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=20 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=21 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=22 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=23 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=24 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=25 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=26 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=27 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=28 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=29 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=30 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=31 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=32 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=33 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=34 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=35 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=36 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=37 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=38 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=39 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=40 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=41 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=42 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=43 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=44 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=45 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=46 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=47 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=48 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=49 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=50 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=51 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=52 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=53 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=54 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=55 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=56 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=57 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=58 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=59 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=60 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=61 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=62 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=63 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=64 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=65 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=66 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=67 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=68 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=69 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=70 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=71 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=72 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=73 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=74 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=75 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=76 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=77 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=78 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=79 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=80 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=81 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=82 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=83 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=84 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=85 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=86 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=87 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=88 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=89 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=90 onclick="point()"></form></th>
  </tr>
  <tr bgcolor="778899">
    <th width=10%><form><input type="button" id=91 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=92 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=93 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=94 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=95 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=96 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=97 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=98 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=99 onclick="point()"></form></th>
    <th width=10%><form><input type="button" id=100 onclick="point()"></form></th>
  </tr>
</table>
<form><input type="text" size="10" value=points></form>
<form><input type="text" size="10" value=missCount></form>
<form><input type="button" id="Start" value="Start" onclick="colorChange()"</form>
</body>
</html>

Can anyone tell me what went wrong?

And I've heard I could create the table in a for loop to shorten code, but my one attempt there did nothing. So I sucked it up for 100 lines for buttons.

AndrewGSW · 02-01-2013, 12:24 AM

The first thing that jumps out is that this:

Code:

<script language="JavaScript">

indicates you are working with very old code!!!

It should be:

Code:

<script type="text/javascript">

The semi-colon at the end of this line:

Code:

function GameOver ();

shouldn't be there and i wouldn't have a space between the function name and the brackets (even though it works).

These two things jumped out at me immediately

so I haven't looked any further.

Ugh!! You are using bgcolor and the center tag. Both are OBSOLETE - throw away that script! and get a modern one.

Old Pedant · 02-01-2013, 12:35 AM

You *REALLY* need to learn to use a debugger. Really. Now.

I would recommend using CHROME as your browser. It's the easiest debugger to use.

Just bring up your page in CHROME and then hit the F12 key.

Click on the CONSOLE tab and voila! There is your FIRST error:

Code:

  function GameOver ();

Oops...what is that semicolon doing there! Kill it quick.

But your colorChange() *IS* working. The problem is that, since your buttons have no *VALUES*, the color has nothing to affect! Put an "X" (say) as the value of each button and the "X" will change color.

Or try changing the background color.

And why did you choose black for the default? That means that if black comes up the user gets no clue at all, right?

Old Pedant · 02-01-2013, 12:49 AM

Try using backgroundColor instead of color, that worked. Though I changed your "black" to "yellow".

But you can't replay the game by hitting start again, because it does not first clear the board.

Reaga · 02-01-2013, 12:52 AM

ooh, I did not think of resetting the buttons. Crap. As for the old code, my professor hasn't actually taught. He rambles but doesn't show any implementation.

Old Pedant · 02-01-2013, 01:00 AM

Also, your code can (and did!) pick the same square and same color twice in a row! So the user can't see any change.

And you have some problem with the timeout on a new game, as things seem to speed up a lot.

Oh...and the browser converts style.backgroundColor="Blue" to "blue". Chrome does, at least. I vaguely recall the FireFox converts it to RGB. So you really should *NOT* be testing the color of the cell. Instead, just remember it in a variable.

Also, IDs that start with a digit are frowned on. There are circumstances where they won't work.

Reaga · 02-01-2013, 01:21 AM

i already thought of a way to take care of resetting the colors, I'll have them start out white and reset to white after a click or miss.

Old Pedant · 02-01-2013, 02:18 AM

Here. You really got pretty far with that. So I don't feel bad showing you a more modern way.

Code:

<!DOCTYPE html>
<html>
<head>
<title>Speed game</title>
<style type="text/css">
#theGame td {
    height: 25px;
    width: 25px;
    background-color: #eeeeee;
}
#userPoints {
    font-size: x-large;
    color : green;
}
#userMisses {
    font-size: x-large;
    color : red;
}
#message {
    font-size: xx-large;
    font-weight; bold;
    color : red;
}
</style>
</head>
<body>
<div align="center">
    <h2>Speed Game</h2>
    <table id="theGame" border="1" cellPadding="5">
    </table>
    <form id="theForm">
        <input type="button" name="start" value="Click to start"/>
    </form>
    Points scored: <span id="userPoints">0</span><br/>
    Misses so far: <span id="userMisses">0</span><br/>
    <span id="message"></span>
</div>

<script type="text/javascript">
(
  function() /* anonymous master function */
  {
      var points = 0, misses = 0, choice, choicePoints = 0, tick;
      var cells = [ ];

      var tbl = document.getElementById("theGame");
      for ( var r = 0; r < 10; ++r )
      {
          var row = tbl.insertRow(-1); // create a <tr>
          for ( var c = 0; c < 10; ++c )
          {
              var cell = row.insertCell(-1); // create a <td>
              cell.innerHTML = "&nbsp;"; // blank content
              cell.onclick = score; // attach the onclick handler
              cells.push(cell); // add this to the array of cells
          }   
      }
      // just to avoid errors when cell is clicked on prematurely:
      choice = cells[0];

      // attach handler to start button:
      var form = document.getElementById("theForm");
      form.start.onclick = pickOne;

      // cleans up the prior choice and updates the visible score
      // okay to call more than once (as when score() calls miss(), for example)
      function clearChoice( )
      {
          choice.style.backgroundColor = "#eeeeee";
          clearTimeout(tick);
          document.getElementById("userPoints").innerHTML = points;
          document.getElementById("userMisses").innerHTML = misses;
      }
      
      // pick a new cell to give color to
      function pickOne( )
      {
          clearChoice( );
          var which  = Math.floor( 100 * Math.random() );
          choice = cells[which];
          var color, wait;
          switch ( Math.floor( 4 * Math.random() ) )
          {
              case 0 : 
                  color = "red"; wait = 4000; choicePoints = 1; break;
              case 1 : 
                  color = "blue"; wait = 3000; choicePoints = 3; break;
              case 2 : 
                  color = "green"; wait = 2000; choicePoints = 5; break;
              default : 
                  color = "black"; wait = 5000; choicePoints = -5; break;
          }
          choice.style.backgroundColor = color;
          if ( choicePoints > 0 ) { tick = setTimeout( miss, wait ); }
          else                    { tick = setTimeout( pickOne, wait ); }
      }          

      // called when timer times out or (via score()) when wrong cell clicked on
      function miss( )
      {
          points -= 5;
          ++misses;
          clearChoice( );
          if ( misses > 4 ) { gameEnd( ); }
          else              { pickOne( ); }
      }

      // called by a click in any cell
      function score( )
      {
          if ( choicePoints < 0 ) { miss( ); return; }

          clearChoice( );
          if ( this == choice ) 
          {
               points += choicePoints;
               pickOne( );
          } else {
               miss( );
          }
      }
      function gameEnd( )
      {
          document.getElementById("message").innerHTML = "GAME OVER";
          tbl.style.visibility = "hidden";
          form.start.style.visibility = "hidden";
      }
                     
  }
)(); // self-invoke the anonymous master function
</script>
</body>
</html>

Old Pedant · 02-01-2013, 02:22 AM

Quote:

Originally Posted by Reaga

i already thought of a way to take care of resetting the colors, I'll have them start out white and reset to white after a click or miss.

transparent is even better than white.

But a better answer is as I gave you: Dump the unneeded <input>s and just add the onclick's to the <td> cells themselves.

Way back in MSIE 4 days you couldn't do that. If your book tells you that you must, that you can't add onclick to almost any element, then your book is horribly out of date.

Reaga · 02-01-2013, 03:13 AM

I wish there was a book. He has not listed a book for his course.

Old Pedant · 02-01-2013, 03:50 AM

Quote:

Originally Posted by Reaga

I wish there was a book. He has not listed a book for his course.

Well, that may not be all bad. So many of the JS books are pure crap.

There is a fairly well known author who published one of the "popular" ones (popular with ignorant instructors!) in 2011 and it is *TRASH*. Worse, it sells new for around $90! You can't help but feel that every student who got suckered into buying that nonsense should demand their money back from the school...not only for the book but for the entire course...because *CLEARLY* any instructor who chooses that book knows nothing about real world JavaScript programming.

There is a *HOPE* that your instructor may avoid some of the pitfalls.

Here are a few:

-- alert() and prompt() and document.write() are obsolete. You might use them while debugging but never in final code.

-- putting <script> in the <head> is usually not the best idea. Among other things, it means you are pretty much stuck with doing set up code via window.onload (or <body onload="...">). It's not "wrong" to do that (goodness knows I wrote enough pages that way!), but it often makes tons more sense to put the JS after the HTML is going to affect.

-- putting JavaScript event handlers inline (e.g., <input onclick="...">) is considered...well, not wrong, but stylistically bad.

There are many others. Felgall can educate you on. Read his site.

Old Pedant · 02-01-2013, 03:55 AM

An improvement in my code, since it's still POSSIBLE to choose the same cell and color twice in a row (not likely, but possible):

Change this:

Code:

          var which  = Math.floor( 100 * Math.random() );
          choice = cells[which];

TO this:

Code:

         do {
              var which  = Math.floor( 100 * Math.random() );
         } while ( choice == cells[which] ); // repeat if got same cell
         choice = cells[which];

Reaga · 02-05-2013, 06:37 PM

Sorry Old Pendant, but I am attempting to learn so I have not copied your code. I'm mostly looking for why something doesn't work, and not how to fix it. On the plus side, I've made progress.

Code:

<!DOCTYPE html>
<html>
<head>
  <title>Beat the Buttons</title>
  <link rel="StyleSheet" href="example.css" type="text/css">
  <script type="text/javascript">
  var points = 0;
  var missCount = 0;
  var choice = 0;
  var timer1;
  function GameOver()
  {
    window.alert('You scored ' + points + '. Press Start again to try and beat your score.');
    points = 0;
    missCount = 0;
  }
  function miss()
  {
    missCount++;
    points = points - 5;
    var currentButton = getElementById(choice);
    currentButton.style.backgroundColor = 'white';
    if (missCount >= 5)
    {
     GameOver();
    }
    else
    {
      colorChange();
    }
  }
  function colorChange()
  {
    choice = Math.floor(100*Math.random()+1);
    var colorChoice = Math.floor(4*Math.random()+1);
    var currentButton = document.getElementById(choice);
    switch (colorChoice)
    {
      case 1:
        currentButton.style.backgroundColor = 'red';
        timer1 = window.setTimeout("miss()", 4000);
        break;
      case 2:
        currentButton.style.backgroundColor = 'blue';
        timer1 = window.setTimeout("miss()", 3000);
        break;
      case 3:
        currentButton.style.backgroundColor = 'green';
        timer1 = window.setTimeout("miss()", 2000);
        break;
      case 4:
        currentButton.backgroundColor = 'black';
        timer1 = window.setTimeout("colorChange()", 5000);
    }
  }
  function point()
  {
    window.clearTimeout (timer1);
    var currentButton = document.getElementById(choice);
    if (currentButton.style.backgroundColor == 'red')
    {
      points++;
    }
    else if (currentButton.style.backgroundColor == 'blue')
    {
      points = points + 3;
    }
    else if (currentButton.style.backgroundColor == 'green')
    {
      points = points + 5;
    }
    else
    {
      miss();
    }
    currentButton.style.backgroundColor = 'white';
    colorChange();
  }
  </script>
</head>
<body bgcolor="#F8F8FF" onload="window.alert('Score as high as possible.\n1. Missing loses you 5 points\n2. Missing 5 means Game Over\n3. Hitting Black counts as a Miss\n4. Red is 1\n5. Blue is 3\n6. Green is 5')">
<center><p><br />
<h2>Beat the Buttons!</h2>
<br />
<table border="1" cellspacing="5" cellpadding="5">
<script>
for (var y=0; y<10; y++)
  {
  document.write('<tr bgcolor="778899">')
  for (var x=1; x<=10; x++)
    {
      document.write('<td width=10%><form><input type="button" value="X" id=' + (y*10+x) + ' this.style.backgroundColor=\'white\' onclick="point()"></form></td>')
    }
  document.write('</tr>')
  }
</script>
</table>
<form><input type="text" id="score" size="10"></form>
<form><input type="text" id="missed" size="10"></form>
<form><input type="button" id="Start" value="Start" onclick='colorChange()'></form>
</body>
</html>

The following was found out through inserting window.alert messages:
1. The miss() function is not running after I set the currentButton variable. Note, that when I removed that variable from the functions and made it global, no functions worked.

2. colorChange() is hitting the black case, but not changing the button. Upon failing to change the button, the whole process stops.

So these are the key issues, and I need to know why they're happening so I can fix them myself. I am aware that my attempt to set white as a default color has failed, likely due to quotation placement, and that the two textboxes are blank. They are meant to display the score and number of misses, but I'll fix them later.

Reaga · 02-05-2013, 06:48 PM

Derp, I found out why black fails. I forgot to add .style. Though the process still fails to continue once a button is made black.

felgall · 02-05-2013, 09:32 PM

You really ought to get rid of the document.write statements and move the script to the bottom of the page. The code you currently have looks more appropriate for Netscape 4 than it does for more modern browsers. That you are using such an antiquated approach makes it more difficult for others to help you as it is easy to forget all the restrictions that apply to code that became obsolete over ten years ago.