search

zamazan4ik / aws-gamelift-server-sdk-rs

MIT License
5 stars 1 forks source link

Add some useful derives to all of the entities #2

Closed Luminoth closed 2 years ago

Luminoth commented 2 years ago

This largely stems from me wanting to easily print out the objects in the callbacks. I just did a quick pass with clippy and then slapped the common derives on everything in that file. Hopefully that's cool, it'll make my life a lot easier.

zamazan4ik commented 2 years ago

@Luminoth thanks for the PR. I want to provide you with some background regarding this library.

When I've started this crate, my Rust skills were much weaker so I am sure that the current library design is not ideal from Rust's perspective (I've tried my best at that time). So if you have any ideas, what should be reworked or designed somehow in another way - please let me know or even send a PR! Any help is really appreciated.

And one more warning: this crate is not hugely battle-tested yet. I just prepared a simple closed-source proof-of-concept, deployed it to AWS Gamelift, and checked that some functions from the SDK works fine (I didn't perform full coverage). So beware of some errors. If you find any - just let me know :)

zamazan4ik commented 2 years ago

By the way, can you fix the clippy error please?)

Luminoth commented 2 years ago

I'm not sure why that clippy warning wasn't showing up for me locally. Sorry about that, should be fixed now.

I have another change I'm sitting on that I can push after this clears if you'd like. It changes the ProcessParameters callbacks to return futures so they can be awaited directly rather than having to spawn internally.

So far tho, everything has been easy to work with. I've been using your crate to help myself learn more about making use of GameLift without having to deal with standing up a C++ project. It's been really nice, I appreciate that you put it out there for others to make use of.

zamazan4ik commented 2 years ago

I have another change I'm sitting on that I can push after this clears if you'd like. It changes the ProcessParameters callbacks to return futures so they can be awaited directly rather than having to spawn internally.

I think you can send it in another PR and we can review it there. For now, I am not against about such design change :)