Beginners JavaScript

Question

I'm new to JavaScript. I've worked my work through Learning JavaScript (o'reilly) but am just trying to make my first JavaScript.

I thought it best to work on something I'm interested in and as it turned out is fairly complicated.

I'm basically trying to simulate (eventually) a situation in Space Hulk (Boardgame) where a Genestealer has 12 steps between him and the Space Marine. On the first step its 6 on either dice to kill the Genestealer, and after that 5 or 6 to kill. The gun jams if the number on the dice is the same.

I'm just trying to emulate the first step here. I think the problem is with jamCheck.

Basically this outputs as always true, even if I change it to != it always shows gun jammed.

I wondered if the variable needed to be passed into another local variable, but it works for killCheck without having to do this. (and I tried it, although I may be doing it wrong)

It is completely possible there is something really simple wrong here.

I hope you can help, or point me in the right direction.

Many thanks!

        <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
        <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">
        <head>
        <title>SH</title>
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
        <script type="text/javascript">
      //<![CDATA[

    function diceRoll1() {
       iValue = Math.random(); // random number between 0 and 1
       iValue *= 6; // multiply by 6 to move the decimal
       iValue = Math.floor(iValue)+1; // round to nearest integer. +1 to 1-6.
       var roll1 = iValue;
       document.getElementById('result1').innerHTML = 'Dice roll 1 : ' + roll1;
       killCheck (roll1);
       jamCheck (roll1);
       return;
    }

    function diceRoll2() {
       iValue = Math.random(); // random number between 0 and 1
       iValue *= 6; // multiply by 6 to move the decimal
       iValue = Math.floor(iValue)+1; // round to nearest integer. +1 to 1-6.
       var roll2 = iValue;
       document.getElementById('result2').innerHTML = 'Dice roll 2 : ' + roll2;
       killCheck (roll2);
       jamCheck (roll2);
       return;  
    }

    function killCheck(roll1,roll2){
        if (roll1==6 || roll2==6)
        {
        document.getElementById('kill').innerHTML = 'GS KILLED';
        }
        return;
    }

    function jamCheck(roll1,roll2){
        if (roll1 == roll2)
        {
        document.getElementById('jam').innerHTML = 'GUN JAMMED';
        }
        return;
    }   

    //]]>
    </script>
    </head>
    <body onload="diceRoll1();diceRoll2();killCheck();jamCheck();">
      <p id="result1">Dice roll 1</p>
      <p id="result2">Dice roll 2</p>
      <p id="kill">GS ALIVE</p>
      <p id="jam">GUN FINE</p>

    </body>
    </html>

EDIT: I eventually got there with a lot of help from a friend; here is the current code:

...
function getDiceValue() {
var diceValue = Math.random(); 
diceValue *= 6;
diceValue = Math.floor(diceValue) + 1; 
return diceValue;
}

function killCheck(roll1, roll2) {
if (roll1 === 6 || roll2 === 6) {
document.getElementById('kill').innerHTML = 'GS KILLED';
}
return;
}

function jamCheck(roll1, roll2){
if (roll1 === roll2) {
document.getElementById('jam').innerHTML = 'GUN JAMMED';
}
return;
}

function rollDice() {
var roll1 = getDiceValue(),
roll2 = getDiceValue();
document.getElementById('result1').innerHTML = 'Dice roll 1 : ' + roll1;
document.getElementById('result2').innerHTML = 'Dice roll 2 : ' + roll2;
killCheck (roll1, roll2);
jamCheck (roll1, roll2);
}
//]]>
...
<body onload="rollDice();">

just an unrelated note, don't use empty-returns at the end of a functions (they will return nothing anyway). The only reason why you would ever use an empty-return is to stop the function (similar to a break on a loop) — ajax333221
– ajax333221, Commented Apr 29, 2012 at 16:40
Here's a handy die-roll function if you want to clean things up a bit: function rollDie(min, max) { return Math.floor(Math.random() * max) + min; } 6-sided example: rollDie(1,6); — pdizz
– pdizz, Commented Apr 29, 2012 at 16:49
@ajax333221 I read that would return it to killCheck and JamCheck, is it not needed at all? — snoopyj
– snoopyj, Commented Apr 29, 2012 at 20:40
@pdizz thanks I'll give that a go, I thought there must be a way to cut that down! — snoopyj
– snoopyj, Commented Apr 29, 2012 at 20:41
@ajax333221 Just re read Dr.Dredel answer and saw he mentioned I need to return roll1 etc. — snoopyj
– snoopyj, Commented Apr 29, 2012 at 21:01

Yevgeny Simkin · Accepted Answer · 2012-04-29 16:53:05Z

Math.floor() rounds down (think about the name... ceil rounds up) if you REALLY want to round to the "nearest" integer you need to use Math.round().

In your case if you multiply by 6 and round down you'll never get a number higher than 5.

I suspect this is your problem, though I only glanced at your code, so, forgive me if that's just one mistake and not the cause of your issue.

[Edit] upon further reflection, disregard the above. The problem is that your methods expects 2 parameters but you're only passing in one.

I think you're misunderstanding the way parameter passing works.

jamCheck(p1, p2){} what you name these is not relevant. These labels only exist INSIDE your method. I suspect that what is confusing you is that you're using the same labels for the variables you're passing in, as well as the ones in your method. So, when you call the method jamCheck(roll1) it can't do what it needs to cause it's designed to work on 2 variables. Beyond that whatever result you're getting is just the browser trying to make up for code whose syntax is broken. In languages like C or Java you wouldn't even be able to compile such code; You'd be pointed at these lines as not making any sense, by the compiler.

So, the solution is (something like)...

var roll1,roll2;

roll1 = diceRoll1();
roll2 = diceRoll2();
jamCheck(roll1,roll2);
killCheck(roll1,roll2);

But in your diceRoll methods the last thing you'll need to do is return roll1; (or roll2 respectively)

And look to kirean's answer for how to wrap this all up in an init method, so that you're not calling 4 (or more) methods from the body's onload callback.

Thomas Jones · Accepted Answer · 2012-04-29 16:48:45Z

1

Youre doing a couple things wrong here, the first is confusing function scoped variables with global scoped variables.

this function

function jamCheck(roll1,roll2){
    if (roll1 == roll2)
    {
    document.getElementById('jam').innerHTML = 'GUN JAMMED';
    }
    return;
}

expects two parameters, but you are passing in none here body onload="...jamcheck()"

as a result, undefined is equal to undefined, so of course its true.

You need a wrapper function similar to this

function executeGame(){
   var dice1 = rollDice1();
   var dice2 = rollDice2();

   jamCheck(dice1, dice2)
}

And then call this function on body onload.

answered Apr 29, 2012 at 16:48

Thomas Jones

4,94731 silver badges35 bronze badges

1 Comment

snoopyj Over a year ago

Awesome thank you everyone! A good few things for me to try! I'll give them a go and get back to you all. I have much to learn!

Collectives™ on Stack Overflow

Beginners JavaScript

2 Answers 2

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related