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

Kickstart this repo #1

Open
1 of 4 tasks
insraq opened this issue Feb 1, 2022 · 18 comments
Open
1 of 4 tasks

Kickstart this repo #1

insraq opened this issue Feb 1, 2022 · 18 comments

Comments

@insraq
Copy link

insraq commented Feb 1, 2022

Following the discussion here greenheartgames/greenworks#306 , here's the plan to kickstart this repo.

  • Move CI to use this repo @Armaldio
  • Check compatibility with Steamworks SDK v1.53a on Windows @insraq
  • Move missing stuff from greenheartgames/greenworks @insraq (Maybe @Spacetech can help as well)
  • Clean up documentations, add TypeScript definitions (there's already one) - volunteers?

If during the above work, anyone feels that greenheartgames/greenworks is a better base to work on instead of this fork, we can switch to that one as well.

@Armaldio
Copy link
Member

Armaldio commented Feb 1, 2022

I can convert every js file to a typescript file

@Spacetech
Copy link

Happy to see things moving forward 👍

My 2 cents regarding which fork to work off of: It really depends on what folks are more comfortable doing.

The main reason to go with mine is that the NAPI conversion is done and it's compatible with Steamworks v1.53a (currently shipping in my games depots on all OS's).
However the fork is old so a lot of the changes done in greenworks are missing in it. For example, reorganizing the large api file into separate ones is not in my fork. There's also various apis that are missing.

So the choices are:

  1. Reorganize my fork and get it up to date with the existing greenworks
    1. General file reorganization - split the api file into separate parts, like what was done in the original repo.
    2. Go through greenworks commits and find what apis are missing. To stay organized, we could file issues for each of the missing things.
  2. Migrate the existing greenworks repo to NAPI
    1. This would involve updating bindings in every file, updating how the async workers work, etc.

For option 1 - Once the main api file is split into multiple files, it should involve a bunch of smaller, mostly separate changes, where we re-implement the missing apis.
For option 2 - I kinda view this as a single large work item. It takes time and you need to know how NAPI / node bindings work.

My view: I'm not interested in doing the NAPI conversion a second time 😓 , so someone else should take up the mantle for that if option 2 is chosen. I'm happy to code review it though!

@Armaldio
Copy link
Member

Armaldio commented Feb 2, 2022

I think we should also move this issue to discussions and use projets to plan things

@Gurigraphics

This comment has been minimized.

@Spacetech

This comment has been minimized.

@Gurigraphics

This comment has been minimized.

@Spacetech

This comment has been minimized.

@Gurigraphics

This comment has been minimized.

@Armaldio
Copy link
Member

Armaldio commented Feb 2, 2022

@Spacetech @Gurigraphics Please, open a separate issue, thanks

@Armaldio
Copy link
Member

Armaldio commented Feb 5, 2022

So, guys, what are you working on ?
I've started working on the CI

Can I have some feedback here

@ceifa
Copy link

ceifa commented Feb 14, 2022

What do you guys think about add greenworks on npm? So we can have a better version management and add files to be ignored on publish.

@sleepingpandagames
Copy link

sleepingpandagames commented Feb 14, 2022

What do you guys think about add greenworks on npm? So we can have a better version management and add files to be ignored on publish.

I do believe this is a really cool idea, but I'm not sure if Valve will allow that.

I asked on the Steamworks forum if it would be okay to host SDK files on a public repository.
Here is the link to the post.

I'll keep an eye on this.
If I still have no anwser from Valve after a few days, then I'll contact them directly.

In the meantime, @Armaldio, if you want I can send you the latest SDK versions you need via e-mail.

@Armaldio
Copy link
Member

Armaldio commented Feb 15, 2022

  1. Publishing greenworks on npm mean only publishing .node addons & .js files. SDK will not be available.
    SDK is only used to build the .node files

  2. Greenworks is already used on npm. We either have to contact the owner to take ownership or to publish to @greenworks/greenwork (prefered for consistency with GH org)

In the meantime, @Armaldio, if you want I can send you the latest SDK versions you need via e-mail.

It seems a regular account (which I have) is able to download the steamworks SDK, so I have the files ;)

@insraq
Copy link
Author

insraq commented Mar 6, 2022

Sorry for the lack of updates. I have just had a chance to skim through the whole codebase - it seems that Spacetech's fork is really old and is missing several APIs (and have some extra). I wonder whether it makes sense to move forward with Greenworks's repo instead?

@Armaldio Apart from a newer (and maybe better) API, are there other benefits of using NAPI instead of NAN, especially w.r.t CI setup? For me, I am inclined to add the missing feature onto Greenwork's repo and postpone the migration to NAPI - after all, most game developers won't benefit directly from the API migration and it's a lot of extra work.

@Armaldio
Copy link
Member

Armaldio commented Mar 6, 2022

Isn't the missing APIs are a matter of copy pasting ?

  1. I have already put some time on this repo to migrate and improve CI
  2. I think it's better to add missing APIs directly to NAPI instead of migrating them later
  3. People will not have to worry about compiled addon version with NAPI
    That's the whole point. With the old nan, I have to compile to multiple versions, the setup is hard and some versions doesn't compile at all or require hacks. Also, only LTS/supported versions of runtimes are supported. With NAPI, wider ranger of versions are supported. Even deprecated runtimes.

@insraq
Copy link
Author

insraq commented Mar 8, 2022

I see. So NAPI does provide a much simpler CI setup.

I have compared these two codebases - Spacetech's fork is very dated and these two forks diverge a lot. So trying to reconcile the difference seems to be a lot of work. Adding missing API is not simply "a matter of copy paste" unfortunately. If moving to NAPI is a must, then there are two options:

  • Adding the missing API to this repo and bringing it to feature parity with Greenworks' codebase
  • Continuing with Greenworks' codebase and migrating to NAPI

Both are quite a lot of work and will take time to get there - I was thinking in the meantime, does it make sense to start to add missing features to Greenwork's repo so that game developers would have access to the new Steamworks API in the near future?

@Armaldio
Copy link
Member

Armaldio commented Mar 8, 2022

Adding missing API is not simply "a matter of copy paste" unfortunately.

😞


I'm not gonna work on c++ parts, so in the end, it's your call

@Gurigraphics
Copy link

Both are quite a lot of work and will take time to get there

I'm testing FFI-NAPI. This is 50x simpler
https://github.com/Gurigraphics/ffi-napi-steam-js/blob/main/index.js
However, getting the "steam callback" and "steam overlay" to work is still a mystery.

Have you guys already tested this?

I've seen in the benchmarks that FFI is a little slower.
But not everything requires a lot of performance.
And if it worked, it would be a lot less work.

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

6 participants