1

For some time already I'm struggling with following code:

$(document).ready(function(){
     keyBind();
});
var set = [];
var start = 0;
var total = 3;
var timeout = 1000;
function displayImages(i, total){
    var pixBox = $('#picture-box');
    var imgPre = 'resources/exhibit-';
    var imgExt = '.png';
    var sndExt = '.wav';
    var imgSrc = '<img class="image" src="' + imgPre + i + imgExt +'">';
    var sndSrc = new Audio(imgPre + i + sndExt);
    var magic = ((i+1)%total) + ',' + total;

    sndSrc.play();
    pixBox.append(imgSrc);

    var sT = setTimeout('displayImages(' + magic + ')', timeout);
    set.push(sT);

    if(sT >= total+1){ 
        sndSrc.pause();
    }
    if(sT === total+1){     
        clearTimeout(sT);               
        // $('img').remove();
        $('img:not(:last-child)').remove();
    }
    return false;
}
function keyBind(){
    $(document).keydown(function(e) {
        switch (e.which) {
            case 75: //k
                displayImages(start, total); 
                break;
            case 80: //p
                clearTimeout(set);
                break;
            default:
                return;
        }
        e.preventDefault();
    });
}

It's a kind of very primitive slideshow with accompanying sound files for each picture/slide. As you can see, it's controlled by keyboard input (keyBind function). And it all works fine, but only for the first time. When I trigger it fresh, timeOut function is doing it job and both if statements (first responsible for cutting the sound after the last file, second responsible for wrapping images back to first one after last is displayed) are fireing up and all is well.

That is, until I want to restart the sequence again, without refreshing. If I do that, soundfiles are going mute and the image sequence turns into an endless loop.

I've already tried dividing start and stop of sequence in separate functions, I tried to clear the div and reset the sound at the start of sequence and I did tried reseting the timeOut at the beggining too. All to no avail. Do you, good and dear people of SO, have any idea what's wrong with my code and can shed some light on it? It's gonna be a lifesaver.

EDIT

It looks like setTimeout(sT) is not working. I've put a console.log after it and sT is not 0, it still has ID of last iteration. What may be the cause? What am I doing wrong?

roonroon
  • 297
  • 4
  • 17

2 Answers2

1

Some issues:

You use the value that setTimeout returns as if it has some meaning (incremental), but the actual value is implementation dependent. You should only use it for passing it to clearTimeout, but not for things like the following:

if(sT >= total+1) 

It is also not clear why you would want to add them to the set array, as only the last one really is pending. All the others have already expired, since you "chain" calls to setTimeout.

You clear a time-out right after setting one, while there is nothing you did not already know when setting it. So why not avoiding the setTimeout when you would be clearing right after?

Also, the argument to clearTimeout should be a number. It cannot be an array like set:

        case 80: //p
            clearTimeout(set); 

You have a start variable, but ignore it when doing (i+1)%total.

Although not a problem here, you should avoid using strings as arguments to setTimeout, as it is just as evil as eval that way.

Here is a version of your code that fixes several of these issues:

$(document).ready(function(){
     keyBind();
});

var sT = -1;
var total = 3;
var timeout = 1000;

function displayImages(i){
    var pixBox = $('#picture-box');
    var imgPre = 'resources/exhibit-';
    var imgExt = '.png';
    var sndExt = '.wav';
    var imgSrc = '<img class="image" src="' + imgPre + i + imgExt +'">';
    var sndSrc = new Audio(imgPre + i + sndExt);
    i = (i+1)%total;

    sndSrc.play();
    pixBox.append(imgSrc);

    if(i == 0){
        sT = -1;
        sndSrc.pause();
        $('img:not(:last-child)').remove();
    } else {
        sT = setTimeout(displayImages.bind(null, i), timeout);
    }
    return false;
}

function keyBind(){
    $(document).keydown(function(e) {
        switch (e.which) {
            case 75: //k
                displayImages(0); 
                break;
            case 80: //p
                clearTimeout(sT);
                break;
            default:
                return;
        }
        e.preventDefault();
    });
}

I am not quite sure when you actually want to stop the sound and remove the images (except one). You might still need to change this code a bit.

trincot
  • 211,288
  • 25
  • 175
  • 211
  • Yeah, i figured that thing with array on my own. TBH I don't know from where ideas like this come from. But it happened. But that's minor. Major thing is THANK YOU! Tweaked it a bit and it works like a charm. You sir, are awesome!. – roonroon Mar 21 '16 at 23:09
0

EDIT: Adjusted according to correcting comment below: You should not pass arguments to a function directly using settimeout. This has bugged me out a number of times. See this old post for a solution to this particular part of your code: How can I pass a parameter to a setTimeout() callback?

Community
  • 1
  • 1
Sune S.-T.
  • 225
  • 2
  • 14