RobLoach / node-raylib

Node.js bindings for Raylib
https://robloach.github.io/node-raylib/
Other
245 stars 20 forks source link

Suggestions #160

Closed dananderson closed 2 years ago

dananderson commented 2 years ago

I spent some time today getting this package working with https://github.com/lightsourceengine/veil (Node-like runtime written in C99). I got this package working and that support will be in the next release. In that work, I noticed a couple of minor things that can be improved:

  1. Use the C N-API calls, rather than the C++ napi lib. napi adds allocations and wrappers to work with C++. Unless you are using classes, the overhead is not worth it. And, you will probably end up with a smaller .node file.

  2. Consider exporting an ES module. You can setup the package.json to export a CommonJS module and an ES module. The CJS modules are legacy at this point.. and they are difficult to work with. CommonJS also have some loading overhead when imported.

RobLoach commented 2 years ago

Great suggestions, @dananderson!

  1. I believe we used the C++ NAPI because it has a few useful wrappers available to make integration easier. Always open to flipping it out for something better though.
  2. Would love to have an ES module in place. As long as we tag it as a major release so that existing installs don't break :+1:
konsumer commented 2 years ago

We have this issue for esm. I'd also be into C99 NAPI, but I think it would take a little work. We mostly auto-generate bindings, so if you feel up to it, a PR to auto-gen that instead would be much appreciated. We should track that as a separate issue (#161). I propose we close this and track those 2, separately.

dananderson commented 2 years ago

If I have some time next week, I will look into making a PR.

Some other issues I found:

examples/models/models_rlgl_solar_system.js does not work because the rl*() functions are not generated.

All of the examples use a tight while loop, so the node event loop gets starved (never runs). When node runs the main script, it will drain the async jobs and then run the event loop. Since there is a tight loop in the main script, the async jobs and event loop never run. Consider this example:

const r = require('raylib')
const fs = require('fs/promises')

const screenWidth = 800
const screenHeight = 450
r.InitWindow(screenWidth, screenHeight, "raylib [core] example - basic window")
r.SetTargetFPS(60)

fs.readFile(__filename).then(() => { console.log('read the file!') })

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

r.CloseWindow()

The "read the file!" message prints AFTER r.CloseWindow(). That would include all I/O callbacks, timers, etc. Each iteration of the loop should be on a macro task (timer, setImmediate, etc). The tight loop cuts the examples off from much of the node async apis.

Feel free to close, as this was just opened for some discussion.

konsumer commented 2 years ago

I have had issues with this in my own code, like if I try to use callbacks or async code or sometimes even just console.log (see #134). I think the intent is to make them roughly match raylib C examples, but I think we need an alternative (even just as an external addon) in node that works differently (because raylib calls sleep.)

dananderson commented 2 years ago

Ok, this issue can be closed. I can provide some insight into the node event loop in #134