Difficulty getting Timer to operate according to Math.random

Can you tell me where I’m going wrong getting the random number into the timer function?

Process: Person chooses a staging countdown number for the initial black screen, then it moves to the yellow screen. That works. The following script obtains a random number from 1-6 secs and puts it into the timer and the screen is yellow.

With the completion of the timer, the screen goes green for 5 secs, the resets back to the home screen, ready to start the sequence again.

In my case, the screen stays yellow, and I don’t think the countdown is working.

/*https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/random	*/
// set countdown period from 1 to 6 secs randomly, yellow screen
function yellow() {
  let cwindow = document.getElementById("colorwindow");
	cwindow.style.backgroundColor='yellow';
// Returns a random integer from 1 to 6:
var min=1;
var max=7;
function getRandomInt(min, max) {
  min = Math.ceil(min);
  max = Math.floor(max);
  var randomNo = Math.floor(Math.random() * (max - min) + min); //The maximum is exclusive and the minimum is inclusive

// set timer period according to random number picked
  var counter = setInterval(timer, randomNo);
	function timer() {
	  count=randomNo * 1000;
	  if (count <= 1000)
	  {
		 clearInterval(counter);	 
		 green();
		 return;
	  }	
	document.getElementById("timer").innerHTML=count;
	console.log(count + " yellow"); // does not show in console
  }
}
}

// set countdown period of 5 secs with green GO BG
function green() { // never turns green
  let green = document.getElementById("colorwindow");
  green.style.backgroundColor="green";
  
  var count=5;
  var counter=setInterval(timer, 1000);
  let cwindow = document.getElementById("colorwindow");
		cwindow.style.backgroundColor='black';
	function timer() {
	  count=count-1;
	  if (count <= 0)
	  {
		 clearInterval(counter);
		 resetall();	
		 return;
	  }	  
}
}

// reset to original home screen
function resetall() {
  window.location=("testing-yellow-red-lights.html");
}

resetbut.addEventListener('click', resetall);

You’ve got a couple different things going on - the big one is getRandomInt is never called. TBH, I’m not quite sure why you’ve got getRandomInt in there as a function. If something isn’t called, it doesn’t need to be defined as a function…

Once you get past that, the timer function in getRandomInt isn’t going to work - it never deprecates the count, so it’ll never move on.

After you get it working, I’d really recommend a refactoring to clean up the structure as on quick glance looks like there is a lot of duplicated code in there.

1 Like

How do I turn the timer function into a non-function so it still takes on the value of the randomNo from the previous function?

// set timer period according to random number picked
  var counter = setInterval(randomNo);
//function timer() {
	  count=randomNo-1;
	  if (count <= 0)
	  {
		 clearInterval(counter);	 
		 green(); // does not go to next function
		 return;
	  }
	document.getElementById("timer").innerHTML=count;
	console.log(count + " yellow"); // shows a digit in console
 //}

I think you need to see promise tutorials.
They tend to use timers to demonstrate the promise concept
Like https://javascript.info/promise-basics

I think I am very close to making the random function work with the timer. Problem now is the timer is not operating in sequence, but numbers are appearing out of order; only when a digit hits 0 does the next screen open. The jsfiddle above has been updated.

function staging(ev) {
  var count=ev;
  var counter=setInterval(function() {
  let cwindow = document.getElementById("colorwindow");
	cwindow.style.backgroundColor='black';
  let dwindow = document.getElementById("timing");
	dwindow.style.display="none";
timer();
	function timer() {
	  count=count-1;
	  if (count <= 0)
	  {
		 clearInterval(counter);
		 yellow();
	  }		 
	document.getElementById("timer").innerHTML=count;
	}
}, 1000);
}

let btn5 = document.getElementById("but5");
btn5.addEventListener('click', event => {
  staging(5);
});

let btn10 = document.getElementById("but10");
btn10.addEventListener('click', event => {
  staging(10);
});

let btn15 = document.getElementById("but15");
btn15.addEventListener('click', event => {
  staging(15);
});


function yellow() {
  var counter = setInterval(function() {
  let cwindow = document.getElementById("colorwindow");
	cwindow.style.backgroundColor='yellow';
  let dwindow = document.getElementById("countdown");
// hide the countdown 	dwindow.style.display="none"; 
	getRandomIntInclusive(); 
		function getRandomIntInclusive(min, max) {
		  min = Math.ceil(1);
		  max = Math.floor(6);
		  var count = Math.floor(Math.random() * (max - min + 1) + min); //The maximum is inclusive and the minimum is inclusive
		  timer(count);
		}

	function timer(count) {
	  count = count - 1;
	  if (count <= 0)
	  {
		 clearInterval(counter);
		 green();
	  }		 
document.getElementById("timer").innerHTML = count; // 	show the countdown
	}
  }, 1000);
}


function green() {
  var count = 3;
  var counter=setInterval(function() {
  let cwindow = document.getElementById("colorwindow");
	cwindow.style.backgroundColor='green';
timer();
	function timer() {
	  count = count - 1;
	  if (count <= 0)
	  {
		clearInterval(counter);
		resetall();
	  }		 
	document.getElementById("timer").innerHTML=count;
	}
}, 1000);
}



// reset to original home screen
function resetall() {
  window.location=("testing-yellow-red-lights.html");
}

resetbut.addEventListener('click', resetall);

What I see happening is that while in Yellow mode it is continually getting a random number and only when the random number it gets is zero does it go to green.
You are misusing the setInterval method.
The first parameter is executed continually every “delay” milliseconds.
“delay” is the second parameter.
Check out https://developer.mozilla.org/en-US/docs/Web/API/setInterval.

I think is would be easier to use the the async, await api for this.

I’ve been having a difficult time understanding what the expected behaviour is supposed to be.

Here’s the updated code, that has also been linted to help add some consistancy.

Currently the existing behaviour on clicking 5 is:

Black: 4, 3, 2, 1, 0
Yellow: random, random, random, 0
Green: 2, 1, 0

Where yellow keeps on going with different random numbers until it shows 0.

Can you please detail what the expected behaviour is supposed to be instead?

1 Like

I’ve been working at making things consistent in the code, and have reached an update worth posting.

function getRandomIntInclusive(min, max) {
  //The maximum is inclusive and the minimum is inclusive
  const count = Math.floor(Math.random() * (max - min + 1) + min);
  return count;
}

function stagingCallback() {
  const count = getRandomIntInclusive(1, 6);
  yellow(count, yellowCallback);
}

function yellowCallback() {
  green(3, greenCallback);
}

function greenCallback() {
  resetall();
}

function timer(count, callback) {
  // show the countdown
  document.getElementById("timer").innerHTML = count;
  if (count < 1) {
    return callback();
  }
  return count - 1;
}

function staging(count, callback) {
  function updateCounter() {
    const cwindow = document.getElementById("colorwindow");
    cwindow.style.backgroundColor = "black";
    const dwindow = document.getElementById("timing");
    dwindow.style.display = "none";
    count = timer(count, function() {
      clearInterval(counter);
      callback();
    });
  }
  var counter = setInterval(updateCounter, 1000);
}

function yellow(count, callback) {
  function updateCounter() {
    const cwindow = document.getElementById("colorwindow");
    cwindow.style.backgroundColor = "yellow";
    count = timer(count, function() {
      clearInterval(counter);
      callback();
    });
  }
  var counter = setInterval(updateCounter, 1000);
}

function green(count, callback) {
  function updateCounter() {
    const cwindow = document.getElementById("colorwindow");
    cwindow.style.backgroundColor = "green";
    count = timer(count, function() {
      clearInterval(counter);
      callback();
    });
  }
  var counter = setInterval(updateCounter, 1000);

}

// reset to original home screen
function resetall() {
  window.setTimeout(function() {
    window.location = ("testing-yellow-red-lights.html");
  }, 1000);
}


const btn5 = document.getElementById("but5");
btn5.addEventListener("click", () => staging(5, stagingCallback));

const btn10 = document.getElementById("but10");
btn10.addEventListener("click", () => staging(10, stagingCallback));

const btn15 = document.getElementById("but15");
btn15.addEventListener("click", () => staging(15, stagingCallback));

const resetbut = document.querySelector("#resetbut");
resetbut.addEventListener("click", resetall);

I’ve reached a point where the staging, yellow and green functions are basically identical to each other, achieved by moving configuration information out of them as callback functions.

I’ll now take a leaf from other kinds of updates and replace the count parameter with an object that has count as a property, so that other properties can be used too.

The separate staging/yellow/green functions have all been made identical to each other now, allowing us to remove those functions and use a single countdown function for them instead.

function startCountingFrom(count) {
  window.setTimeout(function () {
    const dwindow = document.getElementById("timing");
    dwindow.style.display = "none";
  }, 1000);
  countdown({count, bgcolor: "black", callback: stagingCallback});
}

function stagingCallback() {
  function getRandomIntInclusive(min, max) {
    //The maximum is inclusive and the minimum is inclusive
    const count = Math.floor(Math.random() * (max - min + 1) + min);
    return count;
  }

  const count = getRandomIntInclusive(1, 6);
  countdown({count, bgcolor: "yellow", callback: yellowCallback});
}

function yellowCallback() {
  countdown({count: 3, bgcolor: "green", callback: greenCallback});
}

function greenCallback() {
  resetall();
}

function timer(count, callback) {
  // show the countdown
  document.getElementById("timer").innerHTML = count;
  if (count < 1) {
    return callback();
  }
  return count - 1;
}

function countdown({count, bgcolor, callback}) {
  function updateCounter() {
    const cwindow = document.getElementById("colorwindow");
    cwindow.style.backgroundColor = bgcolor;
    count = timer(count, function() {
      clearInterval(counter);
      callback();
    });
  }
  var counter = setInterval(function () {
    updateCounter();
  }, 1000);
}

// reset to original home screen
function resetall() {
  window.setTimeout(function() {
    window.location = ("testing-yellow-red-lights.html");
  }, 1000);
}

const btn5 = document.getElementById("but5");
btn5.addEventListener("click", () => startCountingFrom(5));

const btn10 = document.getElementById("but10");
btn10.addEventListener("click", () => startCountingFrom(10));

const btn15 = document.getElementById("but15");
btn15.addEventListener("click", () => startCountingFrom(15));

const resetbut = document.querySelector("#resetbut");
resetbut.addEventListener("click", resetall);

That all seems to be enough of an improvement for the moment.

3 Likes

Yellow is supposed to generate ONE random number. That number is saved and passed on to the timer, then the timer begins its countdown from there. It should count down anywhere from 1 to 6 secs to the green.

The person taps 5, 10, or 15 and gets that many seconds to go to the RC car drivers station and get ready. The yellow tells the drivers to be alert, but not for how long, and when the green lights up, the cars take off. That’s how it work. Its a ready, set, go for the RC car drivers for drag racing.

This is what I wanted!

Looks like it was more complicated than I thought. I supposed that I was bumping up against some inherent Javascript limitation with the way I was doing it, but it was just my lack of understanding of how to form it.

It’s so cool to see the things I’ve read about actually put into practice so eloquently.

I see what you did with the parameters. I included the countdown on all the screens to see what was happening. The countdown needs to be shown only on the black staging screen. So I added another parameter to refer to the color of the timer font, and changed the timer font color (“tcolor”) to the screen color:

function yellowCallback() {
  countdown({count: 3, bgcolor: "green", tcolor: "green", callback: greenCallback});
}
function countdown({count, bgcolor, tcolor, callback}) {
  function updateCounter() {
    const cwindow = document.getElementById("colorwindow");
    cwindow.style.backgroundColor = bgcolor;	
    const twindow = document.getElementById("timer");
    twindow.style.color = tcolor;
    count = timer(count, function() {
      clearInterval(counter);
      callback();
    });
  }
1 Like

That’s why I asked for further detail. Is the following correct instead?

Black: 5, 4, 3, 2, 1
Yellow: show yellow color for a random amount of time
Green: show green color

Yes, that is correct. The random amount is in the range of 1 to 6 secs.

Good one. Because staging has the countdown shown and the others don’t have the countdown shown, that just means having two different functions, one that shows the count, and another that doesn’t.

function showCount(count, bgcolor) {
  const cwindow = document.getElementById("colorwindow");
  cwindow.style.backgroundColor = bgcolor;
  document.getElementById("timer").innerHTML = count;
}
function noCount(count, bgcolor) {
  const cwindow = document.getElementById("colorwindow");
  cwindow.style.backgroundColor = bgcolor;
  document.getElementById("timer").innerHTML = "";
}

The countdown functions are affected just by having a separate update property added to them.

function stagingCountdown(count) {
  countdown({
    count,
	bgcolor: "black",
	update: showCount,
	callback: yellowCountdown
  });
}

function yellowCountdown() {
  countdown({
    count: getRandomIntInclusive(1, 6),
	bgcolor: "yellow",
	update: noCount,
	callback: greenCountdown
  });
}

function greenCountdown() {
  countdown({
    count: 2,
	bgcolor: "green",
	update: noCount,
	callback: resetCallback
  });
}

I also made a few other tweaks, such as by using a single setTimeout call to help simplify the countdown.

function countdown({count, bgcolor, update, callback}) {
  let timedUpdate = updateCounter;
  function updateCounter() {
    update(count, bgcolor);
    count -= 1;
    if (count === 0) {
      timedUpdate = callback;
    }
    setTimeout(timedUpdate, 1000);
  }
  updateCounter();
}

The updated code is found at https://jsfiddle.net/pmw57/jtnxhrsu/4/

1 Like

The way I understand you, this is what you want to achieve.
onSubmit:
black screen for value on the clicked input
.then:
Yellow screen for random(1…6) seconds
.then:
Green screen for 5 secs
.then:
Black screen
The only reason you need the setInterval function is to change the number on the screen and as you are doing that whatever the colour, put it into a function with colour and delay parameters
That way that function can look after changing the colour and changing the number.
If that function is an async function and it is coded to await the completion of the timer then the code is just a series of calls to the function with the colour and delay values.
Sooner or later every programmer learns that there comes a time to junk everything and start again from scratch.

Thank you! The working page is located here, with some tweaks by the boss:
https://www.reedyrace.com/ae/ateamapps/helps/lights/index.html

2 Likes

This link is for the async, await api
The first code sample is exactly what you need for getting the timer to work.

It’s already working perfectly well.

Do you feel like putting together some working code that achieves the same result using async?

Paul, I have done it using a sequence of setTimeout’s
It doest want to run in a codepen, only as a web page.
First the html, I changed it slightly.

<html>
<head>
	<meta charset="utf-8">
	<meta name="viewport" content="width=device-width, initial-scale=1.0">
	<title>Demonstration for use of async code</title>
    <link href="asyncdemo.css" rel="stylesheet" type="text/css">
    <script defer src="asyncDemo.js"></script>


</head>
<body id = 'body' data-tgtpid = "0" data-db-filename = "" data-rowcount = 0 data-mode = "reader" data-gender = "male">

<div id="content">

	<header id="page-header">
	<h1>Ready, Set, Go</h1>
	</header>


<!-- could be a main content section -->
	<section id="timing">
		<header class="section-header">
		<h2>Staging Duration (seconds)</h2>
		</header>

		<article class="article1">
			<div id = "buttons" class="durationarray">
				<div class="but durationcell"><input id="but5" type="button" value="5"></div>
				<div class="but durationcell"><input id="but10" type="button" value="10"></div>
				<div class="but durationcell"><input id="but15" type="button" value="15"></div>
			</div>
		</article>
	</section>
	<section id="lights">

		<article class="article1">
		<div id="colorwindow"><h1 id="countdown"><span id="timer"></span></h1></div>
		</article>
	</section>
<!---->
	<footer id="footer">
		<input id="resetbut" type="button" value="Start Over" class="but resetbut">
	</footer>

</div>

</body>
</html>

Now the css, also changed slightly.
the colour of the numbers and the window colours as a class.


html {
  box-sizing: border-box;
}
*, *::before, *::after {
  box-sizing: inherit;
}

body {
	font-family: arial, sans-serif;
	margin: 0;
	padding: 0;
	background-color: black;
}
#content {
	margin: 0;
	padding: 1em;
}
input, button, reset {
	font: inherit;
}
p, h1, h2, h3, h4, h5, h6 {
	color: white;
}
.durationarray {
  display: grid;
  grid-template-columns: repeat(3, 1fr);
  grid-template-rows: 100%;
  gap: 8px;
  justify-items: stretch;
  height: 5em;
}

.durationcell {
  justify-self: center;
  align-self: center;
  text-align: center;
  width: 100%;
	background-color: #666;
}
input[type="button"],
.but {
  display:block;
  border-radius: 5px;
  font-weight: bold;
  font-size: 2em;
  color: white;
  width: 100%;
	background-color: #666;
}

#resetbut {
  font-size: 3em;
}
#timer {
  display: block;
  margin:auto;
  padding-top:40px;
  color: red;
  text-align:center;
  font-size: 12em;
}

#colorwindow {
	width: 100%;
	height: 400px;
	margin: 1em 0;
	padding: 1em 2em 1em 1em;
}
.black {
	background-color: black;
}
.yellow {
	background-color: yellow;
}
.green {
	background-color: green;
}

And finally the js
I used a click event rather than a submit event to start the sequence.

"use strict";
const $id = id => document.getElementById(id);
const min = 1;
const max = 7;
let delayA = ""
let delayB = "";
let delayC = ""
let div = $id('colorwindow');
const setDelays = ev => {
	let randNr = Math.floor(Math.random() * (max - min) + min);

	let tgt = ev.target;
	delayA = ev.target.value;
	delayB = +randNr;
	delayC = 5;
	action();
}

const showCountDown = ticks => {
	let el = $id('timer');
	if(ticks < 1) {
		el.innerHTML = "";
	} else {
		el.innerHTML = ticks;
		let tickCounter = () => {
			if(ticksLeft < 1) {
				clearTimeout(timerTick);
				el.innerHTML = "";
			} else {
				el.innerHTML = ticksLeft - 1;
				ticksLeft--;
			}
		}
		let ticksLeft = ticks;
		let timerTick = setInterval(tickCounter, 1000);
	}
}

const action = () => {
	console.log(`Delays are: ${delayA}, ${delayB} and ${delayC}`);
	$id("buttons"),addEventListener('click', setDelays, false);
	$id('colorwindow').classList.add("black");
	showCountDown(delayA);
	setTimeout(() => {
		showCountDown(delayB);
		console.log(`Changing to yellow after ${delayA} Secs` );
		div.classList.remove("black");
		div.classList.add("yellow");
		setTimeout(() => {
			showCountDown(delayC);
			console.log(`Changing to green after ${delayB} Secs` );
			div.classList.remove("yellow");
			div.classList.add("green")
			setTimeout(() => {
				showCountDown(-1);
				console.log(`Changing to black after ${delayC} Secs` );
				div.classList.remove("green");
				div.classList.add("black")},
				delayC * 1000);


			},
			delayB * 1000);


		},
		delayA * 1000);

}

$id("buttons").addEventListener('click', setDelays, false);


The filenames are asyncDemo,html, asyncDemo.css and asyncDemo.js

1 Like