gtim / svelte-chess

Fully playable chess component for Svelte.
https://gtim.github.io/svelte-chess/
20 stars 6 forks source link

Is it possible to have a `detail.checked` property in the `move` event? #2

Closed buhodev closed 1 year ago

buhodev commented 1 year ago

When implementing sounds, my mental modal consists on these three types of sounds:

  1. movement sounds (standard, captured, castle, and check)
  2. result sounds (defeat, victory, and draw)
  3. and sounds for UI actions (button click, etc).

It's really easy to implement sounds for the first three types of movements (all of them can be inferred from the detail property of the move event.) But when it comes to implement the sound for the check movement, that state/property is not present there (like the captured property, that appears when a player captures a piece).

To account for this, my current implementation uses the inCheck prop:

<script>
  import { onMount } from 'svelte';
  import { Chess } from 'svelte-chess';

  let chess;
  let sounds;
  let history;
  let inCheck;
  let isGameOver;
  let playerColor = 'white';

  function handleMove(event) {
    // captured event
    if (event.detail.captured) {
      sounds.capture.play();
      return;
    }
    // castle event
    if (event.detail.san == 'O-O' || event.detail.san == 'O-O-O') {
      sounds.castle.play();
      return;
    }
    // standard move event
    sounds.move.play();
  }

  // check event  
  $: if(inCheck) sounds.check.play();

  function handleGameOver(event) {
    let winner = undefined;

    if (event.detail.result === 0.5) {
      winner = 'draw'
      alert('Draw!');
    }  
    if (event.detail.result === 0) {
      winner = 'black'
      alert('Black won!');
    }  
    if (event.detail.result === 1) {
      winner = 'white'
      alert('White won!');
    }

    // draw event
    if (winner === 'draw') sounds.draw.play();

    // victory event
    if (winner === playerColor) sounds.victory.play();

    // defeat event
    if (winner !== 'draw' && winner !== playerColor) sounds.defeat.play();
  }

  // use the Audio API when the component mounts
  onMount(() => {
    sounds = {
      move: new Audio('/sounds/move.mp3'),
      capture: new Audio('/sounds/capture.mp3'),
      check: new Audio('/sounds/check.mp3'),
      victory: new Audio('/sounds/victory.mp3'),
      defeat: new Audio('/sounds/defeat.mp3'),
      draw: new Audio('/sounds/draw.mp3'),
      castle: new Audio('/sounds/castle.mp3')
    };
  });
</script>

<Chess
  on:move={handleMove}
  on:gameOver={handleGameOver}
  bind:this={chess}
  bind:history
  bind:inCheck
  bind:isGameOver
/>

What's the problem?

When the inCheck prop changes to true, the check sound is played. But, the condition for the standard movement is met as well, so the move sound is played too along with the check sound.

I can try to fix this problem adding another condition in the handleMove function. There are two ways we can accomplish that:

The first one is playing the standard sound only when the inCheck property is false and keeping the logic for playing the check event outside the handleMove function like before:

function handleMove(event) {
  // captured event
  if (event.detail.captured) {
    sounds.capture.play();
    return;
  }
  // castle event
  if (event.detail.san == 'O-O' || event.detail.san == 'O-O-O') {
    sounds.castle.play();
    return;
  }
  // standard move event
-  sounds.move.play();
+  if (!inCheck) sounds.move.play();
}

// check event  
$: if(inCheck) sounds.check.play();

The second way is moving the logic for playing the check sound inside the handleMove function (as another condition):

function handleMove(event) {
  // captured event
  if (event.detail.captured) {
    sounds.capture.play();
    return;
  }
  // castle event
  if (event.detail.san == 'O-O' || event.detail.san == 'O-O-O') {
    sounds.castle.play();
    return;
  }
+  // check event
+  if (inCheck) {
+    sounds.check.play()
+    return;
+  }

  // standard move event
  sounds.move.play();

}

- // check event
- $: if(inCheck) sounds.check.play();

Is the problem solved this way? Nope! There's a subtle bug.

When player 1 executes the move that produces the inCheck state, the standard move sound is played right after, not the check sound. However, for the next move (when player 2 escapes from the check), the check sound is played then, although there's not inCheck state anymore.

My question is: is it possible to have a detail.checked or detail.check property in the move event (something like the detail.captured property that is exposed when the player captures a piece?)

Reproduction

  1. Go to the REPL
  2. Uncomment lines 25 to 28, and comment line 35
  3. Click the first button ('go 1 before check')
  4. Move the queen to h4 (listen to the standard move sound, instead of the check sound)
  5. Move king to e2 (listen to the check sound, instead of the standard move sound)
gtim commented 1 year ago

Thanks for your clear feature request! Extending the Move type with check seems the cleanest solution, just like you suggest. I've extended it with both check and checkmate for 0.8.4, does it resolve this issue for you?

buhodev commented 1 year ago

It works like a charm ✨ Thank you very much for your quick implementation!