fix(agones): report network player counts, not this proxy's#64
Draft
hbrombeer wants to merge 1 commit into
Draft
fix(agones): report network player counts, not this proxy's#64hbrombeer wants to merge 1 commit into
hbrombeer wants to merge 1 commit into
Conversation
/agones printed the players connected to the proxy running it. With two proxies in front of one lobby, each saw half of it: two players standing together in the lobby were reported as "1 players" on both. Velocity cannot do better — playersConnected and playerCount are proxy-local by definition. Ask ProxyService, which answers for the whole network. When the network cannot be asked the numbers are still shown, but labelled as proxy-local. An unlabelled local count is indistinguishable from a network one, which is how this went unnoticed.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



The bug
Two players standing in the same lobby, each connected through a different proxy.
/agonesreported(1 players)— on both proxies.Live from the dev cluster at the time of the report:
AgonesCommandreadserver.playersConnected.sizeandproxyServer.playerCount. Both are proxy-local by definition — Velocity has no idea the other proxy exists. The command printed the half it could see as if it were the whole network.The fix
Ask
ProxyService.getNetworkPlayerCounts(), backed by service-player's session table (which already recordsproxy_idandserver_namefor every player).When the presence service cannot be reached, the counts still render — but the command says they are proxy-local. That matters: an unlabelled local count looks exactly like a network count, which is precisely why this shipped unnoticed. Silently degrading back to the old numbers would be reprinting the bug.
plugin-proxy-apiiscompileOnly, never shaded: theProxyServiceRegistrywe read must be the same class plugin-proxy loaded, or we would read a registry nobody writes to.Merge order
Last of 5. Until the ones above land,
getNetworkPlayerCounts()returns null and/agonesbehaves as today, minus the false confidence (it now labels the numbers).library-grpc-contracts-player0.4.0plugin-proxy-api0.2.0service-player— implement the RPCplugin-player— implementcountPlayersByServer()Verified
./gradlew :velocity:testgreen against locally-publishedplugin-proxy-api:0.2.0. New tests: two players on one lobby across two proxies read as(2 players); a fallback to proxy-local counts is labelled; a server nobody is on shows(0 players)rather than vanishing.