RobLoach / node-raylib

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

Use TypeScript #36

Closed adenine-dev closed 4 years ago

adenine-dev commented 5 years ago

Things added

Things left to do

RobLoach commented 5 years ago

Looks pretty neat! To test it, it should just be....

npm it

Would be also be great to remove the JavaScript from the repo, and have a pretest script to compile the typescript.

adenine-dev commented 5 years ago

Yep I am blind my bad, all js files are now ts files with the exception of:

On the note of the test being broken when pulling the repo in its current state both cli tests fail. I don't know if this is an issue with my machine or if it is just broken. Either way everything else is ts and passes.

I also changed the enumerations to be more up to date with the current raylib enums (assuming i didn't miss something)

RobLoach commented 5 years ago

Which version of Node are you on? This is what I get with npm it on Node 11 with master...

-- Building raylib static library
-- Compiling with the flags:
--   PLATFORM=PLATFORM_DESKTOP
--   GRAPHICS=GRAPHICS_API_OPENGL_33
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/node-raylib/build
info CMD BUILD
info RUN cmake --build "/tmp/node-raylib/build" --config Release
[27/27] Linking CXX shared library Release/node-raylib.node
npm notice created a lockfile as package-lock.json. You should commit this file.
added 545 packages from 843 contributors and audited 1826 packages in 39.556s
found 0 vulnerabilities

> raylib@0.1.1 test /tmp/node-raylib
> mocha

  raylib
    window
      ✓ IsWindowReady()
      ✓ IsWindowReady("incorrect", "number", "of", "args")
    audio
      ✓ IsAudioDeviceReady()
    file
      ✓ FileExists()
      ✓ IsFileExtension()
    rlgl
      ✓ LoadText()
    enums
      ✓ KEY_A
      ✓ COMPRESSED_ASTC_4x4_RGBA
    color
      ✓ GetColor()
    camera
      ✓ Camera()
    cli
      ✓ should execute on a script (101ms)
      ✓ should display the help (101ms)

  12 passing (211ms)

I haven't tested this on Mac, or Windows, either. Soooo, it's possible things are broken.

adenine-dev commented 5 years ago

Yes I was on windows running node 12.6.0 and the relevant output is:

  1) raylib
       cli
         should execute on a script:
     Error: spawnSync C:\Users\alexa\Documents\node-raylib\bin\node-raylib ENOENT
      at Object.spawnSync (internal/child_process.js:1041:20)
      at spawnSync (child_process.js:609:24)
      at Object.execFileSync (child_process.js:636:15)
      at Context.<anonymous> (test\test.js:86:33)
      at processImmediate (internal/timers.js:439:21)

  2) raylib
       cli
         should display the help:
     Error: spawnSync C:\Users\alexa\Documents\node-raylib\bin\node-raylib ENOENT
      at Object.spawnSync (internal/child_process.js:1041:20)
      at spawnSync (child_process.js:609:24)
      at Object.execFileSync (child_process.js:636:15)
      at Context.<anonymous> (test\test.js:93:34)
      at processImmediate (internal/timers.js:439:21)
RobLoach commented 5 years ago

Hmm, perhaps executing CLI stuff on Windows is different? The path looks correct.

adenine-dev commented 5 years ago

After way too much tinkering with it I have determined that linux's version of node lets you run node files without the node file.js command, instead just requiring the file.js command, how or why I do not know, however I was able to solve the issue by changing the call from execFileSync to execSync (and changing params accordingly.) This solves the issue on windows however I don't know if it still works on linux.

RobLoach commented 5 years ago

Ah, so you need the node executable. That makes sense. Good tinkering!

Also happy it works on Windows. Had no clue :wink:

adenine-dev commented 5 years ago

Wonderful, and you were able to verify that it works on linux?

RobLoach commented 5 years ago

Strangely, the tests work correctly, but when running the basic window example, I'm getting errors on InitWindow....

bin/node-raylib examples/core/core_basic_window.js 
/home/rob/Documents/raylib-ts/examples/core/core_basic_window.js:18
r.InitWindow(screenWidth, screenHeight, "raylib [core] example - basic window")
  ^

TypeError: r.InitWindow is not a function
    at Object.<anonymous> (/home/rob/Documents/raylib-ts/examples/core/core_basic_window.js:18:3)
    at Module._compile (internal/modules/cjs/loader.js:816:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:827:10)
    at Module.load (internal/modules/cjs/loader.js:685:32)
    at Function.Module._load (internal/modules/cjs/loader.js:620:12)
    at Module.require (internal/modules/cjs/loader.js:723:19)
    at require (internal/modules/cjs/helpers.js:14:16)
    at Object.<anonymous> (/home/rob/Documents/raylib-ts/bin/node-raylib:44:1)
    at Module._compile (internal/modules/cjs/loader.js:816:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:827:10)
adenine-dev commented 5 years ago

This is because it was not using commonjs modules, instead doing whatever typescript does by default. I have fixed this. However there were issues with the functions Vector2, Vector3, etc. being constructors now. I have also fixed this. I also added the rest of the model functions because somehow I didn't see the TODO in models.ts

RobLoach commented 5 years ago

Sent a PR to fix the conflicts with the fixed UpdateCamera() function: https://github.com/alexa-griffin/node-raylib/pull/1

adenine-dev commented 5 years ago

Added that PR after struggling for way to long with github. UpdateCamera now works as expected. The issue was that we forgot to run npm run compile so it was still using the old bindings. I am not sure if it should be in the pretest script because that would slow down testing quite a bit, however it is your call.

Gamerfiend commented 5 years ago

Is this to be merged into master? I'd love to use TypeScript right off the bat :)

RobLoach commented 5 years ago

It's coming along. There are still some concerns I have with the change in its current state...

  1. The diff. If we switch completely over to TypeScript, we shouldn't need the .js files in the repo, which should clean up the diff additions... diff

  2. Duplicate code. Rather than just defining the Raylib functions in the .h files as the Node.js addon, we now have to have to also define them in the .ts files.

  3. Packaging. We should likely compile the Typescript on prepare, and just ship .js files in package.json "files".

As an alternative for now, I wouldn't be opposed to having TypeScript definition files to help TypeScript use "right off the bat" :wink:

Gamerfiend commented 5 years ago

How can I help? Write examples in TypeScript perhaps?

RobLoach commented 5 years ago

That'd be cool! :smiley:

RobLoach commented 4 years ago

Added one example of using TypeScript: https://github.com/RobLoach/node-raylib/blob/master/templates/typescript_game/typescript_game.ts

Also added the types definition to package.json... Mind seeing if it gets picked up correctly? THANKS!

RobLoach commented 4 years ago

The duplicate code may cause this to be difficult to maintain. As an alternative, I've committed the types definition, along with added documentation on how to update it. Tested a bit in VSCode and it seems to be working, but would love more testing!

Going to close this for now, let's discuss more over at https://github.com/RobLoach/node-raylib/issues/35 .