tmedwards / sugarcube-2

SugarCube is a free (gratis and libre) story format for Twine/Twee.
https://www.motoslave.net/sugarcube/2/
BSD 2-Clause "Simplified" License
177 stars 41 forks source link

Keyup function doesn't work with <<button>> and <<link>> #148

Closed wiljammer closed 2 years ago

wiljammer commented 2 years ago

Hello, I would like to report a bug with the sound handling of Twine 2.3.14 and Sugarcube 2.34.1. The problem is following: I am using arrowkeys to make sounds - when the key is pressed, a looping sound is heard. I have used the story Javascript (code below) to play the sound: keydown starts a loop and keyup stops it. It works nicely in all the passages except those with buttons. It seems that the button-macro is messing up the keyup -command resulting that the audio keeps playing even when the keypress is lifted. This also happens with link -macro unless the link-macro itself is replaced with the link pressing ( see example below). Anyway, I can work with the replace-macro, but I would love to have the buttons also in my game! The issue also persists when using "cacheaudio" / "audio" -macros or "on" -macro (code example below).

With this code the audio works

<div id="task">
<<link "Click for a new task">>
    <<replace "#task">>Use arrow keys to make sounds<</replace>>
<</link>>
</div>

With these examples it doesn't work:

<<button "Click for a new task">><<run Dialog.setup("Arrowtest")>><<run Dialog.wiki("Use arrow keys to make sounds")>><<run Dialog.open()>><</button>>

<<link "Click for a new task">>
    <<replace "#task">>Use arrow keys to make sounds<</replace>>
<</link>>
<div id="task"></div>

Story JS code:

SimpleAudio.tracks.add("left", "left.mp3");

(function () {
    $(document).on('keydown', function (ev) {
        if (ev.key === 'ArrowLeft') {
             SimpleAudio.select("left").loop(true).play();
            }
    });
$(document).on('keyup', function (ev) {
        if (ev.key === 'ArrowLeft') {
             SimpleAudio.select("left").stop();
        }
});
}());

It doesn't stop with this either (in story javascript there is only the events-macro by Chapel):

<<cacheaudio "left" "left.mp3">> 
<<on 'keydown'>>
<<which 37>>
     <<audio "left" loop play>>
<</on>>
<<on 'keyup'>>
<<which 37>>
     <<audio "vasen" stop>>
<</on>>

<<button "test">> 
<</button>>

Windows 10, Chrome 95,

tmedwards commented 2 years ago

I'll look at this when I have a chance, but at this point I can't help but think that the macros' involvement is coincidence.

The why of that is for two reasons:

  1. The event handlers for the macro (<<link>>/<<button>>) are locally bound, meaning they're not going to see any events, keyboard or otherwise, that aren't focused on them or a descendant.
  2. The macro doesn't listen for keydown or keyup events, so instances shouldn't be able to interact with those events in the first place.

TL;DR I'm not saying that you're not seeing something. Just that at this point I don't see how that macro is involved in any way other than coincidently.

wiljammer commented 2 years ago

Ok, thanks for looking in to it when you have time! I hope you can find the solution. It is a weird thing... I'll let you know if I crack this somehow!

Viljami

pe 29. lokak. 2021 klo 5.43 Thomas M. Edwards @.***) kirjoitti:

I'll look at this when I have a chance, but at this point I can't help but think that the macros' involvement is coincidence.

The why of that is for two reasons:

  1. The event handlers for the macro (<>/<
  2. The macro doesn't listen for keydown or keyup events, so they shouldn't be able to interact with them in the first place.

TL;DR I'm not saying that you're not seeing something. Just that at this point I don't see how that macro is involved in any way other than coincidently.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tmedwards/sugarcube-2/issues/148#issuecomment-954363946, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUBMIQFQATJABVYLWY24WN3UJIC47ANCNFSM5G4XX5YQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

tmedwards commented 2 years ago

Hello! I've finally had a chance to look into this.

Unfortunately, I could not replicate the issue with the provided code examples. I tested in both Firefox (v95.0.1) and Chrome (v96.0.4664.110) and the audio commands attached to the left arrow worked flawlessly—I do have a suggestion below though.

Is there any other context you could provide? Or a working example—of it not working 😄 .

Key Handling Suggestion

The keydown event fires as long as a key is held down. Your current code does not check to see whether a keydown event for the specific key was seen before running its looping audio command again. You should use a sentinel to prevent the code from being spammed several times a second, since the audio is already set to loop.

For example:

(function () {
    // Flag to prevent `keydown` event spam.
    var isArrowLeft = false;

    $(document)
        .on('keydown', function (ev) {
            if (ev.key === 'ArrowLeft' && !isArrowLeft) {
                isArrowLeft = true;
                SimpleAudio.select('left').loop(true).play();
            }
        })
        .on('keyup', function (ev) {
            if (ev.key === 'ArrowLeft') {
                SimpleAudio.select('left').stop();
                isArrowLeft = false;
            }
        });
})();
tmedwards commented 2 years ago

Stale issue.