-
Notifications
You must be signed in to change notification settings - Fork 811
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
Fix the Unreal Plugin's GetGameServer #1581
Conversation
Build Succeeded 👏 Build Id: c61d9100-9ab1-4eae-84b2-8bc007efaf3f 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:
|
LGTM, interesting that the order of BindLambda needed to change, I don't think we noticed any issues there. But the changes look sensible, especially checking the response code for errors |
@WVerlaek |
@markmandel - how do you feel about squeezing in this bug fix right before the release cut? |
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.
@markmandel - how do you feel about squeezing in this bug fix right before the release cut?
SGTM, let's do it.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dotcom, markmandel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* separate MakeRequest and SendRequest * Handle response code in GetGameServer
What type of PR is this?
/kind bug
What this PR does / Why we need it:
The fix is extremely simple.
first commit :
I think the point where
OnProcessRequestComplete().BindLambda()
afterSendRequest()
is done needs to be fixed.(InGetGameServer()
)https://github.com/googleforgames/agones/compare/master...dotcom:master?diff=split#diff-80dbe36c10c363357ae5c22ced334ea2L151
For implementers who want to add more processing to the Request, I have split the processing.
second commit:
We should check whether the request is acceptable or not with
EHttpResponseCodes::IsOk
I'm separating the commits for two different fixes, but squash them as needed.
I'd like further comments from people who are familiar with UE4.
Thank you.