Skip to content
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

async addon #134

Open
konsumer opened this issue Apr 22, 2022 · 15 comments
Open

async addon #134

konsumer opened this issue Apr 22, 2022 · 15 comments

Comments

@konsumer
Copy link
Collaborator

konsumer commented Apr 22, 2022

I was thinking about how prevalent async control-structure is in nodejs (callbacks, promises, async/await), and how weird it gets if you use the standard SetTargetFPS/WindowShouldClose loop, and how I could make it still feel very "raylib ish" but also work well, and I came up with this:

import r from 'raylib'
import { MaintainFPS } from 'raylib/addons/async.js'

const screenWidth = 800
const screenHeight = 450

r.InitWindow(screenWidth, screenHeight, 'raylib [async] example - async game-loop')

while(await MaintainFPS(60)) {
  r.BeginDrawing()
  r.ClearBackground(r.RAYWHITE)
  r.DrawText('Congrats! You created your first window!', 190, 200, 20, r.LIGHTGRAY)
  r.EndDrawing()
}

r.CloseWindow()

I think if MaintainFPS resolved to current real FPS, that might be cool, and serve the double-function of getting FPS and also returning a number/false (to close.)

I couldn't put this in our own project, in an example, because it mixes cjs and mjs, but I use it here, since it makes top-level async possible (so simpler syntax.) In a normal es6 node project, it would work fine.

If this seems like a sensible feature, I will make a PR for it.

Related: #130

@konsumer konsumer changed the title async async addon Apr 22, 2022
@konsumer
Copy link
Collaborator Author

konsumer commented Apr 23, 2022

I started working on idea here and made @RobLoach and @twuky collabs. Current repo illustrates issue where the external-computed & raylib FPS seem to disagree. If you run it, you will see different numbers for real FPS vs DrawFPS, and it will show TICK on console every second (to show that the timeouts are right.) Maybe it's ok to just use a seperate FPS, if you need that. This works great otherwise, but it might be weird if someone is trying to do DrawFPS.

I tried several variations, like moving the await MaintainFPS(60) inside the loop (before/after BeginDrawing/EndDrawing) and even making a totally different style loop that uses setTimeout and none seemed to make a difference:

let i = 0
const getWaitTime = targetFPS => {
  const timetarget = 1 / targetFPS
  const timeframe = r.GetFrameTime()
  const waittime = (timetarget - timeframe) * 2000
  console.log(i++ % targetFPS === 0 ? 'TICK' : '')
  return waittime > 0 ? waittime : 0
}

function draw () {
  if (r.WindowShouldClose()) {
    r.CloseWindow()
  } else {
    r.BeginDrawing()
    r.ClearBackground(r.RAYWHITE)
    r.DrawText('Congrats! You created your first window!', 190, 200, 20, r.LIGHTGRAY)
    r.DrawFPS(10, 10)
    r.EndDrawing()
    setTimeout(draw, getWaitTime(60))
  }
}

draw()

Here is what seems to be the correct TICK timing, but it shows 400-1000 in DrawFPS:

2022-04-22 17 28 15

@konsumer
Copy link
Collaborator Author

I will probably use this anyway, in my own project, because it makes things work right (like console.log and async input callbacks) but it may not be a good thing to add to node-raylib, I'm not sure.

@RobLoach
Copy link
Owner

WindowShouldClose() already handles this. Given you use SetTargetFPS(0)... Albeit it's not async, but you can see that it calls glfwWaitEvents() on desktop, and emscripten_sleep() on web.... I guess that's what you're trying to achieve is have the game loop become async?

You can just have a game loop with SetTargetFPS(0). SetTargetFPS() will make it so that the loop is only called once every x seconds. Like you said, I don't believe this should be part of the core raylib package, but could of course live elsewhere!

@konsumer
Copy link
Collaborator Author

I tried with SetTargetFPS(0) and without and it had the same results. I mean, it works, but it thinks it's a different FPS than it really is. It effects other things, like music, too. I noticed that setting vsync gets it closer, at least on my desktop:

r.SetConfigFlags(r.FLAG_VSYNC_HINT)
r.InitWindow(320, 240, 'demo')

@konsumer konsumer mentioned this issue Aug 18, 2022
@dananderson
Copy link

Control of the node event loop is limited. The loop must be activated with macro tasks or it will sleep. Micro tasks are aggressively drained. raylib can hit vsync, which is effectively a sleep. And, raylib has it's own sleep logic in it's main loop. The reality is that the main loop will not be perfect. Here is how I would approach it:

The node event loop should look like this, running at the desired FPS.

  • macrotask: run one raylib frame
  • node event loop micro task and macro task processing

The raylib frame sleep time calculation would be considering the node event processing time. Not ideal, but it's what we got. The node application should not need to do frame time calculations for sleep. The node application should use setImmediate() or setTimeout(0) to schedule the next frame.

The user shouldn't have to worry about this too much.

Here is a very rough sketch of what the api could look like. Make the node specific behavior clear without changing the raylib C apis.

// this object/class provided by node-raylib
const MainLoop = {
  run() {
    this.onInit()
    this.runFrame()
  }

  runFrame() {
    if (r.WindowShouldClose()) {
     this.onClose()
    }
    this.onFrame()
    setImmediate(() => this.runFrame())
  }
}

// user fills in events

MainLoop.onInit = () =>  {
  r.InitWindow(screenWidth, screenHeight, "raylib [core] example - basic window")
}

MainLoop.onFrame = () => {
  r.BeginDrawing();
  r.ClearBackground(r.RAYWHITE)
  r.DrawText("Congrats! You created your first node-raylib window!", 120, 200, 20, r.LIGHTGRAY)
  r.EndDrawing()
}

MainLoop.onClose = () => {
  r.CloseWindow()
}

MainLoop.run()

@konsumer
Copy link
Collaborator Author

konsumer commented Aug 18, 2022

await MaintainFPS(60) works using this, raylib just tracks the wrong FPS. I think this would still have the same problems with FPS, and I'm not sure it improves on that simple (kind of like raylib, but async) API.

Along similar lines, this seems simpler, but I still think I like my MaintainFPS better:

// IMPLEMENTATION
function mainLoop (update, exit) {
  if (r.WindowShouldClose()) {
    exit()
  } else {
    update()
    setImmediate(() => mainLoop(update, exit))
  }
}

// USAGE

r.InitWindow(screenWidth, screenHeight, "raylib [core] example - basic window")

const update = () => {
  r.BeginDrawing()
  r.ClearBackground(r.RAYWHITE)
  r.DrawText("Congrats! You created your first node-raylib window!", 120, 200, 20, r.LIGHTGRAY)
  r.EndDrawing()
}

const close = () => r.CloseWindow()

mainLoop(update, close)

It should also be noted these just run as fast as they can within the event-loop, there is no framerate-limiting or anything.

@twuky
Copy link
Collaborator

twuky commented Aug 18, 2022

Its possible to call a callback function from the native addon. So in theory we could create a promise in the JS wrapper for the function, pass promise.resolve into the native addon and have it resolve it after whatever raylib call needs to be done.

looks like this may be a good example using the c99 napi?
https://github.com/theapi/napi_async_promise/blob/master/src/addon.c

@dananderson
Copy link

In order for the node application to continue running, it will need a macro task registered. The while loop in the examples needs to be broken up into slices, which will require a callback. There are lots of different ways to express this in API, but let's make sure we understand the problem.

The secondary issue with MaintainFPS is that it is an async function. With how the examples are structured, it would need to be implemented with a top level await, which is not supported by CommonJS / require.

The main issue with MaintainFPS is it does not work. The function returns a resolved promise, via return, or an unresolved promise, via a wrapped timeout. When index.js is evaluated, it will get to MaintainFPS and a promise will be returned. The await will halt javascript execution. If the promise is not resolved, node will return to processing it's main uv loop. If the promise is resolved, a job will be put on the micro task queue. Node will drain the micro task queue before returning to the uv loop. That means JS execution will continue in index.js at that await point. (If more items are added to the micro task queue, they must be drained before returning to the uv loop.)

The comments in MaintainFPS mention a fast and slow behavior. Sometimes the loop is executed back to back when draining the micro task queue and other times the setTimeout gets the macro task queue involved. The raylib FPS calculations will be sporadic because the frame is happening in different places. And, MaintainFPS introduces a wait using the time of the previous frame. All of this would result in the fast/slow frame behavior.

I think I understand the node side of things, but the node issues could be masking something at a lower level.

Native callbacks will not fix the issue.

@dananderson
Copy link

Also, if top level awaits were possible, something like this would work:

r.WindowShouldCloseAsync = () => new Promise((resolve, reject) => setImmediate(() => r.WindowShouldClose()))

while (await r.WindowShouldCloseAsync()) {
  // do frame
}

@konsumer
Copy link
Collaborator Author

konsumer commented Aug 18, 2022

In order for the node application to continue running, it will need a macro task registered. The while loop in the examples needs to be broken up into slices, which will require a callback. There are lots of different ways to express this in API, but let's make sure we understand the problem.

I'm not sure I follow. Who says we "need a macro task registered" or it "needs to be broken up into slices"? I'm not really against implementing it either way, but I like the shape of my API better. Maybe it's just because I don't understand the issue with my approach, but it seems like an overly-complicated way to do roughly the same thing, with 3 callbacks instead of none.

The main issue with MaintainFPS is it does not work.

My example works fine and other async tasks running alongside it seem to work ok. How does it not work for you?

The secondary issue with MaintainFPS is that it is an async function. With how the examples are structured, it would need to be implemented with a top level await, which is not supported by CommonJS / require.

top-level await for esm is fine, and you can wrap in async function in CommonJS. This seems like a non-issue to me, similar to saying "fetch requires top-level awaits so we shouldn't use it". A generator or callback could also be used, if promises are a sticking point, but I think it's much more ergonomic using promises.

Here it is for commonjs:

const r = require('raylib')
const { MaintainFPS } = require('./async.js')

async function demo () {
  r.InitWindow(800, 450, 'raylib [async] example - async game-loop')
  
  while (await MaintainFPS(60)) {
    r.BeginDrawing()
    r.ClearBackground(r.RAYWHITE)
    r.DrawText('Congrats! You created your first window!', 190, 200, 20, r.LIGHTGRAY)
    r.EndDrawing()
  }

  r.CloseWindow()
}

demo()

Also, if top level awaits were possible, something like this would work:

This doesn't maintain a specific FPS though. Using SetTargetFPS makes the node-side code blocking (it uses sleep and freezes event-loop.)

@dananderson
Copy link

Who says we "need a macro task registered" or it "needs to be broken up into slices"?

The node event loop that's who.

In fact, I will clearImmediate() on this conversation and my process will exit(). Good luck!

@konsumer
Copy link
Collaborator Author

konsumer commented Aug 19, 2022

In fact, I will clearImmediate() on this conversation and my process will exit().

@dananderson Hmm, obviously that is up to you, but I am really just trying to understand why an object with onInit/onFrame/onClose is better. My single-function seems to work well with async (and not break anything with node event-loop) and is simpler to use, so again, I'm not at all against doing it your way, but I need to understand why it's better before I can say "yep, that is the way to go." I have no ego-attachment to my dumb little thing (neither of our solutions is very complicated) just an interest in making the API as easy to use as it can be, and still do what it needs to.

@RobLoach
Copy link
Owner

I think having a WindowShouldCloseAsync() or similar async function in node-raylib directly would be super worth it! 👍

@konsumer
Copy link
Collaborator Author

I think having a WindowShouldCloseAsync() or similar async function in node-raylib directly would be super worth it! 👍

Agreed. Originally we talked about it as a totally separate thing, but I could see it being really useful as part of node-raylib. I think the main problem is that both solutions discussed here don't correctly calculate FPS, and mess up other timing things (like music streaming) so those functions would also need to be adjusted.

@RobLoach
Copy link
Owner

Definitely. Was originally thinking since it's outside the implemntation of raylib's core functionality, it does make sense to have it directly in there, since it does make async things work "properly".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants