1
0
mirror of https://github.com/vcmi/vcmi.git synced 2024-12-24 22:14:36 +02:00

Merge pull request #2966 from IvanSavenko/simplify_ui_lock

Simplified locking of game UI state
This commit is contained in:
Ivan Savenko 2023-10-04 16:50:23 +03:00 committed by GitHub
commit e322d0a084
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 80 additions and 96 deletions

View File

@ -473,8 +473,6 @@ static void quitApplication()
vstd::clear_pointer(console);// should be removed after everything else since used by logging
boost::this_thread::sleep_for(boost::chrono::milliseconds(750));//???
if(!settings["session"]["headless"].Bool())
GH.screenHandler().close();
@ -486,6 +484,10 @@ static void quitApplication()
}
std::cout << "Ending...\n";
// this method is always called from event/network threads, which keep interface mutex locked
// unlock it here to avoid assertion failure on GH destruction in exit()
GH.interfaceMutex.unlock();
exit(0);
}

View File

@ -104,7 +104,7 @@
#include "../lib/spells/CSpellHandler.h"
// The macro below is used to mark functions that are called by client when game state changes.
// They all assume that CPlayerInterface::pim mutex is locked.
// They all assume that interface mutex is locked.
#define EVENT_HANDLER_CALLED_BY_CLIENT
#define BATTLE_EVENT_POSSIBLE_RETURN \
@ -113,8 +113,6 @@
if (isAutoFightOn && !battleInt) \
return;
boost::recursive_mutex * CPlayerInterface::pim = new boost::recursive_mutex;
CPlayerInterface * LOCPLINT;
std::shared_ptr<BattleInterface> CPlayerInterface::battleInt;
@ -532,7 +530,6 @@ void CPlayerInterface::garrisonsChanged(ObjectInstanceID id1, ObjectInstanceID i
void CPlayerInterface::garrisonsChanged(std::vector<const CGObjectInstance *> objs)
{
boost::unique_lock<boost::recursive_mutex> un(*pim);
for (auto object : objs)
{
auto * hero = dynamic_cast<const CGHeroInstance*>(object);
@ -752,7 +749,7 @@ void CPlayerInterface::activeStack(const BattleID & battleID, const CStack * sta
{
//FIXME: we want client rendering to proceed while AI is making actions
// so unlock mutex while AI is busy since this might take quite a while, especially if hero has many spells
auto unlockPim = vstd::makeUnlockGuard(*pim);
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
autofightingAI->activeStack(battleID, stack);
return;
}
@ -768,11 +765,7 @@ void CPlayerInterface::activeStack(const BattleID & battleID, const CStack * sta
cb->battleMakeUnitAction(battleID, BattleAction::makeDefend(stack));
}
{
boost::unique_lock<boost::recursive_mutex> un(*pim);
battleInt->stackActivated(stack);
//Regeneration & mana drain go there
}
battleInt->stackActivated(stack);
}
void CPlayerInterface::battleEnd(const BattleID & battleID, const BattleResult *br, QueryID queryID)
@ -1015,8 +1008,6 @@ void CPlayerInterface::showInfoDialogAndWait(std::vector<Component> & components
void CPlayerInterface::showYesNoDialog(const std::string &text, CFunctionList<void()> onYes, CFunctionList<void()> onNo, const std::vector<std::shared_ptr<CComponent>> & components)
{
boost::unique_lock<boost::recursive_mutex> un(*pim);
movementController->requestMovementAbort();
LOCPLINT->showingDialog->setn(true);
CInfoWindow::showYesNoDialog(text, components, onYes, onNo, playerID);
@ -1114,7 +1105,6 @@ void CPlayerInterface::tileHidden(const std::unordered_set<int3> &pos)
void CPlayerInterface::openHeroWindow(const CGHeroInstance *hero)
{
boost::unique_lock<boost::recursive_mutex> un(*pim);
GH.windows().createAndPushWindow<CHeroWindow>(hero);
}
@ -1339,7 +1329,7 @@ void CPlayerInterface::showRecruitmentDialog(const CGDwelling *dwelling, const C
GH.windows().createAndPushWindow<CRecruitmentWindow>(dwelling, level, dst, recruitCb);
}
void CPlayerInterface::waitWhileDialog(bool unlockPim)
void CPlayerInterface::waitWhileDialog()
{
if (GH.amIGuiThread())
{
@ -1347,7 +1337,7 @@ void CPlayerInterface::waitWhileDialog(bool unlockPim)
return;
}
auto unlock = vstd::makeUnlockGuardIf(*pim, unlockPim);
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
boost::unique_lock<boost::mutex> un(showingDialog->mx);
while(showingDialog->data)
showingDialog->cond.wait(un);
@ -1385,8 +1375,8 @@ void CPlayerInterface::centerView (int3 pos, int focusTime)
{
GH.windows().totalRedraw();
{
auto unlockPim = vstd::makeUnlockGuard(*pim);
IgnoreEvents ignore(*this);
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
boost::this_thread::sleep_for(boost::chrono::milliseconds(focusTime));
}
}
@ -1819,14 +1809,14 @@ void CPlayerInterface::artifactDisassembled(const ArtifactLocation &al)
artWin->artifactDisassembled(al);
}
void CPlayerInterface::waitForAllDialogs(bool unlockPim)
void CPlayerInterface::waitForAllDialogs()
{
while(!dialogs.empty())
{
auto unlock = vstd::makeUnlockGuardIf(*pim, unlockPim);
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
boost::this_thread::sleep_for(boost::chrono::milliseconds(5));
}
waitWhileDialog(unlockPim);
waitWhileDialog();
}
void CPlayerInterface::proposeLoadingGame()

View File

@ -76,7 +76,6 @@ public: // TODO: make private
//minor interfaces
CondSh<bool> *showingDialog; //indicates if dialog box is displayed
static boost::recursive_mutex *pim;
bool makingTurn; //if player is already making his turn
CCastleInterface * castleInt; //nullptr if castle window isn't opened
@ -185,8 +184,8 @@ public: // public interface for use by client via LOCPLINT access
void showHeroExchange(ObjectInstanceID hero1, ObjectInstanceID hero2);
void showArtifactAssemblyDialog(const Artifact * artifact, const Artifact * assembledArtifact, CFunctionList<void()> onYes);
void waitWhileDialog(bool unlockPim = true);
void waitForAllDialogs(bool unlockPim = true);
void waitWhileDialog();
void waitForAllDialogs();
void openTownWindow(const CGTownInstance * town); //shows townscreen
void openHeroWindow(const CGHeroInstance * hero); //shows hero window with given hero

View File

@ -90,7 +90,7 @@ template<typename T> class CApplyOnLobby : public CBaseForLobbyApply
public:
bool applyOnLobbyHandler(CServerHandler * handler, void * pack) const override
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
T * ptr = static_cast<T *>(pack);
ApplyOnLobbyHandlerNetPackVisitor visitor(*handler);
@ -296,7 +296,7 @@ void CServerHandler::applyPacksOnLobbyScreen()
boost::unique_lock<boost::recursive_mutex> lock(*mx);
while(!packsForLobbyScreen.empty())
{
boost::unique_lock<boost::recursive_mutex> guiLock(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
CPackForLobby * pack = packsForLobbyScreen.front();
packsForLobbyScreen.pop_front();
CBaseForLobbyApply * apply = applier->getApplier(typeList.getTypeID(pack)); //find the applier
@ -614,8 +614,6 @@ void CServerHandler::startMapAfterConnection(std::shared_ptr<CMapInfo> to)
void CServerHandler::startGameplay(VCMI_LIB_WRAP_NAMESPACE(CGameState) * gameState)
{
setThreadName("startGameplay");
if(CMM)
CMM->disable();
client = new CClient();

View File

@ -367,7 +367,6 @@ void CClient::endGame()
GH.curInt = nullptr;
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
logNetwork->info("Ending current game!");
removeGUI();
@ -499,8 +498,6 @@ std::string CClient::aiNameForPlayer(bool battleAI, bool alliedToHuman)
void CClient::installNewPlayerInterface(std::shared_ptr<CGameInterface> gameInterface, PlayerColor color, bool battlecb)
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
playerint[color] = gameInterface;
logGlobal->trace("\tInitializing the interface for player %s", color.toString());
@ -513,8 +510,6 @@ void CClient::installNewPlayerInterface(std::shared_ptr<CGameInterface> gameInte
void CClient::installNewBattleInterface(std::shared_ptr<CBattleGameInterface> battleInterface, PlayerColor color, bool needCallback)
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
battleints[color] = battleInterface;
if(needCallback)
@ -531,7 +526,7 @@ void CClient::handlePack(CPack * pack)
CBaseForCLApply * apply = applier->getApplier(typeList.getTypeID(pack)); //find the applier
if(apply)
{
boost::unique_lock<boost::recursive_mutex> guiLock(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
apply->applyOnClBefore(this, pack);
logNetwork->trace("\tMade first apply on cl: %s", typeList.getTypeInfo(pack)->name());
gs->apply(pack);
@ -614,7 +609,6 @@ void CClient::battleStarted(const BattleInfo * info)
{
if(att || def)
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
CPlayerInterface::battleInt = std::make_shared<BattleInterface>(info->getBattleID(), leftSide.armyObject, rightSide.armyObject, leftSide.hero, rightSide.hero, att, def);
}
else if(settings["session"]["spectate"].Bool() && !settings["session"]["spectate-skip-battle"].Bool())
@ -622,7 +616,6 @@ void CClient::battleStarted(const BattleInfo * info)
//TODO: This certainly need improvement
auto spectratorInt = std::dynamic_pointer_cast<CPlayerInterface>(playerint[PlayerColor::SPECTATOR]);
spectratorInt->cb->onBattleStarted(info);
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
CPlayerInterface::battleInt = std::make_shared<BattleInterface>(info->getBattleID(), leftSide.armyObject, rightSide.armyObject, leftSide.hero, rightSide.hero, att, def, spectratorInt);
}
}
@ -648,15 +641,17 @@ void CClient::battleFinished(const BattleID & battleID)
void CClient::startPlayerBattleAction(const BattleID & battleID, PlayerColor color)
{
assert(vstd::contains(battleints, color));
auto battleint = battleints.at(color);
if(vstd::contains(battleints, color))
if (!battleint->human)
{
// we want to avoid locking gamestate and causing UI to freeze while AI is making turn
auto unlock = vstd::makeUnlockGuardIf(*CPlayerInterface::pim, !battleints[color]->human);
assert(vstd::contains(battleints, color));
battleints[color]->activeStack(battleID, gs->getBattle(battleID)->battleGetStackByID(gs->getBattle(battleID)->activeStack, false));
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
battleint->activeStack(battleID, gs->getBattle(battleID)->battleGetStackByID(gs->getBattle(battleID)->activeStack, false));
}
else
{
battleint->activeStack(battleID, gs->getBattle(battleID)->battleGetStackByID(gs->getBattle(battleID)->activeStack, false));
}
}

View File

@ -74,7 +74,8 @@ void ClientCommandManager::handleGoSoloCommand()
{
Settings session = settings.write["session"];
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(!CSH->client)
{
printCommandMessage("Game is not in playing state");
@ -120,7 +121,8 @@ void ClientCommandManager::handleControlaiCommand(std::istringstream& singleWord
singleWordBuffer >> colorName;
boost::to_lower(colorName);
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(!CSH->client)
{
printCommandMessage("Game is not in playing state");
@ -416,14 +418,6 @@ void ClientCommandManager::handleSetCommand(std::istringstream& singleWordBuffer
}
}
void ClientCommandManager::handleUnlockCommand(std::istringstream& singleWordBuffer)
{
std::string mxname;
singleWordBuffer >> mxname;
if(mxname == "pim" && LOCPLINT)
LOCPLINT->pim->unlock();
}
void ClientCommandManager::handleCrashCommand()
{
int* ptr = nullptr;
@ -460,7 +454,7 @@ void ClientCommandManager::printCommandMessage(const std::string &commandMessage
if(currentCallFromIngameConsole)
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(LOCPLINT && LOCPLINT->cingconsole)
{
LOCPLINT->cingconsole->print(commandMessage);
@ -547,9 +541,6 @@ void ClientCommandManager::processCommand(const std::string & message, bool call
else if (commandName == "set")
handleSetCommand(singleWordBuffer);
else if(commandName == "unlock")
handleUnlockCommand(singleWordBuffer);
else if(commandName == "crash")
handleCrashCommand();

View File

@ -78,9 +78,6 @@ class ClientCommandManager //take mantis #2292 issue about account if thinking a
// set <command> <on/off> - sets special temporary settings that reset on game quit.
void handleSetCommand(std::istringstream& singleWordBuffer);
// Unlocks specific mutex known in VCMI code as "pim"
void handleUnlockCommand(std::istringstream& singleWordBuffer);
// Crashes the game forcing an exception
void handleCrashCommand();

View File

@ -106,7 +106,6 @@ void BattleInterface::playIntroSoundAndUnlockInterface()
{
auto onIntroPlayed = [this]()
{
boost::unique_lock<boost::recursive_mutex> un(*CPlayerInterface::pim);
if(LOCPLINT->battleInt)
onIntroSoundPlayed();
};
@ -781,7 +780,7 @@ void BattleInterface::onAnimationsFinished()
void BattleInterface::waitForAnimations()
{
{
auto unlockPim = vstd::makeUnlockGuard(*CPlayerInterface::pim);
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
ongoingAnimationsState.waitUntil(false);
}

View File

@ -113,32 +113,41 @@ bool InputHandler::ignoreEventsUntilInput()
void InputHandler::preprocessEvent(const SDL_Event & ev)
{
if((ev.type==SDL_QUIT) ||(ev.type == SDL_KEYDOWN && ev.key.keysym.sym==SDLK_F4 && (ev.key.keysym.mod & KMOD_ALT)))
if(ev.type == SDL_QUIT)
{
#ifdef VCMI_ANDROID
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
handleQuit(false);
#else
handleQuit();
#endif
return;
}
#ifdef VCMI_ANDROID
else if (ev.type == SDL_KEYDOWN && ev.key.keysym.scancode == SDL_SCANCODE_AC_BACK)
else if(ev.type == SDL_KEYDOWN)
{
handleQuit(true);
}
#endif
else if(ev.type == SDL_KEYDOWN && ev.key.keysym.sym==SDLK_F4)
{
boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
Settings full = settings.write["video"]["fullscreen"];
full->Bool() = !full->Bool();
if(ev.key.keysym.sym == SDLK_F4 && (ev.key.keysym.mod & KMOD_ALT))
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
handleQuit(true);
return;
}
GH.onScreenResize();
return;
if(ev.key.keysym.scancode == SDL_SCANCODE_AC_BACK)
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
handleQuit(true);
return;
}
if(ev.type == SDL_KEYDOWN && ev.key.keysym.sym == SDLK_F4)
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
Settings full = settings.write["video"]["fullscreen"];
full->Bool() = !full->Bool();
GH.onScreenResize();
return;
}
}
else if(ev.type == SDL_USEREVENT)
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
handleUserEvent(ev.user);
return;
@ -149,21 +158,27 @@ void InputHandler::preprocessEvent(const SDL_Event & ev)
case SDL_WINDOWEVENT_RESTORED:
#ifndef VCMI_IOS
{
boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
GH.onScreenResize();
}
#endif
break;
case SDL_WINDOWEVENT_FOCUS_GAINED:
if(settings["general"]["enableUiEnhancements"].Bool()) {
CCS->musich->setVolume(settings["general"]["music"].Integer());
CCS->soundh->setVolume(settings["general"]["sound"].Integer());
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(settings["general"]["enableUiEnhancements"].Bool()) {
CCS->musich->setVolume(settings["general"]["music"].Integer());
CCS->soundh->setVolume(settings["general"]["sound"].Integer());
}
}
break;
case SDL_WINDOWEVENT_FOCUS_LOST:
if(settings["general"]["enableUiEnhancements"].Bool()) {
CCS->musich->setVolume(0);
CCS->soundh->setVolume(0);
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(settings["general"]["enableUiEnhancements"].Bool()) {
CCS->musich->setVolume(0);
CCS->soundh->setVolume(0);
}
}
break;
}
@ -171,6 +186,7 @@ void InputHandler::preprocessEvent(const SDL_Event & ev)
}
else if(ev.type == SDL_SYSWMEVENT)
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(!settings["session"]["headless"].Bool() && settings["general"]["notifications"].Bool())
{
NotificationHandler::handleSdlEvent(ev);
@ -180,6 +196,7 @@ void InputHandler::preprocessEvent(const SDL_Event & ev)
//preprocessing
if(ev.type == SDL_MOUSEMOTION)
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if (CCS && CCS->curh)
CCS->curh->cursorMove(ev.motion.x, ev.motion.y);
}

View File

@ -95,8 +95,6 @@ void CGuiHandler::handleEvents()
void CGuiHandler::fakeMouseMove()
{
dispatchMainThread([](){
assert(CPlayerInterface::pim);
boost::unique_lock lock(*CPlayerInterface::pim);
GH.events().dispatchMouseMoved(Point(0, 0), GH.getCursorPosition());
});
}
@ -114,7 +112,7 @@ void CGuiHandler::stopTextInput()
void CGuiHandler::renderFrame()
{
{
boost::recursive_mutex::scoped_lock un(*CPlayerInterface::pim);
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if(nullptr != curInt)
curInt->update();

View File

@ -48,6 +48,8 @@ private:
std::unique_ptr<InputHandler> inputHandlerInstance;
public:
boost::mutex interfaceMutex;
/// returns current position of mouse cursor, relative to vcmi window
const Point & getCursorPosition() const;

View File

@ -304,8 +304,6 @@ CMainMenu::CMainMenu()
CMainMenu::~CMainMenu()
{
boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
if(GH.curInt == this)
GH.curInt = nullptr;
}

View File

@ -15,6 +15,7 @@
#include "../CCallback.h"
#include "../CGameInfo.h"
#include "../CPlayerInterface.h"
#include "../gui/CGuiHandler.h"
#include "../../lib/CGeneralTextHandler.h"
#include "../../lib/TerrainHandler.h"
@ -37,7 +38,7 @@ void CMapHandler::waitForOngoingAnimations()
{
while(CGI->mh->hasOngoingAnimations())
{
auto unlockPim = vstd::makeUnlockGuard(*CPlayerInterface::pim);
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
boost::this_thread::sleep_for(boost::chrono::milliseconds(1));
}
}

View File

@ -287,6 +287,7 @@ bool ArtifactUtilsClient::askToAssemble(const CGHeroInstance * hero, const Artif
{
auto askThread = new boost::thread([hero, art, slot, assemblyPossibilities]() -> void
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
for(const auto combinedArt : assemblyPossibilities)
{
bool assembleConfirmed = false;
@ -294,7 +295,7 @@ bool ArtifactUtilsClient::askToAssemble(const CGHeroInstance * hero, const Artif
onYesHandlers += std::bind(&CCallback::assembleArtifacts, LOCPLINT->cb.get(), hero, slot, true, combinedArt->getId());
LOCPLINT->showArtifactAssemblyDialog(art->artType, combinedArt, onYesHandlers);
LOCPLINT->waitWhileDialog(false);
LOCPLINT->waitWhileDialog();
if(assembleConfirmed)
break;
}

View File

@ -289,7 +289,6 @@ void GeneralOptionsTab::setGameResolution(int index)
widget<CLabel>("resolutionLabel")->setText(resolutionToLabelString(resolution.x, resolution.y));
GH.dispatchMainThread([](){
boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
GH.onScreenResize();
});
}
@ -314,7 +313,6 @@ void GeneralOptionsTab::setFullscreenMode(bool on, bool exclusive)
updateResolutionSelector();
GH.dispatchMainThread([](){
boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
GH.onScreenResize();
});
}
@ -374,7 +372,6 @@ void GeneralOptionsTab::setGameScaling(int index)
widget<CLabel>("scalingLabel")->setText(scalingToLabelString(scaling));
GH.dispatchMainThread([](){
boost::unique_lock<boost::recursive_mutex> lock(*CPlayerInterface::pim);
GH.onScreenResize();
});
}

View File

@ -131,6 +131,5 @@ Below a list of supported commands, with their arguments wrapped in `<>`
`activate <0/1/2>` - activate game windows (no current use, apparently broken long ago)
`redraw` - force full graphical redraw
`screen` - show value of screenBuf variable, which prints "screen" when adventure map has current focus, "screen2" otherwise, and dumps values of both screen surfaces to .bmp files
`unlock pim` - unlocks specific mutex known in VCMI code as "pim"
`not dialog` - set the state indicating if dialog box is active to "no"
`tell hs <hero ID> <artifact slot ID>` - write what artifact is present on artifact slot with specified ID for hero with specified ID. (must be called during gameplay)