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

CountsAndLists: SDK Reference #3537

Merged
merged 7 commits into from
Dec 13, 2023
Merged

CountsAndLists: SDK Reference #3537

merged 7 commits into from
Dec 13, 2023

Conversation

Kalaiselvi84
Copy link
Contributor

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it:

Which issue(s) this PR fixes:

Closes #

Special notes for your reviewer:

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 5410ef39-9ad6-4675-be6e-1244043c0cf1

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3537/head:pr_3537 && git checkout pr_3537
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.37.0-dev-fce5d20-amd64

Copy link
Collaborator

@igooch igooch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this -- it's a lot of SDK methods! This review is split over two different commits (I didn't see a quick way to move comments on the first commit to head on the PR). Most of the suggestions are minor things around consistency in formatting and return values. Let me know if you have any questions!


### Alpha().GetCounterCount(key)

This function retrieves the current count for a specified Counter in the game server. It ensures the count is accurate based on the predefined keys in the GameServer resource, and returns an error if the key is not predefined.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The count here behaves different than with Player Tracking. For Counters and Lists changes can be made via Game Server Allocation Actions in addition to through the SDK, so this Count may not always be accurate as it could be reading cached data. The count will be eventually consistent when coming back to the SDK. (This is noted in #2716 as a design trade off.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Included this info in the Note section.👍🏻


### Alpha().SetListCapacity(key, amount)

This function sets the capacity for a specified List, identified by its key (name), with the capacity value required to be between 0 and 1000. It returns an error if the key wasn't predefined in the GameServer resource. The function returns `true` if the capacity is successfully set, and an error if the operation fails.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function sets the capacity for a specified List, identified by its key (name), with the capacity value required to be between 0 and 1000. It returns an error if the key wasn't predefined in the GameServer resource. The function returns `true` if the capacity is successfully set, and an error if the operation fails.
This function sets the capacity for a specified List, identified by its key (name), with the capacity value required to be between 0 and 1000. It returns an error if the key wasn't predefined in the GameServer resource. The function returns `true` if the capacity is successfully set, or `false` and an error if the operation fails.


### Alpha().SetCounterCount(key, amount)

This function sets the count to a given value. This operation overwrites any previous values and the new value cannot exceed the Counter's capacity. The function returns `true` if successful, along with an `error` if the operation fails.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function sets the count to a given value. This operation overwrites any previous values and the new value cannot exceed the Counter's capacity. The function returns `true` if successful, along with an `error` if the operation fails.
This function sets the count to a given value. This operation overwrites any previous values and the new value cannot exceed the Counter's capacity. The function returns `true` if successful, or `false` along with an `error` if the operation fails.


### Alpha().IncrementCounter(key, amount)

This function increments the count of a specified Counter by a given nonnegative amount. It updates the current CRD value and maxes out at the maximum value of `int64`. The function returns an `error` if the key wasn't predefined in the GameServer resource and returns `false` if the Counter is already at capacity, indicating no increment will occur. There is a potential race condition when count values are set from both the SDK and through the K8s API(Allocation or otherwise), since the SDK append operation back to the CRD value is batched asynchronous any value incremented past the capacity will be silently truncated.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function increments the count of a specified Counter by a given nonnegative amount. It updates the current CRD value and maxes out at the maximum value of `int64`. The function returns an `error` if the key wasn't predefined in the GameServer resource and returns `false` if the Counter is already at capacity, indicating no increment will occur. There is a potential race condition when count values are set from both the SDK and through the K8s API(Allocation or otherwise), since the SDK append operation back to the CRD value is batched asynchronous any value incremented past the capacity will be silently truncated.
This function increments the count of a specified Counter by a given nonnegative value amount. The function returns `false` and an `error` if the key wasn't predefined in the GameServer resource and returns or if the Counter is already at capacity, indicating no increment will occur. If the update is successful this returns `true`.
There is a potential race condition when count values are set from both the SDK and through the K8s API(Allocation or otherwise), since the SDK update operation back to the CRD value is batched asynchronous any value incremented past the capacity will be silently truncated to capacity.


### Alpha().GetCounterCount(key)

This function retrieves the current count for a specified Counter in the game server. It ensures the count is accurate based on the predefined keys in the GameServer resource, and returns an error if the key is not predefined.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function retrieves the current count for a specified Counter in the game server. It ensures the count is accurate based on the predefined keys in the GameServer resource, and returns an error if the key is not predefined.
This function retrieves the current count for a specified Counter in the game server. This returns `-1` an `error` if the key is not predefined.


This function removes a string from a List's values using its key(name) and the specific string value. It returns an
`error` if the string does not exist in the list or if the key wasn't predefined in the GameServer resource. The function
returns `true` if the deletion is successful, and an error if it fails.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
returns `true` if the deletion is successful, and an error if it fails.
returns `true` if the deletion is successful, or `false` and an `error` if it fails.


This function retrieves the capacity of a specified List using its key(name). It returns an error if the key wasn't
predefined in the GameServer resource. The function provides the List's capacity value if successful, or `-1` along with
an error in case of failure.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
an error in case of failure.
an `error` in case of failure.


#### Alpha().GetListCapacity(key)

This function retrieves the capacity of a specified List using its key(name). It returns an error if the key wasn't
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function retrieves the capacity of a specified List using its key(name). It returns an error if the key wasn't
This function retrieves the capacity of a specified List using its key(name). It returns `-1` and an `error` if the key wasn't


This function checks if a specific string value exists in a List's values, identified by the List's key(name).
The search is case-sensitive. It returns `true` if the string is found in the list, and `false` otherwise.
An error is returned if the key wasn't predefined in the GameServer resource or if there's an issue in fetching the List.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
An error is returned if the key wasn't predefined in the GameServer resource or if there's an issue in fetching the List.
`false` and an error are returned if the key wasn't predefined in the GameServer resource or if there's an issue in fetching the List.

#### Alpha().GetListLength(key)

This function retrieves the number of items (length) in the Values list of a specified List, identified by the List's key(name).
It returns the length of the list if successful, or `-1` along with an error in case of failure, such as if the key wasn't
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
It returns the length of the list if successful, or `-1` along with an error in case of failure, such as if the key wasn't
It returns the length of the list if successful, or `-1` along with an `error` in case of failure, such as if the key wasn't

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 507f3be6-94a0-4252-a6f0-c7609a50dfba

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3537/head:pr_3537 && git checkout pr_3537
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.37.0-dev-ac01dcf-amd64

@Kalaiselvi84
Copy link
Contributor Author

I made all the changes you suggested, Ivy🙂

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 87398b7b-210b-4f35-b84c-27da2c8a64d0

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3537/head:pr_3537 && git checkout pr_3537
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.37.0-dev-899af15-amd64

Copy link
Collaborator

@igooch igooch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there! Just one additional nit.

#### Alpha().SetCounterCapacity(key, amount)

This function sets the maximum capacity for a specified Counter. A capacity value of 0 indicates no capacity limit.
The function returns `true` if the capacity is successfully set, and `false` and an `error` if the operation fails.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The function returns `true` if the capacity is successfully set, and `false` and an `error` if the operation fails.
The function returns `true` if the capacity is successfully set, and `false` and an `error` if the operation fails.

This function retrieves the current count for a specified Counter in the game server. This returns `-1` an `error` if
the key is not predefined.

{{< alert title="Note" color="info">}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move this note up to the top under Counters And Lists and make the language more generic, since this is the case for any of the "get" operations for Counters and Lists.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified the content to make it more generic and relocated it under the title.👍🏻

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1cd71067-2835-4684-a803-e94ae65a122e

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3537/head:pr_3537 && git checkout pr_3537
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.37.0-dev-013184e-amd64

Copy link
Collaborator

@igooch igooch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last nit :) Then it looks good!

silently truncated to capacity.

When retrieving count values through `get` operations, it's important to note that the values may not always represent
the most up-to-date value. Changes to Counters can occur through Game Server Allocation Actions as well as the SDK,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the most up-to-date value. Changes to Counters can occur through Game Server Allocation Actions as well as the SDK,
the most up-to-date value. Changes to Counters and Lists can occur through Game Server Allocation Actions as well as the SDK,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!


When retrieving count values through `get` operations, it's important to note that the values may not always represent
the most up-to-date value. Changes to Counters can occur through Game Server Allocation Actions as well as the SDK,
so this count may not always be accurate as it could be reading cached data. However, count will be
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
so this count may not always be accurate as it could be reading cached data. However, count will be
so this value may not always be accurate as it could be reading cached data. However, value will be

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 6070efbd-e29c-446d-98d5-40be6d95c0b4

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: dc583502-3c06-4ece-906b-bbc0817c3676

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3537/head:pr_3537 && git checkout pr_3537
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.37.0-dev-c09defb-amd64

by default.

{{< alert title="Note" color="info">}}
There is a potential race condition when count values are set from both the SDK and through the K8s API(Allocation or otherwise),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
There is a potential race condition when count values are set from both the SDK and through the K8s API(Allocation or otherwise),
There is a potential race condition when count values are set from both the SDK and through the K8s API (Allocation or otherwise),

#### Alpha().IncrementCounter(key, amount)

This function increments the count of a specified Counter by a given nonnegative value amount. The function returns `false`
and an `error` if the key wasn't predefined in the GameServer resource and returns or if the Counter is already at capacity,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"returns or if the Counter is already...". I don't understand this part. Should it be "returns error if..."?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function returns false and an error if the key wasn't predefined in the GameServer resource or if the Counter is already at capacity.


#### Alpha().AppendListValue(key, value)

This function appends a string value to a List's values, identified by the List's key(name) and the string value.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function appends a string value to a List's values, identified by the List's key(name) and the string value.
This function appends a string value to a List's values, identified by the List's key (name) and the string value.


#### Alpha().DeleteListValue(key, value)

This function removes a string from a List's values using its key(name) and the specific string value. It returns an
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function removes a string from a List's values using its key(name) and the specific string value. It returns an
This function removes a string from a List's values using its key (name) and the specific string value. It returns an


This function sets the capacity for a specified List, identified by its key (name), with the capacity value required to be
between 0 and 1000. It returns an error if the key wasn't predefined in the GameServer resource. The function returns `true`
if the capacity is successfully set, or `false` and an error if the operation fails.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if the capacity is successfully set, or `false` and an error if the operation fails.
if the capacity is successfully set, or `false` and an `error` if the operation fails.


#### Alpha().GetListCapacity(key)

This function retrieves the capacity of a specified List using its key(name). It returns `-1` and an `error` if the key wasn't
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function retrieves the capacity of a specified List using its key(name). It returns `-1` and an `error` if the key wasn't
This function retrieves the capacity of a specified List using its key( name). It returns `-1` and an `error` if the key wasn't


#### Alpha().ListContains(key, value)

This function checks if a specific string value exists in a List's values, identified by the List's key(name).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function checks if a specific string value exists in a List's values, identified by the List's key(name).
This function checks if a specific string value exists in a List's values, identified by the List's key (name).

#### Alpha().ListContains(key, value)

This function checks if a specific string value exists in a List's values, identified by the List's key(name).
The search is case-sensitive. It returns `true` if the string is found in the list, and `false` otherwise.`false` and an error
Copy link
Collaborator

@gongmax gongmax Dec 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The search is case-sensitive. It returns `true` if the string is found in the list, and `false` otherwise.`false` and an error
The search is case-sensitive. It returns `true` if the string is found in the list, and `false` otherwise. `false` and an `error`

This function checks if a specific string value exists in a List's values, identified by the List's key(name).
The search is case-sensitive. It returns `true` if the string is found in the list, and `false` otherwise.`false` and an error
are returned if the key wasn't predefined in the GameServer resource or if there's an issue in fetching the List.
#### Alpha().GetListLength(key)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#### Alpha().GetListLength(key)
#### Alpha().GetListLength(key)

are returned if the key wasn't predefined in the GameServer resource or if there's an issue in fetching the List.
#### Alpha().GetListLength(key)

This function retrieves the number of items (length) in the Values list of a specified List, identified by the List's key(name).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function retrieves the number of items (length) in the Values list of a specified List, identified by the List's key(name).
This function retrieves the number of items (length) in the Values list of a specified List, identified by the List's key (name).


#### Alpha().GetListValues(key)

This function returns all the string values from a specified List, identified by the List's key(name). It returns
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This function returns all the string values from a specified List, identified by the List's key(name). It returns
This function returns all the string values from a specified List, identified by the List's key (name). It returns

@Kalaiselvi84
Copy link
Contributor Author

@gongmax, I have made changes based on your suggestion.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 1bb40e36-8706-4daf-826a-0b98871cbc0f

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3537/head:pr_3537 && git checkout pr_3537
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.37.0-dev-a90120a-amd64

@gongmax gongmax merged commit d89e58b into googleforgames:main Dec 13, 2023
4 checks passed
@Kalaiselvi84 Kalaiselvi84 added kind/feature New features for Agones and removed kind/other labels Dec 18, 2023
@Kalaiselvi84 Kalaiselvi84 deleted the sdk branch March 15, 2024 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New features for Agones size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants