From d9cf5059a72ed2ae8435c68b61b18f3580c75d57 Mon Sep 17 00:00:00 2001 From: heinrich5991 Date: Sun, 3 Feb 2019 00:54:12 +0100 Subject: [PATCH 1/3] Refactor spec mode selection without case fallthrough This allows to turn on the warning for unexpected case fallthroughs. As a side effect, this also reduces code duplication, halving the number of lines required for this logic. --- src/game/client/components/spectator.cpp | 197 ++++++++--------------- src/game/client/components/spectator.h | 3 + 2 files changed, 71 insertions(+), 129 deletions(-) diff --git a/src/game/client/components/spectator.cpp b/src/game/client/components/spectator.cpp index afdc3442a..d93119499 100644 --- a/src/game/client/components/spectator.cpp +++ b/src/game/client/components/spectator.cpp @@ -30,150 +30,89 @@ void CSpectator::ConSpectate(IConsole::IResult *pResult, void *pUserData) pSelf->Spectate(pResult->GetInteger(0), pResult->GetInteger(1)); } -void CSpectator::ConSpectateNext(IConsole::IResult *pResult, void *pUserData) +bool CSpectator::SpecModePossible(int SpecMode, int SpectatorID) { - CSpectator *pSelf = (CSpectator *)pUserData; - if(!pSelf->m_pClient->m_Snap.m_SpecInfo.m_Active || (pSelf->Client()->State() == IClient::STATE_DEMOPLAYBACK && pSelf->DemoPlayer()->GetDemoType() != IDemoPlayer::DEMOTYPE_SERVER)) - return; - - int NewSpecMode = pSelf->m_pClient->m_Snap.m_SpecInfo.m_SpecMode; - int NewSpectatorID = -1; - bool GotNewSpectatorID = false; - - if(NewSpecMode != SPEC_PLAYER) - NewSpecMode = (NewSpecMode + 1) % NUM_SPECMODES; - else - NewSpectatorID = pSelf->m_pClient->m_Snap.m_SpecInfo.m_SpectatorID; - - switch(NewSpecMode) - { // drop through + int i = SpectatorID; + switch(SpecMode) + { case SPEC_PLAYER: - for(int i = NewSpectatorID + 1; i < MAX_CLIENTS; i++) + if(!m_pClient->m_aClients[i].m_Active || m_pClient->m_aClients[i].m_Team == TEAM_SPECTATORS) { - if(!pSelf->m_pClient->m_aClients[i].m_Active || pSelf->m_pClient->m_aClients[i].m_Team == TEAM_SPECTATORS) - continue; - - if(pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != TEAM_SPECTATORS && - (i == pSelf->m_pClient->m_LocalClientID || pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != pSelf->m_pClient->m_aClients[i].m_Team || - (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i] && (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i]->m_PlayerFlags&PLAYERFLAG_DEAD)))) - continue; - - NewSpectatorID = i; - GotNewSpectatorID = true; - break; + return false; } - if(GotNewSpectatorID) - break; - NewSpecMode = SPEC_FLAGRED; - NewSpectatorID = -1; + if(m_pClient->m_aClients[m_pClient->m_LocalClientID].m_Team != TEAM_SPECTATORS && + (i == m_pClient->m_LocalClientID || m_pClient->m_aClients[m_pClient->m_LocalClientID].m_Team != m_pClient->m_aClients[i].m_Team || + (m_pClient->m_Snap.m_paPlayerInfos[i] && (m_pClient->m_Snap.m_paPlayerInfos[i]->m_PlayerFlags&PLAYERFLAG_DEAD)))) + { + return false; + } + return true; case SPEC_FLAGRED: case SPEC_FLAGBLUE: - if(pSelf->m_pClient->m_GameInfo.m_GameFlags&GAMEFLAG_FLAGS) - { - GotNewSpectatorID = true; - break; - } - NewSpecMode = SPEC_FREEVIEW; + return m_pClient->m_GameInfo.m_GameFlags&GAMEFLAG_FLAGS; case SPEC_FREEVIEW: - if(pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team == TEAM_SPECTATORS) - GotNewSpectatorID = true; - else + return m_pClient->m_aClients[m_pClient->m_LocalClientID].m_Team == TEAM_SPECTATORS; + default: + dbg_assert(false, "invalid spec mode"); + return false; + } +} + +static void IterateSpecMode(int Direction, int *pSpecMode, int *pSpectatorID) +{ + dbg_assert(Direction == -1 || Direction == 1, "invalid direction"); + if(*pSpecMode == SPEC_PLAYER) + { + *pSpectatorID += Direction; + if(0 <= *pSpectatorID && *pSpectatorID < MAX_CLIENTS) { - // start again on the beginning in survival - for(int i = 0; i < pSelf->m_pClient->m_Snap.m_SpecInfo.m_SpectatorID; i++) - { - if(!pSelf->m_pClient->m_aClients[i].m_Active || pSelf->m_pClient->m_aClients[i].m_Team == TEAM_SPECTATORS) - continue; - - if(pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != TEAM_SPECTATORS && - (i == pSelf->m_pClient->m_LocalClientID || pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != pSelf->m_pClient->m_aClients[i].m_Team || - (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i] && (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i]->m_PlayerFlags&PLAYERFLAG_DEAD)))) - continue; - - NewSpecMode = SPEC_PLAYER; - NewSpectatorID = i; - GotNewSpectatorID = true; - break; - } + return; + } + *pSpectatorID = -1; + } + *pSpecMode = (*pSpecMode + Direction + NUM_SPECMODES) % NUM_SPECMODES; + if(*pSpecMode == SPEC_PLAYER) + { + *pSpectatorID = 0; + if(Direction == -1) + { + *pSpectatorID = MAX_CLIENTS - 1; } } +} - if(GotNewSpectatorID) - pSelf->Spectate(NewSpecMode, NewSpectatorID); +void CSpectator::HandleSpectateNextPrev(int Direction) +{ + if(!m_pClient->m_Snap.m_SpecInfo.m_Active || (Client()->State() == IClient::STATE_DEMOPLAYBACK && DemoPlayer()->GetDemoType() != IDemoPlayer::DEMOTYPE_SERVER)) + return; + + int NewSpecMode = m_pClient->m_Snap.m_SpecInfo.m_SpecMode; + int NewSpectatorID = -1; + if(NewSpecMode == SPEC_PLAYER) + { + NewSpectatorID = m_pClient->m_Snap.m_SpecInfo.m_SpectatorID; + } + + // Ensure the loop terminates even if no spec modes are possible. + for(int i = 0; i < NUM_SPECMODES + MAX_CLIENTS; i++) + { + IterateSpecMode(Direction, &NewSpecMode, &NewSpectatorID); + if(SpecModePossible(NewSpecMode, NewSpectatorID)) + { + Spectate(NewSpecMode, NewSpectatorID); + return; + } + } +} + +void CSpectator::ConSpectateNext(IConsole::IResult *pResult, void *pUserData) +{ + ((CSpectator *)pUserData)->HandleSpectateNextPrev(1); } void CSpectator::ConSpectatePrevious(IConsole::IResult *pResult, void *pUserData) { - CSpectator *pSelf = (CSpectator *)pUserData; - if(!pSelf->m_pClient->m_Snap.m_SpecInfo.m_Active || (pSelf->Client()->State() == IClient::STATE_DEMOPLAYBACK && pSelf->DemoPlayer()->GetDemoType() != IDemoPlayer::DEMOTYPE_SERVER)) - return; - - int NewSpecMode = pSelf->m_pClient->m_Snap.m_SpecInfo.m_SpecMode; - int NewSpectatorID = MAX_CLIENTS; - bool GotNewSpectatorID = false; - - if(NewSpecMode != SPEC_PLAYER) - NewSpecMode = (NewSpecMode - 1 + NUM_SPECMODES) % NUM_SPECMODES; - else - NewSpectatorID = pSelf->m_pClient->m_Snap.m_SpecInfo.m_SpectatorID; - - switch(NewSpecMode) - { // drop through - case SPEC_FLAGBLUE: - case SPEC_FLAGRED: - if(pSelf->m_pClient->m_GameInfo.m_GameFlags&GAMEFLAG_FLAGS) - { - NewSpectatorID = -1; - GotNewSpectatorID = true; - break; - } - NewSpecMode = SPEC_PLAYER; - NewSpectatorID = MAX_CLIENTS; - case SPEC_PLAYER: - for(int i = NewSpectatorID - 1; i >= 0; i--) - { - if(!pSelf->m_pClient->m_aClients[i].m_Active || pSelf->m_pClient->m_aClients[i].m_Team == TEAM_SPECTATORS) - continue; - - if(pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != TEAM_SPECTATORS && - (i == pSelf->m_pClient->m_LocalClientID || pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != pSelf->m_pClient->m_aClients[i].m_Team || - (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i] && (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i]->m_PlayerFlags&PLAYERFLAG_DEAD)))) - continue; - - NewSpectatorID = i; - GotNewSpectatorID = true; - break; - } - if(GotNewSpectatorID) - break; - NewSpecMode = SPEC_FREEVIEW; - case SPEC_FREEVIEW: - NewSpectatorID = -1; - if(pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team == TEAM_SPECTATORS) - GotNewSpectatorID = true; - else - { - // start again on the beginning in survival - for(int i = MAX_CLIENTS-1; i > pSelf->m_pClient->m_Snap.m_SpecInfo.m_SpectatorID; i--) - { - if(!pSelf->m_pClient->m_aClients[i].m_Active || pSelf->m_pClient->m_aClients[i].m_Team == TEAM_SPECTATORS) - continue; - - if(pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != TEAM_SPECTATORS && - (i == pSelf->m_pClient->m_LocalClientID || pSelf->m_pClient->m_aClients[pSelf->m_pClient->m_LocalClientID].m_Team != pSelf->m_pClient->m_aClients[i].m_Team || - (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i] && (pSelf->m_pClient->m_Snap.m_paPlayerInfos[i]->m_PlayerFlags&PLAYERFLAG_DEAD)))) - continue; - - NewSpecMode = SPEC_PLAYER; - NewSpectatorID = i; - GotNewSpectatorID = true; - break; - } - } - } - - if(GotNewSpectatorID) - pSelf->Spectate(NewSpecMode, NewSpectatorID); + ((CSpectator *)pUserData)->HandleSpectateNextPrev(-1); } CSpectator::CSpectator() diff --git a/src/game/client/components/spectator.h b/src/game/client/components/spectator.h index 685889eb8..763f872e4 100644 --- a/src/game/client/components/spectator.h +++ b/src/game/client/components/spectator.h @@ -20,6 +20,9 @@ class CSpectator : public CComponent int m_SelectedSpecMode; vec2 m_SelectorMouse; + bool SpecModePossible(int SpecMode, int SpectatorID); + void HandleSpectateNextPrev(int Direction); + static void ConKeySpectator(IConsole::IResult *pResult, void *pUserData); static void ConSpectate(IConsole::IResult *pResult, void *pUserData); static void ConSpectateNext(IConsole::IResult *pResult, void *pUserData); From c21cc0ad00334dbd7562add7f3d50e4112c378da Mon Sep 17 00:00:00 2001 From: heinrich5991 Date: Sun, 3 Feb 2019 00:55:10 +0100 Subject: [PATCH 2/3] Remove last remaining case fallthrough in the codebase --- src/game/server/gamecontroller.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/game/server/gamecontroller.cpp b/src/game/server/gamecontroller.cpp index 4ffaae7b0..be892a600 100644 --- a/src/game/server/gamecontroller.cpp +++ b/src/game/server/gamecontroller.cpp @@ -610,9 +610,9 @@ void IGameController::SetGameState(EGameState GameState, int Timer) } break; case IGS_END_ROUND: - if(DoWincheckMatch()) - break; case IGS_END_MATCH: + if(GameState == IGS_END_ROUND && DoWincheckMatch()) + break; // only possible when game is running or over if(m_GameState == IGS_GAME_RUNNING || m_GameState == IGS_END_MATCH || m_GameState == IGS_END_ROUND || m_GameState == IGS_GAME_PAUSED) { From dbf056b253ffd1f4ef3a6da21f961dafe37ff482 Mon Sep 17 00:00:00 2001 From: heinrich5991 Date: Sun, 3 Feb 2019 00:56:05 +0100 Subject: [PATCH 3/3] Simplify code using `str_comp` a bit --- src/engine/shared/console.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/engine/shared/console.cpp b/src/engine/shared/console.cpp index c967c5269..da087ee3c 100644 --- a/src/engine/shared/console.cpp +++ b/src/engine/shared/console.cpp @@ -723,7 +723,7 @@ void CConsole::ParseArguments(int NumArgs, const char **ppArguments) for(int i = 0; i < NumArgs; i++) { // check for scripts to execute - if(ppArguments[i][0] == '-' && ppArguments[i][1] == 'f' && ppArguments[i][2] == 0) + if(str_comp("-f", ppArguments[i]) == 0) { if(NumArgs - i > 1) ExecuteFile(ppArguments[i+1]);