samdze / playdate-nim

Nim bindings with extra features for the Playdate SDK
66 stars 3 forks source link

ADD setFinishCallback to SamplePlayer #64

Closed ninovanhooff closed 8 months ago

ninovanhooff commented 8 months ago

Please check whether I handled the userdata pointer right.

Tested with a proc and with nil (to-unset) from the game's perspective. Did not test whether it was properly cleaned up in the files changed in this repo.

Sorry for the messy git history. Removing those .idea files from all branches trips me up

ninovanhooff commented 8 months ago

Hmm, I get this warning when compiling

CC: ../../playdate-nim/src/playdate/sound.nim
/Users/ninovanhooff/.cache/nim/wheelsprung_r/device/@m..@s..@splaydate-nim@ssrc@splaydate@ssound.nim.c: In function 'setFinishCallback__OOZOOZplaydate45nimZsrcZplaydateZsound_1589':
/Users/ninovanhooff/.cache/nim/wheelsprung_r/device/@m..@s..@splaydate-nim@ssrc@splaydate@ssound.nim.c:837:132: warning: passing argument 2 of 'playdate__OOZOOZplaydate45nimZsrcZplaydateZbindingsZapi_30->sound->sampleplayer->setFinishCallback' from incompatible pointer type [-Wincompatible-pointer-types]
  837 |   (*(*(*playdate__OOZOOZplaydate45nimZsrcZplaydateZbindingsZapi_30).sound).sampleplayer).setFinishCallback((*this_0).Sup.resource, privateFinishCallback__OOZOOZplaydate45nimZsrcZplaydateZsound_1427, NIM_NIL);
      |                                                                                                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                                                                                                    |
      |                                                                                                                                    void (*)(void *, void *)
/Users/ninovanhooff/.cache/nim/wheelsprung_r/device/@m..@s..@splaydate-nim@ssrc@splaydate@ssound.nim.c:837:132: note: expected 'void (*)(SoundSource *, void *)' {aka 'void (*)(struct SoundSource *, void *)'} but argument is of type 'void (*)(void *, void *)'
CC: ../../../.choosenim/toolchains/nim-1.6.16/lib/pure/parsejson.nim
CC: ../../../.choosenim/to
samdze commented 8 months ago

Going to check this, from the first tests I get a compiler error:

/Users/sam/.nimble/pkgs/playdate-0.13.0/playdate/sound.nim:235:157: error: incompatible function pointer types passing 'void (void *, void *)' to parameter of type 'sndCallbackProc *' (aka 'void (*)(struct SoundSource *, void *)') [-Wincompatible-function-pointer-types]
                (*(*(*playdate__OOZOOZOOZOOZOOZOnimbleZpkgsZplaydate4548O4951O48ZplaydateZbindingsZapi_30).sound).sampleplayer).setFinishCallback((*this_0).Sup.resource, privateFinishCallback__OOZOOZOOZOOZOOZOnimbleZpkgsZplaydate4548O4951O48ZplaydateZsound_1396, NIM_NIL);
                                                                                                                                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

This is trying to use setFinishCallback like this (should be supported):

samplePlayer.setFinishCallback(
    proc(userData: pointer) =
        playdate.system.logToConsole("Sound finished playing.")
)

I'll investigate a bit. In the meantime can you try squashing this PR into a single clean commit? Would be easier to deal with

samdze commented 8 months ago

Also note that this feature can be implemented using a different approach too, avoiding tables. Check out how the menu item callbacks are implemented in system.nim. The userdata pointer is leveraged to bridge the C callback to the Nim one.

Should be pretty much the same as what you're trying to do here.

ninovanhooff commented 8 months ago

Replaced by #67