-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Update to latest library versions #3
Conversation
secureStationURL.SetSID(1) | ||
secureStationURL.SetStream(10) | ||
secureStationURL.SetType(2) | ||
secureStationURL.SetPortNumber(uint16(port)) |
Check failure
Code scanning / CodeQL
Incorrect conversion between integer types High
strconv.Atoi
Incorrect conversion of an integer with architecture-dependent bit size from
strconv.Atoi
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.
Code reads OK. Server-side parsing of the metabinary is a bit weird but if the game needs it... LGTM
metaBinary.CreationTime.FromTimestamp(time.Unix(creationTimestamp, 0)) | ||
metaBinary.UpdatedTime.FromTimestamp(time.Unix(updatedTimestamp, 0)) | ||
metaBinary.ReferredTime.FromTimestamp(time.Unix(referredTimestamp, 0)) | ||
metaBinary.ExpireTime.FromTimestamp(time.Unix(expireTimestamp, 0)) |
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.
I still think this is silly behavior for this function. Code's valid though
tags[i] = string(tag) | ||
} | ||
|
||
extraDatas := make([]string, len(dataStorePreparePostParam.ExtraData)) |
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.
Might want an upper-bound length check here?
That's just what the original code did, nothing in its behavior has been changed or removed. There is room for improvement once we get common DataStore, but that is outside the scope of the migration (This also answers the review comments) |
Resolves #1
Changes:
Needs more testing, seems to have issues