-
Notifications
You must be signed in to change notification settings - Fork 823
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
Unreal SDK - Added counters to unreal sdk #3935
Unreal SDK - Added counters to unreal sdk #3935
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Build Succeeded 🥳 Build Id: 01fbce8a-8930-430b-845b-454f9acc8cb4 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😭 Build Id: b480c9f0-481a-4ef5-aaf5-b76e25478735 Status: FAILURE To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 🥳 Build Id: 13a8808e-11a7-4104-9cfe-db4a4cbbce36 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 🥳 Build Id: 74231d6b-3a0a-49cc-b5b1-12f866cbbd87 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
@tvandijck and @KiaArmani as the most recent contributors to the Unreal SDK, would you review this PR? |
Any updates? It would be great to have it in 1.43 :) |
Build Succeeded 🥳 Build Id: 54dafa02-7d35-4c2e-b255-65007ec87795 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
1.43 release is getting cut in a few hours, so it's probably too close for this release. @zifter or @KAllan357 would you be able to test and validate the changes in this PR? |
@igooch I'm working with @GloryOfNight in the same company and these changes are made by my request. I actually can confirm, that it's tested, because we are using those changes currently in production. |
We only using counter decrement. Get/Set Counter is not tested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Found an issue with default initialized content string that by default {} (braces). That caused Get req. to fail.
Tested, found issue with GetCounters() call related to default initialized content string as braces. Fixed it. |
Build Succeeded 🥳 Build Id: 9c33eb73-b894-4fca-8c1c-d50494d4ff84 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for me
Build Succeeded 🥳 Build Id: 9e937153-a4eb-4f57-900c-c909794b5c77 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Any updates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving based on reviews by @KiaArmani and @zifter. Thank you for the contribution!
Build Succeeded 🥳 Build Id: a435d9bc-db09-4eb1-9577-ff4e14b46998 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
What type of PR is this?
/kind feature
What this PR does / Why we need it:
Added counters to unreal sdk
Which issue(s) this PR fixes:
n/a
Closes #3648 (partially)
Special notes for your reviewer:
Validity of http requests not tested.
Build log as requested in CONTRIBUTING.md:
buildWin64Editor.log