From df4b980246a91662962d3e96e98fbc034d6008f0 Mon Sep 17 00:00:00 2001 From: Tyler Holland Date: Tue, 31 Mar 2026 00:16:01 -0400 Subject: [PATCH] fix: resolve slow UI updates for non-host players (#121) Three fixes for non-host responsiveness: 1. LeaveSession now calls UpdateUI() after clearing state, so the leaver's UI immediately reflects the exit instead of showing stale lobby data. 2. HandleSessionUpdate re-applies local spec overrides after rebuilding the player list from host data. This prevents in-flight SESSION_UPDATE messages from reverting spec changes the non-host just made. Also clears joinPending when the local player appears in the host's player list (implicit ACK), avoiding false "may not have been received" warnings. 3. JOIN_ACK timeout increased from 5s to 10s to accommodate ChatThrottleLib delays in larger lobbies. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/Comm.lua | 33 ++++++ src/Session.lua | 1 + src/UI/Lobby.lua | 2 +- tests/test_core.lua | 243 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 278 insertions(+), 1 deletion(-) diff --git a/src/Comm.lua b/src/Comm.lua index af0f102..82dd641 100755 --- a/src/Comm.lua +++ b/src/Comm.lua @@ -229,6 +229,39 @@ function WHLSN:HandleSessionUpdate(data, sender) for _, pd in ipairs(data.players) do self.session.players[#self.session.players + 1] = WHLSN.Player.FromDict(pd) end + + -- Re-apply local spec overrides for the non-host's own player entry. + -- A SESSION_UPDATE already in-flight may carry stale spec data that + -- overwrites the optimistic local update from SpecOverride. + if not self:NamesMatch(data.host, self:GetMyFullName()) then + local saved = self.db and self.db.char and self.db.char.specOverrides + if saved and saved.mainRole then + local freshPlayer = self:DetectLocalPlayer(saved.offspecs, saved.mainRole) + if freshPlayer then + for i, p in ipairs(self.session.players) do + if self:NamesMatch(p.name, self:GetMyFullName()) then + self.session.players[i] = freshPlayer + break + end + end + end + end + end + + -- Implicit ACK: if the local player appears in the host's player list, + -- the host has acknowledged our join — clear the pending state. + if self.session.joinPending then + for _, p in ipairs(self.session.players) do + if self:NamesMatch(p.name, self:GetMyFullName()) then + self.session.joinPending = false + if self.joinAckTimer then + self.joinAckTimer:Cancel() + self.joinAckTimer = nil + end + break + end + end + end end if data.compactGroups then diff --git a/src/Session.lua b/src/Session.lua index 153f172..069d1ea 100755 --- a/src/Session.lua +++ b/src/Session.lua @@ -114,6 +114,7 @@ function WHLSN:LeaveSession() end self:ClearSessionState() + self:UpdateUI() self:Print("You have left the lobby.") end diff --git a/src/UI/Lobby.lua b/src/UI/Lobby.lua index 6f83ea0..decf891 100755 --- a/src/UI/Lobby.lua +++ b/src/UI/Lobby.lua @@ -541,7 +541,7 @@ function WHLSN:RequestJoin() self:Print("Join request sent.") self.session.joinPending = true - self.joinAckTimer = C_Timer.NewTimer(5, function() + self.joinAckTimer = C_Timer.NewTimer(10, function() WHLSN.session.joinPending = false WHLSN.joinAckTimer = nil WHLSN:Print("Join request may not have been received. Try again.") diff --git a/tests/test_core.lua b/tests/test_core.lua index b6e28ef..518ff21 100755 --- a/tests/test_core.lua +++ b/tests/test_core.lua @@ -1965,6 +1965,166 @@ describe("SendSessionUpdate compactGroups", function() end) end) +describe("LeaveSession UI update", function() + before_each(function() + WHLSN:OnInitialize() + WHLSN.sent_messages = {} + WHLSN.SendCommMessage = function(self, prefix, msg, channel) + self.sent_messages[#self.sent_messages + 1] = { prefix = prefix, msg = msg, channel = channel } + end + WHLSN.Serialize = function(self, data) return data end + WHLSN.Deserialize = function(self, msg) return true, msg end + WHLSN.ShowMainFrame = function() end + WHLSN.DetectLocalPlayer = function() + return WHLSN.Player:New("TestPlayer", "tank", {}, {}) + end + end) + + it("should call UpdateUI after leaving session", function() + WHLSN.session.status = "lobby" + WHLSN.session.host = "HostA" + + local uiUpdated = false + WHLSN.UpdateUI = function() uiUpdated = true end + + WHLSN:LeaveSession() + + assert.is_true(uiUpdated) + end) +end) + +-- Save the real DetectLocalPlayer before any tests can monkey-patch it +local realDetectLocalPlayer = WHLSN.DetectLocalPlayer + +describe("HandleSessionUpdate spec override preservation", function() + before_each(function() + if not mock_db.char then mock_db.char = {} end + mock_db.char.activeSession = nil + WHLSN:OnInitialize() + -- Restore real DetectLocalPlayer (previous tests may have monkey-patched it) + WHLSN.DetectLocalPlayer = realDetectLocalPlayer + WHLSN.UpdateUI = function() end + WHLSN.Serialize = function(self, data) return data end + WHLSN.Deserialize = function(self, msg) return true, msg end + end) + + it("should re-apply local spec override when receiving stale host data", function() + -- Simulate the non-host having saved a spec override to "healer" + WHLSN.db.char.specOverrides = { mainRole = "healer", offspecs = {} } + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "HostPlayer", + players = { + { name = "TestPlayer-Illidan", mainRole = "tank", offspecs = {}, utilities = {} }, + { name = "HostPlayer", mainRole = "tank", offspecs = {}, utilities = {} }, + }, + } + WHLSN:HandleSessionUpdate(data, "HostPlayer") + + -- The local player's role should be "healer" (from override), not "tank" (from stale data) + local localPlayer = nil + for _, p in ipairs(WHLSN.session.players) do + if WHLSN:NamesMatch(p.name, WHLSN:GetMyFullName()) then + localPlayer = p + break + end + end + assert.is_not_nil(localPlayer) + assert.equals("healer", localPlayer.mainRole) + end) + + it("should not re-apply spec overrides for the host", function() + -- When we ARE the host, host data is authoritative. + -- Call HandleSessionUpdate directly because OnCommReceived filters out + -- messages from self (sender == UnitName("player")). + WHLSN.db.char.specOverrides = { mainRole = "healer", offspecs = {} } + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "TestPlayer-Illidan", + players = { + { name = "TestPlayer-Illidan", mainRole = "tank", offspecs = {}, utilities = {} }, + }, + } + WHLSN:HandleSessionUpdate(data, "TestPlayer-Illidan") + + -- Should remain "tank" since we are the host + local localPlayer = WHLSN.session.players[1] + assert.equals("tank", localPlayer.mainRole) + end) + + it("should not crash when no spec overrides are saved", function() + WHLSN.db.char.specOverrides = nil + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "HostPlayer", + players = { + { name = "TestPlayer-Illidan", mainRole = "tank", offspecs = {}, utilities = {} }, + }, + } + assert.has_no.errors(function() + WHLSN:HandleSessionUpdate(data, "HostPlayer") + end) + + assert.equals("tank", WHLSN.session.players[1].mainRole) + end) +end) + +describe("HandleSessionUpdate implicit joinPending ACK", function() + before_each(function() + if not mock_db.char then mock_db.char = {} end + mock_db.char.activeSession = nil + WHLSN:OnInitialize() + WHLSN.UpdateUI = function() end + WHLSN.Serialize = function(self, data) return data end + WHLSN.Deserialize = function(self, msg) return true, msg end + end) + + it("should clear joinPending when local player is in host player list", function() + WHLSN.session.joinPending = true + local timerCancelled = false + WHLSN.joinAckTimer = { Cancel = function() timerCancelled = true end } + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "HostPlayer", + players = { + { name = "HostPlayer", mainRole = "tank", offspecs = {}, utilities = {} }, + { name = "TestPlayer-Illidan", mainRole = "healer", offspecs = {}, utilities = {} }, + }, + } + WHLSN:HandleSessionUpdate(data, "HostPlayer") + + assert.is_false(WHLSN.session.joinPending) + assert.is_true(timerCancelled) + assert.is_nil(WHLSN.joinAckTimer) + end) + + it("should not clear joinPending when local player is not in player list", function() + WHLSN.session.joinPending = true + WHLSN.joinAckTimer = { Cancel = function() end } + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "HostPlayer", + players = { + { name = "HostPlayer", mainRole = "tank", offspecs = {}, utilities = {} }, + { name = "OtherPlayer", mainRole = "healer", offspecs = {}, utilities = {} }, + }, + } + WHLSN:HandleSessionUpdate(data, "HostPlayer") + + assert.is_true(WHLSN.session.joinPending) + end) +end) + describe("HandleSessionUpdate compactGroups", function() before_each(function() WHLSN:OnInitialize() @@ -2147,3 +2307,86 @@ describe("CompleteSession host guard", function() assert.is_false(broadcastCalled) end) end) + +describe("HandleSessionUpdate implicit joinPending ACK", function() + before_each(function() + if not mock_db.char then mock_db.char = {} end + mock_db.char.activeSession = nil + WHLSN:OnInitialize() + WHLSN.UpdateUI = function() end + WHLSN.Serialize = function(self, data) return data end + WHLSN.Deserialize = function(self, msg) return true, msg end + end) + + it("should clear joinPending when local player is in host player list", function() + WHLSN.session.joinPending = true + local timerCancelled = false + WHLSN.joinAckTimer = { Cancel = function() timerCancelled = true end } + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "HostPlayer", + players = { + { name = "HostPlayer", mainRole = "tank", offspecs = {}, utilities = {} }, + { name = "TestPlayer-Illidan", mainRole = "healer", offspecs = {}, utilities = {} }, + }, + } + WHLSN:HandleSessionUpdate(data, "HostPlayer") + + assert.is_false(WHLSN.session.joinPending) + assert.is_true(timerCancelled) + assert.is_nil(WHLSN.joinAckTimer) + end) + + it("should not clear joinPending when local player is not in player list", function() + WHLSN.session.joinPending = true + WHLSN.joinAckTimer = { Cancel = function() end } + + local data = { + version = WHLSN.VERSION, + status = "lobby", + host = "HostPlayer", + players = { + { name = "HostPlayer", mainRole = "tank", offspecs = {}, utilities = {} }, + { name = "OtherPlayer", mainRole = "healer", offspecs = {}, utilities = {} }, + }, + } + WHLSN:HandleSessionUpdate(data, "HostPlayer") + + assert.is_true(WHLSN.session.joinPending) + end) +end) + +describe("JOIN_ACK timer duration", function() + before_each(function() + WHLSN:OnInitialize() + WHLSN.sent_messages = {} + WHLSN.SendCommMessage = function(self, prefix, msg, channel) + self.sent_messages[#self.sent_messages + 1] = { prefix = prefix, msg = msg, channel = channel } + end + WHLSN.Serialize = function(self, data) return data end + WHLSN.UpdateLobbyView = function() end + WHLSN.DetectLocalPlayer = function() + return WHLSN.Player:New("TestPlayer", "tank", {}, {}) + end + end) + + it("should use a 10 second timeout for JOIN_ACK", function() + WHLSN.session.host = "HostPlayer" + WHLSN.session.status = "lobby" + + local timerDuration = nil + local origNewTimer = _G.C_Timer.NewTimer + _G.C_Timer.NewTimer = function(duration, cb) + timerDuration = duration + return { Cancel = function() end } + end + + WHLSN:RequestJoin() + + _G.C_Timer.NewTimer = origNewTimer + + assert.equals(10, timerDuration) + end) +end)