2

Tengo un bucle infinito que se ejecuta cada segundo, el problema es, en la primera ejecución, siempre da un pequeño fallo, y después continúa sin problemas.

Pongo el ejemplo:

window.addEventListener('load', ()=>{

  document.querySelector('.magic').addEventListener('click', ()=>{
    startGame();
  })
});

function startGame(){
  //obtenemos un length
  let lng = document.querySelectorAll('.cont').length - 1
  //elegimos aleatoriamente un número entre 0 y el lenght
  let rng = randomNumber(lng);
  //obtenemos el cuadrado que queremos
  let dvc = getDivColored(rng);

  if(dvc.style.backgroundColor == dvc.getAttribute('data')){
    dvc.style.backgroundColor = 'white';
    console.log(dvc);
  }
  else{
    dvc.style.backgroundColor = dvc.getAttribute('data');
    console.log(dvc);
  }

  setTimeout(startGame, 1000);
}

function randomNumber(lng){
  return Math.floor(Math.random() * Math.floor(lng));
}

function getDivColored(rng){
  return document.querySelectorAll('.cont')[rng];
}
.cont{
    width:50px;
    height:50px;
    border:1px solid #000;
    margin:0 15px;
    display: inline-block;
    float: left;
}

.red{
    background: red;
}
.blu{
    background: blue;
}
.yel{
    background: yellow;
}
.gre{
    background: green;
}
.pur{
    background: purple;
}
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<head>
    <title>013 : paleta</title>
    <link rel="stylesheet" type="text/css" href="css/013.css">
    <script src="js/013.js"></script>
</head>
<body>
    <div class="plt">
        <div class="red cont" data="red"></div>
        <div class="blu cont" data="blue"></div>
        <div class="yel cont" data="yellow"></div>
        <div class="gre cont" data="green"></div>
        <div class="pur cont" data="purple"></div>
    </div>
<button type="button" class="magic">MAGIA</button>
</body>
</html>

Las primeras veces que activa aleatoriamente uno de los div, no se apaga, sin embargo, una vez los vuelve a llamar, si.

He probado a hacer el caso contrario:

if(dvc.style.backgroundColor != dvc.getAttribute('data')){
    dvc.style.backgroundColor = dvc.getAttribute('data');
    console.log(dvc);
  }
  else{
    dvc.style.backgroundColor = 'white';
    console.log(dvc);
  }

Añadiendo un != para comprobar que NO sean iguales, y por ende, ejecute lo que le mando. Pero igualmente, hace que se apaguen una vez, y le cueste dos intentos volver a encenderse.

(Apagar en este caso lo llamo a ponerlo en blanco)

¿Qué es lo que tengo mal?

4
  • Lo que quieres hacer es que en cada llamada a la función el elemento cambie su color, verdad ?
    – Christian
    Commented el 24 mar. 2021 a las 14:23
  • 2
    No necesitas restar 1 a length porque randomNumber(lng) nunca va a devolver lng. Math.random() nunca retorna 1, puede retornar 0 o cualquier número mayor a 0 y menor a 1. Commented el 24 mar. 2021 a las 15:28
  • @christian y lo hace, pero a partir de la segunda llamada, que es lo que me parece raro
    – AQMR
    Commented el 25 mar. 2021 a las 7:23
  • @EmilioPlatzer tienes razón con lo de no restar -1 al lng. Pero recuerda que ya que no devuelve mas que 0 o 1 (y lo que haya por medio) lo multiplico por el lng. Al final el resultado es el mismo, curiosamente, no tengo ningún tipo de diferencia con antes. Y no lo entiendo. Si hay 5 .cont, y los enumero, el cont[1] sería el primero, es por eso que restaba uno. Entiendo que multiplicar por 0 o muy cerca, de 0, pero, y el 5? no hay .cont[5]
    – AQMR
    Commented el 25 mar. 2021 a las 7:34

2 respuestas 2

2

No deberías esperar a la siguiente ejecución de startGame() para cambiar el estado del div encendido; puedes usar setTimeout() y, de preferencia, con un tiempo más corto, de forma que puedas ver si el mismo div se encendió dos o más veces seguidas.

Algunas mejoras sugeridas:

  • Como ya te dijeron en comentarios, no restes 1 a length porque no permitirá que se pueda seleccionar el último elemento
  • Declara una variable global donde tengas todos los elementos y puedas seleccionar o manipular por índice
  • Usa clases CSS para cambiar el color de fondo, así ya no necesitas el atributo data
  • No es necesario tener dos funciones para obtener un número al azar y luego seleccionar el elemento, puedes hacerlo fácilmente en una sola

Probablemente el uso de setInterval() sería más adecuado, pero eso ya te tocará investigar, probar y decidir si representa una mejora o no.

// Obtener todos los divs y usar para cálculos y operaciones
let coloredDivs = document.querySelectorAll('.cont');
// Crear variable para almacenar temporizador y poder limpiar
let timer;

window.addEventListener('load', ()=> {
    document.querySelector('.magic').addEventListener('click', ()=> {
        // Limpiar temporizador, si es que está activo
        clearTimeout(timer);
        startGame();
    });
});

function startGame(){
    //obtenemos el cuadrado que queremos
    let dvc = getDivColored();

    // "Encender" el div seleccionado agregando clase white
    dvc.classList.add('white');
    
    // Hay que "apagar" antes de volver a "encender" (el mismo u otro)
    setTimeout(() => dvc.classList.remove('white'), 750);
    // Volver a ejecutar
    timer = setTimeout(startGame, 1000);
}

function getDivColored(){
  //elegimos aleatoriamente un número entre 0 y el lenght
  let rng = Math.floor(Math.random() * coloredDivs.length);
  return coloredDivs[rng];
}
.cont{
    width:50px;
    height:50px;
    border:1px solid #000;
    margin:0 15px;
    display: inline-block;
    float: left;
}

.red{
    background: red;
}
.blu{
    background: blue;
}
.yel{
    background: yellow;
}
.gre{
    background: green;
}
.pur{
    background: purple;
}
.white {
    background: white;
}
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<head>
    <title>013 : paleta</title>
    <link rel="stylesheet" type="text/css" href="css/013.css">
    <script src="js/013.js"></script>
</head>
<body>
    <div class="plt">
        <div class="red cont" data="red"></div>
        <div class="blu cont" data="blue"></div>
        <div class="yel cont" data="yellow"></div>
        <div class="gre cont" data="green"></div>
        <div class="pur cont" data="purple"></div>
    </div>
<button type="button" class="magic">MAGIA</button>
</body>
</html>

2
  • En referencia a la última mejora, me recomendaron que dividiera funciones por lo que hacen, para no liarme al menos mientras voy aprendiendo. Pero muchísimas gracias @Triby, me has ayudado mucho! =D
    – AQMR
    Commented el 26 mar. 2021 a las 7:37
  • @AQMR, entiendo la idea de funciones con "tarea única", pero tampoco hay que ser tan estrictos en ese sentido. La tarea de la función es obtener un elemento al azar, no importa que se tengan que ejecutar 2 o más instrucciones, se siguen considerando parte de la misma tarea.
    – Triby
    Commented el 26 mar. 2021 a las 14:35
0

Sin embargo ya descubrí mi problema inicial gracias a la respuesta de @Triby,. El problema es que comparaba estilos backgroundColor, y en el css lo tenía simplemente como background.

Una vez corregido eso, solamente me quedaba comparar si en la classList contenía la clase white. Nada de estilos, y ya estaría.

Así queda como lo quería en un principio:

window.addEventListener('load', ()=>{

  document.querySelectorAll('.cont').forEach(item=>{
    item.addEventListener('click', (item)=>{
      let col = getColor(item.target);
      changeCol(col);
    });
  });

  document.querySelector('.magic').addEventListener('click', ()=>{
    //creamos un boton para parar el ciclo
    let stop_btn = createStopBtn();
    if(!document.querySelector('.stop-btn')){
      document.body.appendChild(stop_btn);
    }
    var timer = false;
    startGame(stop_btn, timer);
  })
});

function getColor(item){
  return item.getAttribute('data');
}

function changeCol(col){
  if(document.body.style.backgroundColor != col){
    document.body.style.backgroundColor = col;
  }
  else{
    document.body.style.backgroundColor = 'white';
  }
}

function startGame(stop_btn, timer){
  //obtener el boton
  if(document.querySelector('.stop-btn') == null){
    console.log('timer has stopped');
    clearTimeout(timer);
  }
  else{
    document.querySelector('.stop-btn').onclick = function(){
      clearTimeout(timer);
      document.querySelector('.stop-btn').remove();
  }}
  //obtenemos un length
  let lng = document.querySelectorAll('.cont').length;
  //elegimos aleatoriamente un número entre 0 y el lenght
  let rng = randomNumber(lng);
  //obtenemos el cuadrado que queremos
  let dvc = getDivColored(rng);
  let data = dvc.getAttribute('data');

  if(dvc.classList.contains(data)){
    dvc.classList.remove(data);
    dvc.classList.add('white');
  }
  else{
    dvc.classList.remove('white');
    dvc.classList.add(data);
  }

  timer = setTimeout(startGame, 1000);
}

function randomNumber(lng){
  return Math.floor(Math.random() * Math.floor(lng));
}

function getDivColored(rng){
  return document.querySelectorAll('.cont')[rng];
}



function createStopBtn(){
  let btn = document.createElement('button');
      btn.type='button';
      btn.innerHTML='STOP';
      btn.className='stop-btn'

  return btn
}
.cont{
    width:50px;
    height:50px;
    border:1px solid #000;
    margin:0 15px;
    display: inline-block;
    float: left;
}

.red{
    background-color: red;
}
.blue{
    background-color: blue;
}
.yellow{
    background-color: yellow;
}
.green{
    background-color: green;
}
.purple{
    background-color: purple;
}
.white {
    background-color: white;
}
<!DOCTYPE html>
<html>
<meta charset="utf-8">
<head>
    <title>013 : paleta</title>
    <link rel="stylesheet" type="text/css" href="css/013.css">
    <script src="js/013.js"></script>
</head>
<body>
    <div class="plt">
        <div class="red cont" data="red"></div>
        <div class="blue cont" data="blue"></div>
        <div class="yellow cont" data="yellow"></div>
        <div class="green cont" data="green"></div>
        <div class="purple cont" data="purple"></div>
    </div>
<button type="button" class="magic">MAGIA</button>
</body>
</html>

Tu Respuesta

By clicking “Publica tu respuesta”, you agree to our terms of service and acknowledge you have read our privacy policy.

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.