-
Notifications
You must be signed in to change notification settings - Fork 810
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
docs: improve counter and list example comments #3818
docs: improve counter and list example comments #3818
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 Failed 😱 Build Id: 3d6f705a-0a9d-4ca4-b452-a09f064cd672 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: d93a7ef3-6dc8-41c2-b6de-eceaae8c0087 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:
|
@@ -58,11 +58,11 @@ spec: | |||
# Which gameservers in the Fleet are most important to keep around - impacts scale down logic. | |||
# priorities: | |||
# - type: Counter # Sort by a “Counter” | |||
# key: player # The name of the Counter. No impact if no GameServer found. | |||
# order: Descending # Default is "Ascending" so smaller capacity will be removed first on down scaling. | |||
# key: rooms # The name of the Counter. No impact if no GameServer found. |
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.
Ah good catch! Thank you! Got that in the examples.yaml, but not here.
https://github.com/googleforgames/agones/blob/main/examples/fleet.yaml has been updated with the move to beta -- but can we make the descriptions match across both (particularly the "available capacity" addition ?
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.
Thanks for the feedback! Changed in 1716e09.
Added the Beta information and made the descriptions in fleet.yaml and fleet.md more consistent.
# counters: # counters are int64 counters stored against a GameServer in the fleet that can be incremented and decremented by set amounts. Keys must be declared at Fleet creation time. | ||
# games: # arbitrary key. | ||
# count: 0 # initial value. | ||
# capacity: 100 # (Optional) Defaults to 1000 and setting capacity to max(int64) may lead to issues and is not recommended. See GitHub issue https://github.com/googleforgames/agones/issues/3636 for more details. |
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.
# capacity: 100 # (Optional) Defaults to 1000 and setting capacity to max(int64) may lead to issues and is not recommended. See GitHub issue https://github.com/googleforgames/agones/issues/3636 for more details. | |
# capacity: 100 |
Thanks for adding the Counter and List example here - was missing!
Since this is the GameServer template, I think we can keep this pretty low on the description, since that's what the GameServer reference is for -- I'd bring this down to the level we have in https://github.com/googleforgames/agones/blob/release-1.40.0/examples/fleet.yaml
Some for the other fields.
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.
Changed in 188f491 and made both files more consistent.
…ference files Also includes some tidying up of comments
…ple and reference file
Build Failed 😱 Build Id: a2eb7c5e-36bf-46e9-98d6-5704523689a5 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 2b470684-ef36-4078-8993-e13dc45693b0 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: 75254938-bd73-450a-8ae8-5048d40ca623 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:
|
cc @igooch for extra eyes |
# count: # Count and/or capacity must be listed (but may be nil) otherwise the counter will by dropped by the CRD schema. | ||
# lists: | ||
# players: | ||
# capacity: # Capacity and/or values must be listed (but may be nil) otherwise the list will be dropped by the CRD schema. |
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.
# capacity: # Capacity and/or values must be listed (but may be nil) otherwise the list will be dropped by the CRD schema. |
Matching the landing page.
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 have now removed the verbose comments and made it match landing page with:
players:
values: []
# Counts and Lists provides the configuration for generic (player, room, session, etc.) tracking features. | ||
# Now in Beta, and enabled by default | ||
# counters: | ||
# players: |
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.
# players: | |
# rooms: |
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.
This has now been swapped
examples/fleet.yaml
Outdated
@@ -95,7 +100,7 @@ spec: | |||
# Now in Beta, and enabled by default | |||
counters: | |||
players: |
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.
Ow, I just realised my head hurts. I thought I'd fixed this everywhere when writing https://agones.dev/site/docs/guides/counters-and-lists/ , but clearly I missed some.
What we should have in all examples and docs:
rooms
- is a counter
players
is a list.
Somewhere this all got messed up, with an rooms
being a list, and the introduction of sessions
- but the above landing page, that should be the source of truth.
Sorry to make you go back and forth - I just noticed this.
Let's stick to players
and rooms
and remove all else please 😄
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.
Hey no worries at all! Anything to make to make the docs better 🕺.
I have now removed mentions of sessions
in and made rooms
only used as an example as counters files e.g., fleet.yaml
and gameserver.yaml
in examples
and fleet.md
and gameserver.md
in Reference
.
However as a result of this examples for lists using rooms which showed initial values were also removed. If the intention of this was to show user's they are able to set initial values, I can revert this and change the wording from rooms
to maybe map
and initial values to be map1
, map2
, etc.
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'd prefer to leave in some mention of the above either in the example or the docs. The idea is to show that you can the same key in Counters as in List, that a List can have starting values, and that certain values may be nil.
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.
Okie dokes. As that is the intention, I have reintroduced them in gameserver.yaml
in examples
and gameserver.md
in References
since GameServer would be the source of most user's reference when viewing the docs in addition to it being linked from the CountersAndLists
documentation page itself.
Build Failed 😱 Build Id: 6576db29-672a-484c-89f6-f1a2cd65a231 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: a55d43fe-0269-42d2-83b4-6886273e2b34 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: d0f97635-6084-4e0e-9e3a-18e8f20a701d To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
…unters and players a list
Build Failed 😱 Build Id: 3746ba0f-c110-4992-97fb-ee961771a419 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
…t and list landing page
Build Failed 😱 Build Id: 1bae152a-2b80-4d0e-a502-90c69342e3e4 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: 69a29ef8-cc01-4be1-ad9c-57776850d71f To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Failed 😱 Build Id: f8fd9edc-08a2-450e-91b8-4f1c873a1b3e To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Kinda just to retrigger ci
Build Succeeded 👏 Build Id: 69f93d8f-fb98-4d5b-b044-5a0f08e1650f 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.
Could you please pull in Main and resolve some of what may be merge conflicts?
This reintroduce usage of rooms as a key in lists to show that it is allowed to have the same key name in between list and counters. Also readded initial values for lists as example.
Build Failed 😱 Build Id: ffefc047-1193-430d-9c49-3ab081a6ab56 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
Build Succeeded 👏 Build Id: ec6ecb14-41b4-4d83-9dea-1b2c1fa5ceaf 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.
LGTM
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [agones](https://agones.dev) ([source](https://togithub.com/googleforgames/agones)) | minor | `1.40.0` -> `1.41.0` | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>googleforgames/agones (agones)</summary> ### [`v1.41.0`](https://togithub.com/googleforgames/agones/blob/HEAD/CHANGELOG.md#v1410-2024-06-04) [Compare Source](https://togithub.com/googleforgames/agones/compare/v1.40.0...v1.41.0) [Full Changelog](https://togithub.com/googleforgames/agones/compare/v1.40.0...v1.41.0) **Implemented enhancements:** - Configure Allocator Status Code by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3782](https://togithub.com/googleforgames/agones/pull/3782) - Graduate Counters and Lists to Beta by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3801](https://togithub.com/googleforgames/agones/pull/3801) - Passthrough autopilot - Adds an AutopilotPassthroughPort Feature Gate and new pod label by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3809](https://togithub.com/googleforgames/agones/pull/3809) - CountsAndLists: Move to Beta Protobuf by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3806](https://togithub.com/googleforgames/agones/pull/3806) - feat: support multiple port ranges by [@​nrwiersma](https://togithub.com/nrwiersma) in [https://github.com/googleforgames/agones/pull/3747](https://togithub.com/googleforgames/agones/pull/3747) - Changes `sdk-server` to Patch instead of Update by [@​igooch](https://togithub.com/igooch) in [https://github.com/googleforgames/agones/pull/3803](https://togithub.com/googleforgames/agones/pull/3803) - Generate grpc for nodejs from alpha to beta by [@​lacroixthomas](https://togithub.com/lacroixthomas) in [https://github.com/googleforgames/agones/pull/3825](https://togithub.com/googleforgames/agones/pull/3825) - Update CountsAndLists from Alpha to Beta by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3824](https://togithub.com/googleforgames/agones/pull/3824) - feat(gameserver): New DirectToGameServer PortPolicy allows direct traffic to a GameServer by [@​daniellee](https://togithub.com/daniellee) in [https://github.com/googleforgames/agones/pull/3807](https://togithub.com/googleforgames/agones/pull/3807) - Passthrough autopilot - Adds mutating webhook by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3833](https://togithub.com/googleforgames/agones/pull/3833) - Passthrough autopilot - added ports array case and updated unit tests by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3842](https://togithub.com/googleforgames/agones/pull/3842) - Nodejs counters and lists by [@​steven-supersolid](https://togithub.com/steven-supersolid) in [https://github.com/googleforgames/agones/pull/3726](https://togithub.com/googleforgames/agones/pull/3726) - Promote AutopilotPassthroughPort feature gate to Alpha by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3849](https://togithub.com/googleforgames/agones/pull/3849) **Fixed bugs:** - Helm Param Update: Default to agones.controller if agones.extensions is Missing by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3773](https://togithub.com/googleforgames/agones/pull/3773) - fix: rollout strategy issues by [@​nrwiersma](https://togithub.com/nrwiersma) in [https://github.com/googleforgames/agones/pull/3762](https://togithub.com/googleforgames/agones/pull/3762) - Set Minimum Buffer Size to 1 by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3749](https://togithub.com/googleforgames/agones/pull/3749) - Pin ltsc2019 to older SHA by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3829](https://togithub.com/googleforgames/agones/pull/3829) - TestGameServerAllocationDuringMultipleAllocationClients: Readdress flake by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3831](https://togithub.com/googleforgames/agones/pull/3831) - Refactor finalizer name to include valid domain name and path by [@​indexjoseph](https://togithub.com/indexjoseph) in [https://github.com/googleforgames/agones/pull/3840](https://togithub.com/googleforgames/agones/pull/3840) - agones-{extensions,allocator}: Be more defensive about draining by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3839](https://togithub.com/googleforgames/agones/pull/3839) - agones-{extensions,allocator}: Pause after cancelling context by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3843](https://togithub.com/googleforgames/agones/pull/3843) - Change the line to modify in Quickstart: Edit a Game Server by [@​peterzhongyi](https://togithub.com/peterzhongyi) in [https://github.com/googleforgames/agones/pull/3844](https://togithub.com/googleforgames/agones/pull/3844) **Other:** - Prep for Release v1.41.0 by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3800](https://togithub.com/googleforgames/agones/pull/3800) - Update site documentation to reflect firewall prefix and default to Autopilot cluster creation for Agones by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3769](https://togithub.com/googleforgames/agones/pull/3769) - Add a System Diagram and overview page by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3792](https://togithub.com/googleforgames/agones/pull/3792) - Update Side Menu: Preserve and Restore Scroll Position by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3805](https://togithub.com/googleforgames/agones/pull/3805) - fix: typo by [@​skmpf](https://togithub.com/skmpf) in [https://github.com/googleforgames/agones/pull/3808](https://togithub.com/googleforgames/agones/pull/3808) - Helm Config: Add httpUnallocatedStatusCode in Allocator Service by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3802](https://togithub.com/googleforgames/agones/pull/3802) - Update Docs: CountersAndLists to Beta by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3810](https://togithub.com/googleforgames/agones/pull/3810) - Disable Dev feature FeatureAutopilotPassthroughPort by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3815](https://togithub.com/googleforgames/agones/pull/3815) - Disable FeatureAutopilotPassthroughPort in features.go by [@​vicentefb](https://togithub.com/vicentefb) in [https://github.com/googleforgames/agones/pull/3816](https://togithub.com/googleforgames/agones/pull/3816) - SDK proto compatibility guarantees and deprecation policies documentation by [@​igooch](https://togithub.com/igooch) in [https://github.com/googleforgames/agones/pull/3774](https://togithub.com/googleforgames/agones/pull/3774) - Fix dangling "as of" by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3827](https://togithub.com/googleforgames/agones/pull/3827) - Steps to Promote SDK Features from Alpha to Beta by [@​Kalaiselvi84](https://togithub.com/Kalaiselvi84) in [https://github.com/googleforgames/agones/pull/3814](https://togithub.com/googleforgames/agones/pull/3814) - Adds comment for help troubleshooting issues with terraform tfstate by [@​igooch](https://togithub.com/igooch) in [https://github.com/googleforgames/agones/pull/3822](https://togithub.com/googleforgames/agones/pull/3822) - docs: improve counter and list example comments by [@​yonbh](https://togithub.com/yonbh) in [https://github.com/googleforgames/agones/pull/3818](https://togithub.com/googleforgames/agones/pull/3818) - Skip /tmp/ on yamllint by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3838](https://togithub.com/googleforgames/agones/pull/3838) - TestAllocatorAfterDeleteReplica: More logging by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3837](https://togithub.com/googleforgames/agones/pull/3837) - Instructions for upgrading golang version by [@​gongmax](https://togithub.com/gongmax) in [https://github.com/googleforgames/agones/pull/3819](https://togithub.com/googleforgames/agones/pull/3819) - Remove unused function FindGameServerContainer by [@​zmerlynn](https://togithub.com/zmerlynn) in [https://github.com/googleforgames/agones/pull/3841](https://togithub.com/googleforgames/agones/pull/3841) - Adds Unreal to the List of URL Links to Not Check by [@​igooch](https://togithub.com/igooch) in [https://github.com/googleforgames/agones/pull/3847](https://togithub.com/googleforgames/agones/pull/3847) - docs: clarify virtualization setup for Windows versions by [@​andresromerodev](https://togithub.com/andresromerodev) in [https://github.com/googleforgames/agones/pull/3850](https://togithub.com/googleforgames/agones/pull/3850) **New Contributors:** - [@​skmpf](https://togithub.com/skmpf) made their first contribution in [https://github.com/googleforgames/agones/pull/3808](https://togithub.com/googleforgames/agones/pull/3808) - [@​yonbh](https://togithub.com/yonbh) made their first contribution in [https://github.com/googleforgames/agones/pull/3818](https://togithub.com/googleforgames/agones/pull/3818) - [@​peterzhongyi](https://togithub.com/peterzhongyi) made their first contribution in [https://github.com/googleforgames/agones/pull/3844](https://togithub.com/googleforgames/agones/pull/3844) - [@​andresromerodev](https://togithub.com/andresromerodev) made their first contribution in [https://github.com/googleforgames/agones/pull/3850](https://togithub.com/googleforgames/agones/pull/3850) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zOTAuMCIsInVwZGF0ZWRJblZlciI6IjM3LjM5MC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9oZWxtIiwidHlwZS9taW5vciJdfQ==-->
What type of PR is this?
/kind documentation
What this PR does / Why we need it:
Some of the changes include making it more consistent between the usage of
rooms
orplayers
as examples incounters
andlists
whenever possible.The added documentation on https://agones.dev/site/docs/reference/fleet/ roughly matches the one on https://agones.dev/site/docs/reference/gameserver/ for consistency.
Which issue(s) this PR fixes:
Special notes for your reviewer: