Scorbeoards

[ninovanhooff]

https://help.play.date/catalog-developer/scoreboard-api/#pdscore

[ninovanhooff]

This PR serves as a PSA that this feature is being worked on.

I decided to create a branch on the main repo so it's easier to collaborate if needed

[ninovanhooff]

I got stuck calling the binding function with this error:

/Users/ninovanhooff/PlaydateProjects/playdate-nim/src/playdate/scoreboards.nim:31:80: error: incompatible function pointer types passing 'void (PDScore *, NCSTRING)' (aka 'void (PDScore *, char *)') to parameter of type 'PersonalBestCallback' (aka 'void (*)(PDScore *, const char *)') [-Wincompatible-function-pointer-types]
   31 |         nimln_(31);     result = (*this_p0).getPersonalBest(nimToCStringConv(boardID_p1), _ZN11scoreboards26invokePersonalBestCallbackE3ptrIN11scoreboards10PDScoreRawEE7cstring);      if (NIM_UNLIKELY(*nimErr_)) goto BeforeRet_;    goto BeforeRet_;
      |                                                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
Error: execution of an external compiler program 'clang -c -w -ferror-limit=3 -DTARGET_EXTENSION=1 -Wall -Wno-unknown-pragmas -Wdouble-promotion -I/Users/ninovanhooff/Developer/PlaydateSDK/C_API -arch x86_64 -arch arm64 -DTARGET_SIMULATOR=1 -Wstrict-prototypes -g   -I/Users/ninovanhooff/.choosenim/toolchains/nim-2.0.10/lib -I/Users/ninovanhooff/PlaydateProjects/playdate-nim/playdate_example/src -o /Users/ninovanhooff/.cache/nim/playdate_example_d/simulator/@m..@s..@ssrc@splaydate@sscoreboards.nim.c.o /Users/ninovanhooff/.cache/nim/playdate_example_d/simulator/@m..@s..@ssrc@splaydate@sscoreboards.nim.c' failed with exit code: 1

From the example, I called it like this:

discard playdate.scoreboards.getPersonalBest(
            "hills",
            proc(score: PDScore, errorMessage: string) =
              discard
)

[samdze]

You may have to use ConstChar to fix this.
There are a few examples in the codebase, here's one:

playdate-nim/src/playdate/graphics.nim

Lines 216 to 222 in 1a752d3

	proc newBitmapTable*(this: ptr PlaydateGraphics, path: string): LCDBitmapTable {.raises: [IOError]} =
	privateAccess(PlaydateGraphics)
	var err: ConstChar = nil
	var bitmapTable = this.loadBitmapTable(path, addr(err))
	if bitmapTable == nil:
	raise newException(IOError, $err)
	return LCDBitmapTable(resource: bitmapTable)

[ninovanhooff]

That definetly worked.

I now have the issue that a call to addScore works on sim but crashes device, which makes it hard to debug. Even when I replace the entire content of invokeAddScoreCallback by discard, device still crashes.
The crash happens a few seconds after calling addScore. In other words: most probably when the callback is called.

Edit: crash does not happen on 2.5.0 firmware; note that this crash is from fw 2.6.0-beta4

--- crash at 2024/10/19 19:41:52---
build:be97134e-2.6.0-beta.4-buildbot
   r0:600144b1    r1:00000001     r2:00000000    r3: 0043f0e9
  r12:00000000    lr:0805393f     pc:08055cd4   psr: 210f0000
 cfsr:01000000  hfsr:40000000  mmfar:00000000  bfar: 00000000
rcccsr:00000000
heap allocated: 2167168

[ninovanhooff]

@Nycto thanks for the review. I cleaned it up a bunch. Enjoy ;-p

[Nycto]

callback shouldn't be a proc in this situation -- It makes your callsites more complicated than they need to be (and also incurs an unnecessary memory allocation because Nim has to create a lambda). You can also get rid of the emptyValue parameter by using Nim's default proc.

Here is what I mean:

template invokeCallback(callbackSeqs, value, errorMessage, freeValue, builder: untyped) =
  let callback = callbackSeqs.pop()
  if value == nil and errorMessage == nil:
      callback(default(typeof(builder)), "Playdate-nim: No value provided for callback")
      return

  if value == nil:
    callback(default(typeof(builder)), $errorMessage)
    return

  try:
    let nimObj = builder
    callback(nimObj, $errorMessage)
  finally:
    freeValue(value)

Then your callsites will look like this:

proc invokePersonalBestCallback(score: PDScorePtr, errorMessage: ConstChar) {.cdecl, raises: [].} =
  invokeCallback(privatePersonalBestCallbacks, score, errorMessage, playdate.scoreboards.freeScore):
    scoreBuilder(score)

[ninovanhooff]

Couldn't quite figure this out, maybe missing a piece of the puzzle here.

"callback shouldn't be a proc in this situation"

What should they be then? Feel free to create a PR to this branch with the solution if that's easier, or directly commit to the branch if you are sure of it.

[ninovanhooff]

@Nycto Thanks again fr the feedback.

I couldn't get this suggestion to compile. Can you fix it yourself?

#82 (comment)

[Nycto]

Alright -- I published a version that uses untyped instead of an explicit closure proc.

I also had another realization while making this change. For the callbacks, what we actually have here is an Either monad. How would you feel about updating the callbacks to be this:

type
    # ...
    PDResultKind* = enum PDResultSuccess, PDResultError

    PDResult*[T] = object
        case kind*: PDResultKind
        of PDResultSuccess: value: T
        of PDResultError: message: string

    PersonalBestCallback* = proc(result: PDResult[PDScore]) {.raises: [].}
    AddScoreCallback* = proc(result: PDResult[PDScore]) {.raises: [].}
    BoardsListCallback* = proc(result: PDResult[PDBoardsList]) {.raises: [].}
    ScoresCallback* = proc(result: PDResult[PDScoresList]) {.raises: [].}

[Nycto]

I decided to just make the change and push so you can see what it looks like in context. If you prefer the dual argument version let me know and I'll pop this commit off the branch.

[ninovanhooff]

Looks good to me from my phone, will have another look tomorrow. Anything you want to see changed? Otherwise please approve it and I'll merge it

[Nycto]

Done ✔️