Skip to content

Commit

Permalink
Fix autolobby sending invalid game states to the server (#6567)
Browse files Browse the repository at this point in the history
  • Loading branch information
Garanas authored Nov 30, 2024
1 parent 4a824e8 commit e951c9f
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 13 deletions.
17 changes: 17 additions & 0 deletions docs/_posts/2024-11-29-3815.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
layout: post
title: Game version 3815
permalink: changelog/3815
---

# Game version 3815 (29th of November, 2024)

Fix the autolobby messing with the expected server state. We hope the autolobby functions as expected now.

With thanks to Nomander for investigating the issue and deciphering a procedure to test the autolobby changes on the test server. Previously the changes were only tested locally by running multiple game instances on the same computer. When testing locally there is no server, and therefore we did not detect this issue sooner.

Apologies for the inconveniences,

With kind regards,

Jip
3 changes: 1 addition & 2 deletions lua/ui/lobby/autolobby.lua
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ local AutolobbyCommunicationsInstance = false
---@param natTraversalProvider any
---@return UIAutolobbyCommunications
function CreateLobby(protocol, localPort, desiredPlayerName, localPlayerUID, natTraversalProvider)
-- we intentionally do not log the 'natTraversalProvider' parameter as it can cause issues due to being an uninitialized C object
LOG("CreateLobby", protocol, localPort, desiredPlayerName, localPlayerUID)

-- create the interface, needs to be done before the lobby is
Expand All @@ -64,8 +65,6 @@ function CreateLobby(protocol, localPort, desiredPlayerName, localPlayerUID, nat
AutolobbyCommunicationsInstance.LobbyParameters.LocalPlayerPeerId = localPlayerUID
AutolobbyCommunicationsInstance.LobbyParameters.NatTraversalProvider = natTraversalProvider

AutolobbyCommunicationsInstance:SendGameStateToServer('Idle')

return AutolobbyCommunicationsInstance
end

Expand Down
4 changes: 0 additions & 4 deletions lua/ui/lobby/autolobby/AutolobbyController.lua
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,6 @@ AutolobbyCommunications = Class(MohoLobbyMethods, AutolobbyServerCommunicationsC
self:DebugSpew("LaunchGame")
self:DebugSpew(reprs(gameConfig, { depth = 10 }))

self:SendGameStateToServer('Launching')
return MohoLobbyMethods.LaunchGame(self, gameConfig)
end,

Expand Down Expand Up @@ -820,7 +819,6 @@ AutolobbyCommunications = Class(MohoLobbyMethods, AutolobbyServerCommunicationsC
-- start prefetching the scenario
self:Prefetch(self.GameOptions, self.GameMods)

self:SendGameStateToServer('Lobby')
self:SendLaunchStatusToServer('Hosting')

-- update UI for game options
Expand Down Expand Up @@ -855,8 +853,6 @@ AutolobbyCommunications = Class(MohoLobbyMethods, AutolobbyServerCommunicationsC
self.LocalPeerId = localPeerId
self.HostID = hostPeerId

self:SendGameStateToServer('Lobby')

-- occasionally send data over the network to create pings on screen
self.Trash:Add(ForkThread(self.ShareLaunchStatusThread, self))
-- self.Trash:Add(ForkThread(self.CheckForRejoinThread, self)) -- disabled, for now
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

-------------------------------------------------------------------------------
--#region Game <-> Server communications

--
-- All the following logic is tightly coupled with functionality on either the
-- lobby server, the ice adapter, the java server and/or the client. For more
-- context you can search for the various keywords in the following repositories:
Expand All @@ -32,6 +32,36 @@
--
-- Specifically, the following file processes these messages on the server:
-- - https://github.com/FAForever/server/blob/98271c421412467fa387f3a6530fe8d24e360fa4/server/gameconnection.py
--
-- ## How to test game communications
--
-- You cannot test the game <-> server communications using the launch script to
-- launch local instances of the game. This requires a bit of effort.
--
-- Prerequisites:
-- - A Lua development environment
-- - Two separate instances of the FAF client
--
-- Procedure:
-- - Start the FAF client and then logout to view the login screen.
-- - Choose the test server.
-- - Navigate to the leaderboards.
-- - Navigate to the last page of the leaderboards.
-- - Both clients should pick a player with almost no games.
-- - Back in the FAF client login screen, click on 'login'.
-- - - Sanity check: it should state somewhere on screen that you are logging into the test server.
-- - Use the nickname of the player as login. Use `foo` as the password. Not all accounts that exist
-- in production also exist in the test server, so you may have to try a few to find one that works.
--
-- Once logged in:
-- - Via the hamburger menu (top left) go to Settings -> Forged Alliance Forever
-- - Update the command line format to `"%s" /init init_local_development.lua`, it auto saves
-- - Start searching
--
-- The procedure applies to both clients. By updating the command line format you override the
-- game files that the client is loading. Using this approach you can launch the game from
-- any pull request. Just make sure that both clients use the same pull request and have gone
-- through the same procedure.

-- upvalue scope for performance
local GpgNetSend = GpgNetSend
Expand Down Expand Up @@ -107,6 +137,11 @@ AutolobbyServerCommunicationsComponent = ClassSimple {
---@param self UIAutolobbyServerCommunicationsComponent | UIAutolobbyCommunications
---@param value UILobbyState
SendGameStateToServer = function(self, value)
-- any other value seriously messes with the lobby protocol on the server
if value ~= 'Launching' then
return
end

GpgNetSend('GameState', value)
end,

Expand All @@ -124,7 +159,7 @@ AutolobbyServerCommunicationsComponent = ClassSimple {
GpgNetSend('EstablishedPeer', peerId)
end,

--- Sends a message to the server that we disconnected from a peer. Note that a peer may be trying to rejoin. See also the launch status of the given peer.
--- Sends a message to the server that we disconnected from a peer. Note that a peer may be trying to rejoin. See also the launch status of the given peer.
---@param self UIAutolobbyServerCommunicationsComponent | UIAutolobbyCommunications
---@param peerId UILobbyPeerId
SendDisconnectedPeer = function(self, peerId)
Expand Down
23 changes: 22 additions & 1 deletion lua/ui/lobby/changelogData.lua
Original file line number Diff line number Diff line change
@@ -1,8 +1,29 @@
---@type number
last_version = 3814
last_version = 3815

---@type PatchNotes[]
gamePatches = {
{
version = 3815,
name = "Hotfix",
hasPrettyGithubRelease = true,
hasPrettyPatchnotes = false,
description = {
"# Game version 3815 (29th of November, 2024)",
"",
"Fix the autolobby messing with the expected server state. We hope the autolobby functions as expected now.",
"",
"With thanks to Nomander for investigating the issue and deciphering a procedure to test the autolobby changes on ",
"the test server. Previously the changes were only tested locally by running multiple game instances on the same ",
"computer. When testing locally there is no server, and therefore we did not detect this issue sooner.",
"",
"Apologies for the inconveniences,",
"",
"With kind regards,",
"",
"Jip",
},
},
{
version = 3814,
name = "Hotfix",
Expand Down
6 changes: 3 additions & 3 deletions lua/version.lua
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ local Commit = 'unknown' -- The use of `'` instead of `"` is **intentional**

--#endregion

local Version = "3814"
---@alias PATCH "3814"
---@alias VERSION "1.5.3814"
local Version = "3815"
---@alias PATCH "3815"
---@alias VERSION "1.5.3815"
---@return PATCH # Game release
function GetVersion()
LOG(string.format('Supreme Commander: Forged Alliance Lua version %s at %s (%s)', Version, GameType, Commit))
Expand Down
2 changes: 1 addition & 1 deletion mod_info.lua
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
-- - https://github.com/FAForever/fa/blob/deploy/fafdevelop/lua/MODS.LUA

name = "Forged Alliance Forever"
version = 3814 -- needs to be an integer as it is parsed as a short (16 bit integer)
version = 3815 -- needs to be an integer as it is parsed as a short (16 bit integer)
_faf_modname='faf'
copyright = "Forged Alliance Forever Community"
description = "Forged Alliance Forever extends Forged Alliance, bringing new patches, game modes, units, ladder, and much more!"
Expand Down

0 comments on commit e951c9f

Please sign in to comment.