diff --git a/Code/client/Services/Generic/PartyService.cpp b/Code/client/Services/Generic/PartyService.cpp index ed6bbed0a..e9bebf137 100644 --- a/Code/client/Services/Generic/PartyService.cpp +++ b/Code/client/Services/Generic/PartyService.cpp @@ -58,7 +58,8 @@ void PartyService::CreateInvite(const uint32_t aPlayerId) const noexcept void PartyService::AcceptInvite(const uint32_t aInviterId) const noexcept { - if (!m_invitations.contains(aInviterId)) + const auto itor = m_invitations.find(aInviterId); + if (!m_transport.IsConnected() || m_inParty || itor == m_invitations.end() || itor->second <= m_transport.GetClock().GetCurrentTick()) return; PartyAcceptInviteRequest request; @@ -92,7 +93,7 @@ void PartyService::OnUpdate(const UpdateEvent& acEvent) noexcept auto itor = std::begin(m_invitations); while (itor != std::end(m_invitations)) { - if (itor->second < cCurrentTick) + if (itor->second <= cCurrentTick) itor = m_invitations.erase(itor); else ++itor; @@ -140,12 +141,17 @@ void PartyService::OnPartyInfo(const NotifyPartyInfo& acPartyInfo) noexcept void PartyService::OnPartyInvite(const NotifyPartyInvite& acPartyInvite) noexcept { + const auto cCurrentTick = m_transport.GetClock().GetCurrentTick(); + if (m_inParty || acPartyInvite.ExpiryTick <= cCurrentTick) + return; + spdlog::debug("[PartyService]: Got party invite from {}", acPartyInvite.InviterId); m_invitations[acPartyInvite.InviterId] = acPartyInvite.ExpiryTick; auto pArguments = CefListValue::Create(); pArguments->SetInt(0, acPartyInvite.InviterId); + pArguments->SetDouble(1, static_cast(acPartyInvite.ExpiryTick - cCurrentTick)); m_world.GetOverlayService().GetOverlayApp()->ExecuteAsync("partyInviteReceived", pArguments); } @@ -157,6 +163,7 @@ void PartyService::OnPartyJoined(const NotifyPartyJoined& acPartyJoined) noexcep m_isLeader = acPartyJoined.IsLeader; m_leaderPlayerId = acPartyJoined.LeaderPlayerId; m_partyMembers = acPartyJoined.PlayerIds; + m_invitations.clear(); m_world.GetDispatcher().trigger(PartyJoinedEvent(m_isLeader)); } @@ -176,4 +183,5 @@ void PartyService::DestroyParty() noexcept m_isLeader = false; m_leaderPlayerId = -1; m_partyMembers.clear(); + m_invitations.clear(); } diff --git a/Code/server/Components/PartyComponent.h b/Code/server/Components/PartyComponent.h index aaf5eaf44..a4b455afa 100644 --- a/Code/server/Components/PartyComponent.h +++ b/Code/server/Components/PartyComponent.h @@ -8,6 +8,29 @@ struct PartyComponent { PartyComponent() {} + void ExpireInvitations(uint64_t aCurrentTick) noexcept + { + auto itor = Invitations.begin(); + while (itor != Invitations.end()) + { + if (itor->second <= aCurrentTick) + itor = Invitations.erase(itor); + else + ++itor; + } + } + + bool TryConsumeInvitation(uint32_t aInviterId, uint64_t aCurrentTick) noexcept + { + const auto itor = Invitations.find(aInviterId); + if (itor == Invitations.end()) + return false; + + const bool isValid = itor->second > aCurrentTick; + Invitations.erase(itor); + return isValid; + } + std::optional JoinedPartyId; - TiltedPhoques::Map Invitations; + TiltedPhoques::Map Invitations; }; diff --git a/Code/server/Services/PartyService.cpp b/Code/server/Services/PartyService.cpp index 5fc357781..a818f0af2 100644 --- a/Code/server/Services/PartyService.cpp +++ b/Code/server/Services/PartyService.cpp @@ -79,22 +79,9 @@ void PartyService::OnUpdate(const UpdateEvent& acEvent) noexcept // Only expire once every 10 seconds m_nextInvitationExpire = cCurrentTick + 10000; - auto view = m_world.view(); - for (auto entity : view) + for (Player* pPlayer : m_world.GetPlayerManager()) { - auto& partyComponent = view.get(entity); - auto itor = std::begin(partyComponent.Invitations); - while (itor != std::end(partyComponent.Invitations)) - { - if (itor->second < cCurrentTick) - { - itor = partyComponent.Invitations.erase(itor); - } - else - { - ++itor; - } - } + pPlayer->GetParty().ExpireInvitations(cCurrentTick); } } @@ -158,6 +145,9 @@ void PartyService::OnPartyChangeLeader(const PacketEventGetId() == pNewLeader->GetId()) { + if (party.LeaderPlayerId != pNewLeader->GetId()) + RevokeInvitations(party.LeaderPlayerId); + party.LeaderPlayerId = pPlayer->GetId(); spdlog::debug("[PartyService]: Changed party leader to {}, updating party members.", party.LeaderPlayerId); BroadcastPartyInfo(*inviterPartyComponent.JoinedPartyId); @@ -271,7 +261,7 @@ void PartyService::OnPartyInvite(const PacketEvent& acPacket // Expire in 60 seconds const auto cExpiryTick = GameServer::Get()->GetTick() + 60000; - inviteePartyComponent.Invitations[pInviter] = cExpiryTick; + inviteePartyComponent.Invitations[pInviter->GetId()] = cExpiryTick; NotifyPartyInvite notification; notification.InviterId = pInviter->GetId(); @@ -297,11 +287,6 @@ void PartyService::OnPartyAcceptInvite(const PacketEventGetParty(); auto& selfPartyComponent = pSelf->GetParty(); - // Check if we have this invitation so people don't invite themselves - if (selfPartyComponent.Invitations.count(pInviter) == 0) - return; - - spdlog::debug("[PartyService]: Invite found, processing."); if (!inviterPartyComponent.JoinedPartyId) // Ensure inviter is in a party otherwise break { spdlog::debug("[PartyService]: Inviter not in party. Cancelling."); @@ -324,6 +309,13 @@ void PartyService::OnPartyAcceptInvite(const PacketEventGetId(), GameServer::Get()->GetTick())) + { + spdlog::debug("[PartyService]: Invitation missing or expired, cancelling."); + return; + } + party.Members.push_back(pSelf); selfPartyComponent.JoinedPartyId = partyId; @@ -347,6 +339,8 @@ void PartyService::OnPlayerLeave(const PlayerLeaveEvent& acEvent) noexcept void PartyService::RemovePlayerFromParty(Player* apPlayer) noexcept { auto* pPartyComponent = &apPlayer->GetParty(); + pPartyComponent->Invitations.clear(); + RevokeInvitations(apPlayer->GetId()); spdlog::debug("[PartyService]: Removing player from party."); if (pPartyComponent->JoinedPartyId) @@ -381,6 +375,14 @@ void PartyService::RemovePlayerFromParty(Player* apPlayer) noexcept } } +void PartyService::RevokeInvitations(uint32_t aInviterId) noexcept +{ + for (Player* pPlayer : m_world.GetPlayerManager()) + { + pPlayer->GetParty().Invitations.erase(aInviterId); + } +} + void PartyService::BroadcastPlayerList(Player* apPlayer) const noexcept { auto pIgnoredPlayer = apPlayer; @@ -431,6 +433,8 @@ void PartyService::BroadcastPartyInfo(uint32_t aPartyId) const noexcept void PartyService::SendPartyJoinedEvent(Party& aParty, Player* aPlayer) noexcept { + aPlayer->GetParty().Invitations.clear(); + NotifyPartyJoined joinedMessage; joinedMessage.LeaderPlayerId = aParty.LeaderPlayerId; joinedMessage.IsLeader = aParty.LeaderPlayerId == aPlayer->GetId(); diff --git a/Code/server/Services/PartyService.h b/Code/server/Services/PartyService.h index e1803ca19..152e01dfc 100644 --- a/Code/server/Services/PartyService.h +++ b/Code/server/Services/PartyService.h @@ -47,6 +47,7 @@ struct PartyService void OnPartyChangeLeader(const PacketEvent& acPacket) noexcept; void OnPartyKick(const PacketEvent& acPacket) noexcept; void RemovePlayerFromParty(Player* apPlayer) noexcept; + void RevokeInvitations(uint32_t aInviterId) noexcept; void BroadcastPlayerList(Player* apPlayer = nullptr) const noexcept; void BroadcastPartyInfo(uint32_t aPartyId) const noexcept; diff --git a/Code/skyrim_ui/playwright/specs/group.e2e-spec.ts b/Code/skyrim_ui/playwright/specs/group.e2e-spec.ts index 750ddf13f..dc584ed97 100644 --- a/Code/skyrim_ui/playwright/specs/group.e2e-spec.ts +++ b/Code/skyrim_ui/playwright/specs/group.e2e-spec.ts @@ -1,84 +1,385 @@ import { createTest, expect } from '@ngx-playwright/test'; +import type { Page } from '@ngx-playwright/test'; import type { MockPlayer } from '../../src/app/mock/mock-player.js'; -import { createClassXPathSelector } from '../helpers/selector.js'; - import { ApplicationScreen } from '../screens/main-screen.js'; - const test = createTest(ApplicationScreen); -test.describe('Group', () => { +async function connect(page: Page) { + await page + .locator('.app-root-menu') + .getByRole('button', { name: 'Connect', exact: true }) + .click(); + await page.locator('app-connect input').nth(0).fill('test'); + await page.locator('app-connect input').nth(1).fill('test'); + await page.locator('app-connect app-action-buttons button').first().click(); + await expect( + page + .locator('.app-root-menu') + .getByRole('button', { name: 'Disconnect', exact: true }), + ).toBeVisible(); +} + +async function openPartyMenu(page: Page) { + if (!(await page.locator('app-player-manager').count())) { + await page + .locator('.app-root-menu') + .getByRole('button', { name: 'Player Manager', exact: true }) + .click(); + } + await page + .locator('app-player-manager') + .getByRole('button', { name: 'Party Menu', exact: true }) + .click(); +} + +async function openPlayerList(page: Page) { + await page + .locator('app-player-manager') + .getByRole('button', { name: 'Player List', exact: true }) + .click(); +} + +async function closePlayerManager(page: Page) { + await page + .locator('app-player-manager') + .getByRole('button', { name: 'Back', exact: true }) + .click(); + await expect(page.locator('app-player-manager')).toHaveCount(0); +} + +async function addPlayer(page: Page): Promise { + return page.evaluate('skyrimtogether.addMockPlayer()'); +} + +function inviteButton(page: Page, player: MockPlayer) { + return page + .locator('.player-list tbody tr') + .filter({ hasText: player.name }) + .getByRole('button', { name: 'Invite', exact: true }); +} +function receivedInvite(page: Page, player: MockPlayer) { + return page.locator('app-party-menu').getByRole('button', { + name: `Accept invite from ${player.name}`, + exact: true, + }); +} + +async function receiveInvite( + page: Page, + player: MockPlayer, + expiresInMs: number, +) { + await page.evaluate( + ({ id, expiresInMs }) => { + const mock = (window as any).skyrimtogether; + mock.startPlayerMockParty(id); + mock.emit('partyInviteReceived', id, expiresInMs); + }, + { id: player.id, expiresInMs }, + ); +} + +test.describe('Group', () => { test.beforeEach(async ({ page }) => { await page.waitForSelector('.app-root-controls', { state: 'attached' }); await page.press('body', 'F2'); await page.waitForSelector('.app-root-controls', { state: 'visible' }); + await connect(page); - // connect - await page.click(`//app-window${ createClassXPathSelector('app-root-menu') }/button[1]`); - await page.waitForSelector('//app-connect'); - await page.locator('//app-connect/div[1]/input[1]').fill('test'); - await page.locator('//app-connect/div[1]/input[2]').fill('test'); - await page.click('//app-connect/div[1]/app-action-buttons[1]/button[1]'); - await expect(page.locator(`//app-window${ createClassXPathSelector('app-root-menu') }/button[1]`)).toHaveText(/Disconnect/); + // Observe calls at the native bridge, without granting the UI an invitation. + await page.evaluate(() => { + const mock = (window as any).skyrimtogether; + for (const method of ['createPartyInvite', 'acceptPartyInvite']) { + const original = mock[method].bind(mock); + mock[`${method}Calls`] = []; + mock[method] = (id: number) => { + mock[`${method}Calls`].push(id); + original(id); + }; + } + }); }); test('Invite & Kick', async ({ page }) => { - const player = await page.evaluate('skyrimtogether.addMockPlayer()') as MockPlayer; - - // open player manager - await page.click(`//app-window${ createClassXPathSelector('app-root-menu') }/button[2]`); - await page.click(`//app-player-manager/div[1]/button[2]`); - - // launch party - await expect(await page.locator('.player-list-table').count()).toBe(0); - await page.click(`//app-party-menu[1]/div[1]/button[1]`); - await expect(await page.locator('.player-list-table').count()).toBe(0); - await expect(page.locator(`//app-party-menu/div[1]/span[1]`)).toHaveText(/There is nobody in your party yet/); - - // open playerlist and invite player - await page.click(`//app-player-manager/div[1]/button[1]`); - await page.click(`//div${ createClassXPathSelector('player-list') }/table[1]/tbody[1]/tr[1]/td[4]/button[1]`); - await expect(page.locator(`//div${ createClassXPathSelector('player-list') }/table[1]/tbody[1]/tr[1]/td[4]/button[1]`)).toBeDisabled(); - - // accept invite - await page.evaluate(`skyrimtogether.accteptMockPlayerInvite(${ player.id })`); - await expect(page.locator(`//div${ createClassXPathSelector('player-list') }/table[1]/tbody[1]/tr[1]/td[4]/button[1]`)).toBeDisabled(); - - // check member list - await page.click(`//app-player-manager/div[1]/button[2]`); - await expect(await page.locator(`//div${ createClassXPathSelector('member-list') }/table[1]/tbody[1]/tr`).count()).toBe(1); - await expect(page.locator(`//div${ createClassXPathSelector('member-list') }/table[1]/tbody[1]/tr[1]/td[1]`)).toHaveText(`${ player.level }`); - await expect(page.locator(`//div${ createClassXPathSelector('member-list') }/table[1]/tbody[1]/tr[1]/td[2]`)).toHaveText(player.name); - await expect(page.locator(`//div${ createClassXPathSelector('member-list') }/table[1]/tbody[1]/tr[1]/td[3]`)).toHaveText(player.cellName); - - // kick member - await page.click(`//div${ createClassXPathSelector('member-list') }/table[1]/tbody[1]/tr[1]/td[4]/button[2]`); - await expect(await page.locator('.player-list-table').count()).toBe(0); - - // check invite button - await page.click(`//app-player-manager/div[1]/button[1]`); - await expect(page.locator(`//div${ createClassXPathSelector('player-list') }/table[1]/tbody[1]/tr[1]/td[4]/button[1]`)).toBeEnabled(); + const player = await addPlayer(page); + await openPartyMenu(page); + await page + .getByRole('button', { name: 'Launch party', exact: true }) + .click(); + await expect(page.locator('app-party-menu .no-players')).toContainText( + 'There is nobody in your party yet', + ); + + await openPlayerList(page); + await inviteButton(page, player).click(); + await expect(inviteButton(page, player)).toBeDisabled(); + await page.evaluate(`skyrimtogether.accteptMockPlayerInvite(${player.id})`); + await expect(inviteButton(page, player)).toBeDisabled(); + + await openPartyMenu(page); + const member = page.locator('.member-list tbody tr'); + await expect(member).toHaveCount(1); + await expect(member.locator('td').nth(0)).toHaveText(`${player.level}`); + await expect(member.locator('td').nth(1)).toHaveText(player.name); + await expect(member.locator('td').nth(2)).toHaveText(player.cellName); + await member.getByRole('button', { name: 'Kick', exact: true }).click(); + await expect(member).toHaveCount(0); + + await openPlayerList(page); + await expect(inviteButton(page, player)).toBeEnabled(); + await inviteButton(page, player).click(); + await expect( + page.evaluate('skyrimtogether.createPartyInviteCalls'), + ).resolves.toEqual([player.id, player.id]); }); - test('Launch & Leave Party', async ({ page }) => { - // open player manager - await page.click(`//app-window${ createClassXPathSelector('app-root-menu') }/button[2]`); - await page.click(`//app-player-manager/div[1]/button[2]`); + test('Ignored invites can be retried after a short per-player cooldown', async ({ + page, + }) => { + const player = await addPlayer(page); + const otherPlayer = await addPlayer(page); + await openPartyMenu(page); + await page + .getByRole('button', { name: 'Launch party', exact: true }) + .click(); + await openPlayerList(page); - // launch party - await expect(page.locator(`//app-party-menu/div[1]/button[1]`)).toHaveText(/Launch party/); - await page.click(`//app-party-menu/div[1]/button[1]`); + await inviteButton(page, player).click(); + await expect(inviteButton(page, player)).toBeDisabled(); + await expect(inviteButton(page, otherPlayer)).toBeEnabled(); + await expect(inviteButton(page, player)).toBeEnabled({ timeout: 4500 }); + await inviteButton(page, player).click(); + await expect(inviteButton(page, player)).toBeDisabled(); + await expect( + page.evaluate('skyrimtogether.createPartyInviteCalls'), + ).resolves.toEqual([player.id, player.id]); + }); + + test('Received invitation expires and its stale popup cannot accept it', async ({ + page, + }) => { + const player = await addPlayer(page); + await openPartyMenu(page); + await receiveInvite(page, player, 1500); + await expect(receivedInvite(page, player)).toBeVisible(); + const popup = page + .locator('app-notification-popup') + .filter({ hasText: player.name }); + await closePlayerManager(page); + // Hover pauses the presentation timer, but must not extend the invitation. + await popup.hover(); + await page.waitForTimeout(1600); + await popup.getByRole('button', { name: 'Accept', exact: true }).click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([]); + await openPartyMenu(page); + await expect(receivedInvite(page, player)).toHaveCount(0); + await expect( + page.getByRole('button', { name: 'Launch party', exact: true }), + ).toBeVisible(); + }); + + test('Renewing an invite cancels its old deadline and invalidates its old popup', async ({ + page, + }) => { + const player = await addPlayer(page); + await openPartyMenu(page); + await receiveInvite(page, player, 1800); + const popups = page + .locator('app-notification-popup') + .filter({ hasText: player.name }); + await closePlayerManager(page); + await popups.first().hover(); + await receiveInvite(page, player, 6000); + await expect(popups).toHaveCount(2); + await popups + .first() + .getByRole('button', { name: 'Accept', exact: true }) + .click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([]); + await openPartyMenu(page); + await page.waitForTimeout(1900); + await expect(receivedInvite(page, player)).toBeVisible(); + await receivedInvite(page, player).click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([player.id]); + await expect( + page.getByRole('button', { name: 'Leave party', exact: true }), + ).toBeVisible(); + }); + + test('Joining and leaving clears every old received invite', async ({ + page, + }) => { + const player = await addPlayer(page); + const otherPlayer = await addPlayer(page); + await openPartyMenu(page); + await receiveInvite(page, player, 60000); + await receiveInvite(page, otherPlayer, 60000); + await receivedInvite(page, player).click(); + await page + .getByRole('button', { name: 'Leave party', exact: true }) + .click(); + await expect(receivedInvite(page, player)).toHaveCount(0); + await expect(receivedInvite(page, otherPlayer)).toHaveCount(0); + const stalePopup = page + .locator('app-notification-popup') + .filter({ hasText: otherPlayer.name }); + await closePlayerManager(page); + await stalePopup + .getByRole('button', { name: 'Accept', exact: true }) + .click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([player.id]); + + await receiveInvite(page, otherPlayer, 60000); + await openPartyMenu(page); + await receivedInvite(page, otherPlayer).click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([player.id, otherPlayer.id]); + }); - // leave party - await expect(page.locator(`//app-party-menu/button[1]`)).toHaveText(/Leave party/); - await page.click(`//app-party-menu/button[1]`); + test('A rejected acceptance preserves another valid invitation', async ({ + page, + }) => { + const player = await addPlayer(page); + const otherPlayer = await addPlayer(page); + await openPartyMenu(page); + await receiveInvite(page, player, 60000); + await receiveInvite(page, otherPlayer, 60000); + await page.evaluate(rejectedId => { + const mock = (window as any).skyrimtogether; + const accept = mock.acceptPartyInvite.bind(mock); + mock.acceptPartyInvite = (id: number) => { + if (id === rejectedId) { + // A stale server invite produces no successful partyInfo response. + mock.acceptPartyInviteCalls.push(id); + return; + } + accept(id); + }; + }, player.id); - // check if the launch button reappeared - await expect(page.locator(`//app-party-menu/div[1]/button[1]`)).toHaveText(/Launch party/); + await receivedInvite(page, player).click(); + await expect(receivedInvite(page, player)).toHaveCount(0); + await expect(receivedInvite(page, otherPlayer)).toBeVisible(); + await receivedInvite(page, otherPlayer).click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([player.id, otherPlayer.id]); + await expect( + page.getByRole('button', { name: 'Leave party', exact: true }), + ).toBeVisible(); + }); + + test('Leaving and relaunching permits an immediate fresh invite', async ({ + page, + }) => { + const player = await addPlayer(page); + await openPartyMenu(page); + await page + .getByRole('button', { name: 'Launch party', exact: true }) + .click(); + await openPlayerList(page); + await inviteButton(page, player).click(); + await openPartyMenu(page); + await page + .getByRole('button', { name: 'Leave party', exact: true }) + .click(); + await page + .getByRole('button', { name: 'Launch party', exact: true }) + .click(); + await openPlayerList(page); + await expect(inviteButton(page, player)).toBeEnabled(); + await inviteButton(page, player).click(); + await expect(inviteButton(page, player)).toBeDisabled(); + await expect( + page.evaluate('skyrimtogether.createPartyInviteCalls'), + ).resolves.toEqual([player.id, player.id]); + }); + + test('Disconnecting clears invites and rejects callbacks from the previous connection', async ({ + page, + }) => { + const player = await addPlayer(page); + await openPartyMenu(page); + await receiveInvite(page, player, 60000); + await page + .locator('app-player-manager') + .getByRole('button', { name: 'Back', exact: true }) + .click(); + await page + .locator('.app-root-menu') + .getByRole('button', { name: 'Disconnect', exact: true }) + .click(); + await page + .locator('app-disconnect') + .getByRole('button', { name: 'Proceed', exact: true }) + .click(); + await connect(page); + await openPartyMenu(page); + await expect(receivedInvite(page, player)).toHaveCount(0); + await receiveInvite(page, player, 60000); + const popups = page + .locator('app-notification-popup') + .filter({ hasText: player.name }); + await closePlayerManager(page); + await popups + .first() + .getByRole('button', { name: 'Accept', exact: true }) + .click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([]); + await openPartyMenu(page); + await receivedInvite(page, player).click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([player.id]); + }); + + test('A disconnected inviter cannot be accepted from its popup', async ({ + page, + }) => { + const player = await addPlayer(page); + await openPartyMenu(page); + await receiveInvite(page, player, 60000); + await page.evaluate(`skyrimtogether.disconnectMockPlayer(${player.id})`); + await expect(receivedInvite(page, player)).toHaveCount(0); + await closePlayerManager(page); + await page + .locator('app-notification-popup') + .filter({ hasText: player.name }) + .getByRole('button', { name: 'Accept', exact: true }) + .click(); + await expect( + page.evaluate('skyrimtogether.acceptPartyInviteCalls'), + ).resolves.toEqual([]); + }); - // close the player manager - await page.click(`//app-player-manager/div${ createClassXPathSelector('actions') }[1]//button[1]`); - await page.waitForSelector('//app-player-manager', { state: 'detached' }); + test('Launch & Leave Party', async ({ page }) => { + await openPartyMenu(page); + await page + .getByRole('button', { name: 'Launch party', exact: true }) + .click(); + await page + .getByRole('button', { name: 'Leave party', exact: true }) + .click(); + await expect( + page.getByRole('button', { name: 'Launch party', exact: true }), + ).toBeVisible(); + await page + .locator('app-player-manager') + .getByRole('button', { name: 'Back', exact: true }) + .click(); + await expect(page.locator('app-player-manager')).toHaveCount(0); }); }); diff --git a/Code/skyrim_ui/src/app/mock/skyrimtogether.mock.ts b/Code/skyrim_ui/src/app/mock/skyrimtogether.mock.ts index 50f2d4815..7cb92084e 100644 --- a/Code/skyrim_ui/src/app/mock/skyrimtogether.mock.ts +++ b/Code/skyrim_ui/src/app/mock/skyrimtogether.mock.ts @@ -310,9 +310,9 @@ export class SkyrimtogetherMock extends EventEmitter implements SkyrimTogether { ); } - inviteToPlayerMockParty(playerId: number) { + inviteToPlayerMockParty(playerId: number, expiresInMs = 60000) { playerStore.update(updateEntities(playerId, { invitedLocalPlayer: true })); - this.emit('partyInviteReceived', playerId); + this.emit('partyInviteReceived', playerId, expiresInMs); } startPlayerMockParty(playerId: number) { diff --git a/Code/skyrim_ui/src/app/models/party-invite.ts b/Code/skyrim_ui/src/app/models/party-invite.ts new file mode 100644 index 000000000..5ba91886d --- /dev/null +++ b/Code/skyrim_ui/src/app/models/party-invite.ts @@ -0,0 +1,4 @@ +export interface PartyInvite { + inviterId: number; + expiresInMs: number; +} diff --git a/Code/skyrim_ui/src/app/services/client.service.ts b/Code/skyrim_ui/src/app/services/client.service.ts index 2dc68f759..775b22c2c 100644 --- a/Code/skyrim_ui/src/app/services/client.service.ts +++ b/Code/skyrim_ui/src/app/services/client.service.ts @@ -4,6 +4,7 @@ import { AsyncSubject, BehaviorSubject, ReplaySubject, Subject } from 'rxjs'; import { environment } from '../../environments/environment'; import { Debug } from '../models/debug'; import { PartyInfo } from '../models/party-info'; +import { PartyInvite } from '../models/party-invite'; import { Player } from '../models/player'; import { ChatService } from './chat.service'; import { ErrorEvents, ErrorService } from './error.service'; @@ -53,7 +54,7 @@ export class ClientService implements OnDestroy { public partyLeftChange = new Subject(); /** Connect party invite received. */ - public partyInviteReceivedChange = new Subject(); + public partyInviteReceivedChange = new Subject(); /** Disconnect player to server change. */ public playerDisconnectedChange = new Subject(); @@ -619,7 +620,7 @@ export class ClientService implements OnDestroy { }); } - private onPartyInviteReceived(inviterId: number) { + private onPartyInviteReceived(inviterId: number, expiresInMs = 60000) { if (environment.game) { console.log( `%conPartyInviteReceived`, @@ -628,7 +629,7 @@ export class ClientService implements OnDestroy { ); } this.zone.run(() => { - this.partyInviteReceivedChange.next(inviterId); + this.partyInviteReceivedChange.next({ inviterId, expiresInMs }); }); } } diff --git a/Code/skyrim_ui/src/app/services/group.service.ts b/Code/skyrim_ui/src/app/services/group.service.ts index 706a19f93..77201e228 100644 --- a/Code/skyrim_ui/src/app/services/group.service.ts +++ b/Code/skyrim_ui/src/app/services/group.service.ts @@ -122,13 +122,6 @@ export class GroupService implements OnDestroy { for (let id of partyInfo.playerIds) { group.members.push(id); - - for (const player of playerList.players) { - if (player.id === id) { - player.hasBeenInvited = false; - break; - } - } } group.owner = partyInfo.leaderId; @@ -147,8 +140,6 @@ export class GroupService implements OnDestroy { const group = this.createGroup(this.group.getValue()); if (group) { - this.playerListService.resetHasBeenInvitedFlags(); - group.isEnabled = false; group.owner = undefined; group.members.splice(0); @@ -232,6 +223,7 @@ export class GroupService implements OnDestroy { if (group) { this.soundService.play(Sound.Focus); this.loadingService.setLoading(true); + this.playerListService.resetPartyInvitations(); this.clientService.launchParty(); group.isEnabled = true; @@ -248,6 +240,7 @@ export class GroupService implements OnDestroy { } this.soundService.play(Sound.Ok); + this.playerListService.resetPartyState(); this.clientService.leaveParty(); group.isEnabled = false; @@ -260,7 +253,7 @@ export class GroupService implements OnDestroy { public invite(playerId: number) { this.soundService.play(Sound.Ok); - this.clientService.createPartyInvite(playerId); + this.playerListService.sendPartyInvite(playerId); } async accept(inviterId: number) { @@ -279,7 +272,7 @@ export class GroupService implements OnDestroy { this.soundService.play(Sound.Ok); - this.clientService.acceptPartyInvite(inviterId); + this.playerListService.acceptPartyInvite(inviterId); } } diff --git a/Code/skyrim_ui/src/app/services/player-list.service.ts b/Code/skyrim_ui/src/app/services/player-list.service.ts index cc81ac68e..bd8fbeb56 100644 --- a/Code/skyrim_ui/src/app/services/player-list.service.ts +++ b/Code/skyrim_ui/src/app/services/player-list.service.ts @@ -1,6 +1,7 @@ import { Injectable, OnDestroy } from '@angular/core'; import { TranslocoService } from '@ngneat/transloco'; import { BehaviorSubject, Subscription } from 'rxjs'; +import { PartyInvite } from '../models/party-invite'; import { Player } from '../models/player'; import { PlayerList } from '../models/player-list'; import { PlayerManagerTab } from '../models/player-manager-tab.enum'; @@ -22,6 +23,17 @@ export class PlayerListService implements OnDestroy { private memberKickedSubscription: Subscription; private cellSubscription: Subscription; private partyInviteReceivedSubscription: Subscription; + private partyInfoSubscription: Subscription; + private partyLeftSubscription: Subscription; + + private readonly inviteCooldownMs = 3000; + private readonly sentInviteTimers = new Map(); + private readonly receivedInvites = new Map< + number, + { expiresAt: number; timer: number } + >(); + private partyLeaderId: number | undefined; + private partyMemberIds: number[] = []; private isConnect = false; @@ -38,15 +50,21 @@ export class PlayerListService implements OnDestroy { this.onMemberKicked(); this.onCellChange(); this.onPartyInviteReceived(); + this.onPartyInfo(); + this.onPartyLeft(); } ngOnDestroy() { + this.resetPartyInvitations(); this.debugSubscription.unsubscribe(); this.connectionSubscription.unsubscribe(); this.playerConnectedSubscription.unsubscribe(); this.playerDisconnectedSubscription.unsubscribe(); + this.memberKickedSubscription.unsubscribe(); this.cellSubscription.unsubscribe(); this.partyInviteReceivedSubscription.unsubscribe(); + this.partyInfoSubscription.unsubscribe(); + this.partyLeftSubscription.unsubscribe(); } private onDebug() { @@ -62,6 +80,7 @@ export class PlayerListService implements OnDestroy { return; } this.isConnect = connect; + this.resetPartyState(); this.playerList.next(undefined); this.updatePlayerList(); @@ -85,6 +104,8 @@ export class PlayerListService implements OnDestroy { this.playerDisconnectedSubscription = this.clientService.playerDisconnectedChange.subscribe( (playerDisco: Player) => { + this.clearSentInvite(playerDisco.id); + this.clearReceivedInvite(playerDisco.id); const playerList = this.getPlayerList(); if (playerList) { @@ -127,22 +148,69 @@ export class PlayerListService implements OnDestroy { private onPartyInviteReceived() { this.partyInviteReceivedSubscription = this.clientService.partyInviteReceivedChange.subscribe( - async (inviterId: number) => { - const playerList = this.getPlayerList(); + ({ inviterId, expiresInMs }: PartyInvite) => { + const invitingPlayer = this.getPlayerById(inviterId); + if ( + !this.isConnect || + this.partyLeaderId !== undefined || + !invitingPlayer || + inviterId === this.clientService.localPlayerId + ) { + return; + } - if (playerList) { - const invitingPlayer = this.getPlayerById(inviterId); - invitingPlayer.hasInvitedLocalPlayer = true; - this.playerList.next(playerList); - this.popupNotificationService.addPartyInvite( - invitingPlayer.name, - () => this.acceptPartyInvite(inviterId), - ); + this.clearReceivedInvite(inviterId); + if (!Number.isFinite(expiresInMs) || expiresInMs <= 0) { + this.updatePlayerList(); + return; } + + const invitation = { + expiresAt: performance.now() + expiresInMs, + timer: setTimeout(() => { + this.clearReceivedInvite(inviterId); + this.updatePlayerList(); + }, expiresInMs), + }; + this.receivedInvites.set(inviterId, invitation); + invitingPlayer.hasInvitedLocalPlayer = true; + this.updatePlayerList(); + this.popupNotificationService.addPartyInvite( + invitingPlayer.name, + () => { + // A popup for a replaced invite must not accept the newer one. + if (this.receivedInvites.get(inviterId) === invitation) { + this.acceptPartyInvite(inviterId); + } + }, + ); }, ); } + private onPartyInfo() { + this.partyInfoSubscription = this.clientService.partyInfoChange.subscribe( + partyInfo => { + if (this.partyLeaderId !== partyInfo.leaderId) { + this.resetHasBeenInvitedFlags(); + } + this.partyLeaderId = partyInfo.leaderId; + this.partyMemberIds = partyInfo.playerIds; + this.clearReceivedInvites(); + for (const playerId of partyInfo.playerIds) { + this.clearSentInvite(playerId); + } + this.updatePlayerList(); + }, + ); + } + + private onPartyLeft() { + this.partyLeftSubscription = this.clientService.partyLeftChange.subscribe( + () => this.resetPartyState(), + ); + } + public getLocalPlayer(): Player { let localPlayerId = this.clientService.localPlayerId; return this.getPlayerById(localPlayerId); @@ -169,27 +237,51 @@ export class PlayerListService implements OnDestroy { } public sendPartyInvite(inviteeId: number) { - const playerList = this.getPlayerList(); - - if (playerList) { - this.getPlayerById(inviteeId).hasBeenInvited = true; - - this.updatePlayerList(); - - this.clientService.createPartyInvite(inviteeId); + const player = this.getPlayerById(inviteeId); + if ( + !this.isConnect || + !player || + this.partyLeaderId !== this.clientService.localPlayerId || + inviteeId === this.clientService.localPlayerId || + this.partyMemberIds.includes(inviteeId) || + this.sentInviteTimers.has(inviteeId) + ) { + return; } + + player.hasBeenInvited = true; + this.sentInviteTimers.set( + inviteeId, + setTimeout(() => { + this.clearSentInvite(inviteeId); + this.updatePlayerList(); + }, this.inviteCooldownMs), + ); + this.updatePlayerList(); + this.clientService.createPartyInvite(inviteeId); } public acceptPartyInvite(inviterId: number) { - const playerList = this.getPlayerList(); - - if (playerList) { - this.getPlayerById(inviterId).hasInvitedLocalPlayer = false; - - this.clientService.acceptPartyInvite(inviterId); - - this.playerList.next(playerList); + const invitation = this.receivedInvites.get(inviterId); + if ( + !this.isConnect || + this.partyLeaderId !== undefined || + !this.getPlayerById(inviterId) || + !invitation + ) { + return; } + if (performance.now() >= invitation.expiresAt) { + this.clearReceivedInvite(inviterId); + this.updatePlayerList(); + return; + } + + // The server can reject this invite. Keep other invitations until partyInfo + // confirms a successful join, and consume this one before the bridge call. + this.clearReceivedInvite(inviterId); + this.updatePlayerList(); + this.clientService.acceptPartyInvite(inviterId); } public getPlayerById(playerId: number): Player { @@ -197,7 +289,11 @@ export class PlayerListService implements OnDestroy { } public resetHasBeenInvitedFlags() { - const playerList = this.getPlayerList(); + for (const timer of this.sentInviteTimers.values()) { + clearTimeout(timer); + } + this.sentInviteTimers.clear(); + const playerList = this.playerList.getValue(); if (playerList) { for (const player of playerList.players) { @@ -207,4 +303,44 @@ export class PlayerListService implements OnDestroy { this.updatePlayerList(); } } + + public resetPartyInvitations() { + this.resetHasBeenInvitedFlags(); + this.clearReceivedInvites(); + this.updatePlayerList(); + } + + public resetPartyState() { + this.partyLeaderId = undefined; + this.partyMemberIds = []; + this.resetPartyInvitations(); + } + + private clearSentInvite(playerId: number) { + clearTimeout(this.sentInviteTimers.get(playerId)); + this.sentInviteTimers.delete(playerId); + const player = this.playerList + .getValue() + ?.players.find(player => player.id === playerId); + if (player) { + player.hasBeenInvited = false; + } + } + + private clearReceivedInvite(playerId: number) { + clearTimeout(this.receivedInvites.get(playerId)?.timer); + this.receivedInvites.delete(playerId); + const player = this.playerList + .getValue() + ?.players.find(player => player.id === playerId); + if (player) { + player.hasInvitedLocalPlayer = false; + } + } + + private clearReceivedInvites() { + for (const playerId of this.receivedInvites.keys()) { + this.clearReceivedInvite(playerId); + } + } } diff --git a/Code/skyrim_ui/src/typings.d.ts b/Code/skyrim_ui/src/typings.d.ts index 0104f5d0c..682846fa4 100644 --- a/Code/skyrim_ui/src/typings.d.ts +++ b/Code/skyrim_ui/src/typings.d.ts @@ -93,7 +93,10 @@ declare namespace SkyrimTogetherTypes { type PartyLeftCallback = (inviterId: number) => void; - type PartyInviteReceivedCallback = (inviterId: number) => void; + type PartyInviteReceivedCallback = ( + inviterId: number, + expiresInMs?: number, + ) => void; } /** Global Skyrim: Together object. */ diff --git a/Code/tests/server/party_invitations.cpp b/Code/tests/server/party_invitations.cpp new file mode 100644 index 000000000..b3606fec5 --- /dev/null +++ b/Code/tests/server/party_invitations.cpp @@ -0,0 +1,62 @@ +#include + +#include + +TEST_CASE("Party invitation acceptance enforces its deadline without a cleanup pass", "[server.party]") +{ + PartyComponent party; + party.Invitations[1] = 60000; + party.Invitations[2] = 60000; + party.Invitations[3] = 60000; + + REQUIRE(party.TryConsumeInvitation(1, 59999)); + REQUIRE_FALSE(party.TryConsumeInvitation(1, 59999)); + REQUIRE_FALSE(party.TryConsumeInvitation(2, 60000)); + REQUIRE_FALSE(party.TryConsumeInvitation(3, 60001)); + REQUIRE(party.Invitations.empty()); +} + +TEST_CASE("Accepting an invitation cannot consume another player's invitation", "[server.party]") +{ + PartyComponent party; + party.Invitations[10] = 60000; + party.Invitations[20] = 61000; + + REQUIRE_FALSE(party.TryConsumeInvitation(30, 1000)); + REQUIRE(party.Invitations.size() == 2); + REQUIRE_FALSE(party.TryConsumeInvitation(10, 60000)); + REQUIRE(party.TryConsumeInvitation(20, 60000)); + REQUIRE(party.Invitations.empty()); +} + +TEST_CASE("Resending an invitation renews the deadline used by cleanup and acceptance", "[server.party]") +{ + PartyComponent party; + party.Invitations[10] = 60000; + party.Invitations[20] = 60000; + party.Invitations[10] = 90000; + + party.ExpireInvitations(60000); + + REQUIRE(party.Invitations.size() == 1); + REQUIRE_FALSE(party.TryConsumeInvitation(20, 60000)); + REQUIRE(party.TryConsumeInvitation(10, 89999)); + REQUIRE(party.Invitations.empty()); +} + +TEST_CASE("Party invitation cleanup preserves live invitations and removes expired ones", "[server.party]") +{ + PartyComponent party; + for (uint32_t inviter = 0; inviter < 100; ++inviter) + party.Invitations[inviter] = 60000 + inviter; + + party.ExpireInvitations(60049); + REQUIRE(party.Invitations.size() == 50); + for (uint32_t inviter = 0; inviter < 100; ++inviter) + REQUIRE(party.Invitations.contains(inviter) == (inviter >= 50)); + + party.ExpireInvitations(60099); + REQUIRE(party.Invitations.empty()); + party.ExpireInvitations(60100); + REQUIRE(party.Invitations.empty()); +} diff --git a/Code/tests/server/xmake.lua b/Code/tests/server/xmake.lua new file mode 100644 index 000000000..4b177457a --- /dev/null +++ b/Code/tests/server/xmake.lua @@ -0,0 +1,17 @@ +target("TPServerTests") + set_kind("binary") + set_group("Tests") + add_includedirs("../../server", "../../../Libraries") + set_pcxxheader("../../server/Pch.h") + add_files("../main.cpp", "*.cpp") + add_deps("SkyrimEncoding", "CommonLib", "Console", "TiltedConnect") + add_packages( + "tiltedcore", + "hopscotch-map", + "catch2", + "glm", + "entt", + "spdlog", + "gamenetworkingsockets", + "lua", + "sol2") diff --git a/Code/tests/xmake.lua b/Code/tests/xmake.lua index 1ce224fa8..38dac9756 100644 --- a/Code/tests/xmake.lua +++ b/Code/tests/xmake.lua @@ -13,3 +13,13 @@ target("TPTests") "catch2", "mimalloc", "glm") + +option("server_tests") + set_default(false) + set_showmenu(true) + set_description("Build isolated server component tests") +option_end() + +if has_config("server_tests") then + includes("server") +end