uspgamedev / luasteam

Bindings to connect lua with Steam API
https://luasteam.readthedocs.io/en/stable/
MIT License
116 stars 19 forks source link

Add NetworkingSockets, NetworkingUtils, GameServer #27

Closed zorfmorf closed 11 months ago

zorfmorf commented 1 year ago

I might still add more to this but the current state is tested quite extensively on linux and windows.

Please let me know if you find any issues or want stuff done differently

zorfmorf commented 1 year ago

I've fixed the documentation a bit and added poll group. I think I'm completely done. I might add more if I need it for my project or if anybody finds any bugs or issues.

zorfmorf commented 1 year ago

I've fixed some more documentation issues and cleaned up the code a bit. I also added the missing FlushMessagesOnConnectionand SendMessagesmethods for SteamNetworkingSockets.

By now most of these methods are quite well tested on my side at least and the documentation is complete.

Ismoh commented 1 year ago

Looking forward for approved reviews! Well done and thanks a ton for sharing!

zorfmorf commented 1 year ago

I fixed an issue when trying to accept messages from an invalid poll group or connection handler (just return -1 instead of crashing).

zorfmorf commented 1 year ago

I ended up using all the auth ticket related code in a project of mine, so I fixed an issue with the implementation and added the missing methods on the server side. Works fine on my machine™.

I would appreciate it, if somebody could take the time and motivation to look through this or alternatively promote me to maintainer.

Ismoh commented 1 year ago

I'll have a look, but cannot tell when. Will take a while!

pakeke-constructor commented 1 year ago

Maybe this is bad etiquette, but I am going to shoot one of the maintainers an email about this. It would be a real shame if this wasn't merged. Hopefully they can either review this, or give you maintainer access

yancouto commented 1 year ago

Hello, I'm the main maintainer here, sorry for the huge delay.

Skimming this looks ok, but it's hard to review because it changed the steam API version and that breaks all CI (used for testing and uploading releases) and I need to fix it (which involves some hacky private repos). It's also possible Travis has changed in the meantime, as it often does, but I haven't checked that.

I'm travelling currently and don't even have my usual setup, so I am not sure I can review this in the next couple weeks.

yancouto commented 11 months ago

I have merged this, and will soon do a release. @zorfmorf if you want to do further changes lmk, the review shouldn't take long now. Thanks for all of this!! 😎

zorfmorf commented 11 months ago

@yancouto Thanks for looking through this, I appreciate it a lot and I know it took a lot of time!

Sorry for the messy commit history, I don't know why I didn't rebase on my own actually. I'll be sure to create a cleaner history the next time.

I am certain that the methods work as they are supposed to, however I am not a C++ expert, so I would not be surprised if I introduced a memory leak somewhere! I am travelling right now but I'll look into it again in the next few days and make sure that everything is alright to the best of my abilities.

Yes, I decided not to introduce specific string return values in some situations where you would call the method very often. I am not sure if this actually helps performance wise, so maybe I should have just stayed consistent.

Ismoh commented 11 months ago

Awesome! Thanks so much for implementing and reviewing!

yancouto commented 11 months ago

FYI the build is working on Travis but the release uploading is not, as travis encrypt is broken for me. I contacted their support and will fix it when possible. (Ideally I would move everything to Github Actions but I think Windows will be tricky, as it was pretty tricky to initially get it working on Travis.)

Ismoh commented 11 months ago

If you want to use GitHub actions, feel free to look into NoitaMP workflows. Feel free to use it, but please add credits.

Important part might be vsvarall.bat depens on how you build stuff, but everything should be available on windows then. stackoverflow

Fyi: I had to do it 'manually', because of modifications to dlls, but there a plenty marketplace actions to build stuff out of the box. If I can help, give me a shout. Preferably on discord.

Edit/fyi2: Never used travis.

pakeke-constructor commented 11 months ago

Thank you for all the great work Yancouto! <3

yancouto commented 11 months ago

I finally managed to fix CD and the latest release is working and has all the libraries uploaded!

This is fully done.