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

[Discussion] Assimilate netspeakgames/UnrealAgonesSDK #1683

Closed
domgreen opened this issue Jul 8, 2020 · 8 comments · Fixed by #1739
Closed

[Discussion] Assimilate netspeakgames/UnrealAgonesSDK #1683

domgreen opened this issue Jul 8, 2020 · 8 comments · Fixed by #1739
Labels
kind/breaking Breaking change kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones
Milestone

Comments

@domgreen
Copy link
Contributor

domgreen commented Jul 8, 2020

Would like to open a discussion around moving from the existing Unreal SDK for Agones to one that has been open-sourced by Netspeak over at https://github.com/netspeakgames/UnrealAgonesSDK (full disclosure I am a core contributor).

This version adopts a much more Unreal plugin model for the SDK:

  • uses primitives such as components
  • C++ and blueprint support
  • moves away from ticks to timer delegates
  • all calls return a success and failure delegate (events) to hook in BP or C++

Similarities with existing:

  • wraps the REST API
  • automatically get health ticks (can configure off)
  • automatically connects by calling gameserver then ready (can configure off) then broadcasts the result gs object

Missing Functionalisty:

  • only missing functionality is currently WatchGameServer
  • this is also missing from the existing SDK along with many other calls

This is not backward compatible ...so anyone using the existing SDK would need to alter usage when bringing in this version.

The suggestion would be to gain consensus from Agones maintainers and the Unreal community if this is something that would benefit from being in the main Agones project.

@domgreen
Copy link
Contributor Author

domgreen commented Jul 8, 2020

Might be of interest to @markmandel @drichardson @dotcom @WVerlaek @robbieheywood

@markmandel
Copy link
Member

This is not backward compatible ...so anyone using the existing SDK would need to alter usage when bringing in this version.

What does the migration path look like? Is it particularly hard?

@domgreen
Copy link
Contributor Author

domgreen commented Jul 8, 2020

What does the migration path look like? Is it particularly hard?

Will not be hard, for example say someone has used the old sdk to set a label.

You would remove the old plugin and add the new one as a component (is in docs) and then change the calls (from memory):
Existing:

Hook->SetLabel(TEXT("key"), TEXT("value"));

Netspeak:

AgonesComponent->SetLabel(TEXT("key"), TEXT("value"), successDelegate, errorDelegate);

As there isn't too much API exposed it will not be a big task and both auto-register the health and connect functionality.

The main changes from an engineering perspective are that you now pass in the delegates for success and failure scenarios ... these can be nullified by passing in a empty delegate {} .

@markmandel markmandel added kind/breaking Breaking change kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones labels Jul 8, 2020
@markmandel
Copy link
Member

I think the biggest argument for assimilating this into the core -- this SDK has way more features than ours... 😄 which is a big win.

The other thing is - the old SDK, it's not broken from working, we're just saying that "if you want the cool new things, then you need to upgrade" - which is not a huge ask, I don't think.

Does that resonate with everyone else?

@robbieheywood
Copy link
Contributor

As a user of the existing Unreal SDK, that sounds very good to us 👍 We can continue using the existing SDK and then upgrade when we want to take advantage of the new features.

Also want to say great stuff @domgreen - we love seeing Unreal support within Agones getting even stronger 💯

@domgreen
Copy link
Contributor Author

Will put out a PR for this over the weekend, seems like there are no objections.

@markmandel
Copy link
Member

@domgreen awesome. Sounds good to me for sure!

@dotcom
Copy link
Contributor

dotcom commented Jul 27, 2020

nice code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/breaking Breaking change kind/design Proposal discussing new features / fixes and how they should be implemented kind/feature New features for Agones
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants