New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDL_AddTimer doesn't work properly #67

Open
9chu opened this Issue Dec 14, 2018 · 7 comments

Comments

Projects
None yet
4 participants
@9chu
Copy link

9chu commented Dec 14, 2018

SDL_TimerID
SDL_AddTimer(Uint32 interval, SDL_TimerCallback callback, void *param)
{
    return EM_ASM_INT({
        return Browser.safeSetTimeout(function() {
            Runtime.dynCall('iii', $1, [$0, $2]);
        }, $0);
    }, interval, callback, param);
}

SetTimeout only emit the callback once, it should be setInterval.

@kripken

This comment has been minimized.

Copy link
Member

kripken commented Dec 17, 2018

I think that's what the SDL API says, though: that it will be called once?

https://wiki.libsdl.org/SDL_AddTimer

Or maybe I'm misreading that... For reference emscripten's SDL1 support also just calls it once, but maybe that's wrong too...

@9chu

This comment has been minimized.

Copy link
Author

9chu commented Dec 18, 2018

I think that's what the SDL API says, though: that it will be called once?

https://wiki.libsdl.org/SDL_AddTimer

Or maybe I'm misreading that... For reference emscripten's SDL1 support also just calls it once, but maybe that's wrong too...

To be honest, setInterval is still not correct.
Accroding to the SDL manual, the returned value of the timer's callback function indicates the next interval of the timer. If the returned value from the callback is 0, the timer is canceled.

So, I think the correct implementation may be:

// Cause I'm new to emscripten, I wrote it in pure js :)
mapping = {};
next_id = 0;

function SDL_AddTimer(interval, callback, param) {
    var id = next_id++;
    var internal_callback = function(interval) {
        var ret = callback(interval, param);
        if (ret == 0) {
            del mapping[id];
            return;
        }
        mapping[id] = setTimeout(function() { internal_callback(ret) }, ret); 
    };
    mapping[id] = setTimeout(function() { internal_callback(interval) }, interval);
    return id;
}

function SDL_RemoveTimer(id) {
    if (!(id in mapping))
        return false;
    
    clearTimeout(mapping[id]);
    del mapping[id];
    return true;
}
@Daft-Freak

This comment has been minimized.

Copy link
Member

Daft-Freak commented Jan 14, 2019

09b829b should be it.

@Beuc

This comment has been minimized.

Copy link
Contributor

Beuc commented Jan 22, 2019

FYI 09b829b broke RenPyWeb.
Somehow SDL_AddTimer returned 0 while it didn't use to, causing an application error.

In addition there's no SDL_SetError, so I got an unrelated error which was quite confusing.

@Daft-Freak

This comment has been minimized.

Copy link
Member

Daft-Freak commented Jan 22, 2019

Oops, looks like I need to init SDL_TimerData.nextID to 1...

@Daft-Freak

This comment has been minimized.

Copy link
Member

Daft-Freak commented Jan 22, 2019

Or just move two characters... a8a4fde

@Beuc

This comment has been minimized.

Copy link
Contributor

Beuc commented Jan 22, 2019

This appears to work, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment