From 0ee442e0b826b909f5826b636848b9db762de53b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20B=C3=A1lint=20Misius?= Date: Thu, 12 Dec 2024 15:09:16 +0100 Subject: [PATCH] Fix crash when navigating away from a save while voting on it By tying the lifetimes of the vote request and the queued vote to the online save info. Broken since c73fa1bcdde3, where the vote request was made non-blocking. TODO: Let the user know that their vote may not have gone through if the vote request gets destroyed abnormally. --- src/gui/game/GameModel.cpp | 56 ++++++++++++++++++++------------------ src/gui/game/GameModel.h | 10 +++++-- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/gui/game/GameModel.cpp b/src/gui/game/GameModel.cpp index a13321560..e725205db 100644 --- a/src/gui/game/GameModel.cpp +++ b/src/gui/game/GameModel.cpp @@ -556,33 +556,33 @@ void GameModel::SetUndoHistoryLimit(unsigned int undoHistoryLimit_) void GameModel::SetVote(int direction) { - queuedVote = direction; + currentSave.queuedVote = direction; } void GameModel::Tick() { - if (execVoteRequest && execVoteRequest->CheckDone()) + if (currentSave.execVoteRequest && currentSave.execVoteRequest->CheckDone()) { try { - execVoteRequest->Finish(); - currentSave->vote = execVoteRequest->Direction(); + currentSave.execVoteRequest->Finish(); + currentSave.saveInfo->vote = currentSave.execVoteRequest->Direction(); notifySaveChanged(); } catch (const http::RequestError &ex) { new ErrorMessage("Error while voting", ByteString(ex.what()).FromUtf8()); } - execVoteRequest.reset(); + currentSave.execVoteRequest.reset(); } - if (!execVoteRequest && queuedVote) + if (!currentSave.execVoteRequest && currentSave.queuedVote) { - if (currentSave) + if (currentSave.saveInfo) { - execVoteRequest = std::make_unique(currentSave->GetID(), *queuedVote); - execVoteRequest->Start(); + currentSave.execVoteRequest = std::make_unique(currentSave.saveInfo->GetID(), *currentSave.queuedVote); + currentSave.execVoteRequest->Start(); } - queuedVote.reset(); + currentSave.queuedVote.reset(); } } @@ -717,13 +717,15 @@ std::vector GameModel::GetMenuList() SaveInfo *GameModel::GetSave() // non-owning { - return currentSave.get(); + return currentSave.saveInfo.get(); } std::unique_ptr GameModel::TakeSave() { // we don't notify listeners because we'll get a new save soon anyway - return std::move(currentSave); + SaveInfoWrapper empty; + std::swap(empty, currentSave); + return std::move(empty.saveInfo); } void GameModel::SaveToSimParameters(const GameSave &saveData) @@ -753,12 +755,12 @@ void GameModel::SaveToSimParameters(const GameSave &saveData) void GameModel::SetSave(std::unique_ptr newSave, bool invertIncludePressure) { - currentSave = std::move(newSave); + currentSave = { std::move(newSave) }; currentFile.reset(); - if (currentSave && currentSave->GetGameSave()) + if (currentSave.saveInfo && currentSave.saveInfo->GetGameSave()) { - auto *saveData = currentSave->GetGameSave(); + auto *saveData = currentSave.saveInfo->GetGameSave(); SaveToSimParameters(*saveData); sim->clear_sim(); view->PauseRendererThread(); @@ -768,23 +770,23 @@ void GameModel::SetSave(std::unique_ptr newSave, bool invertIncludePre // Add in the correct info if (saveData->authors.size() == 0) { - auto gameSave = currentSave->TakeGameSave(); + auto gameSave = currentSave.saveInfo->TakeGameSave(); gameSave->authors["type"] = "save"; - gameSave->authors["id"] = currentSave->id; - gameSave->authors["username"] = currentSave->userName; - gameSave->authors["title"] = currentSave->name.ToUtf8(); - gameSave->authors["description"] = currentSave->Description.ToUtf8(); - gameSave->authors["published"] = (int)currentSave->Published; - gameSave->authors["date"] = (Json::Value::UInt64)currentSave->updatedDate; - currentSave->SetGameSave(std::move(gameSave)); + gameSave->authors["id"] = currentSave.saveInfo->id; + gameSave->authors["username"] = currentSave.saveInfo->userName; + gameSave->authors["title"] = currentSave.saveInfo->name.ToUtf8(); + gameSave->authors["description"] = currentSave.saveInfo->Description.ToUtf8(); + gameSave->authors["published"] = (int)currentSave.saveInfo->Published; + gameSave->authors["date"] = (Json::Value::UInt64)currentSave.saveInfo->updatedDate; + currentSave.saveInfo->SetGameSave(std::move(gameSave)); } // This save was probably just created, and we didn't know the ID when creating it // Update with the proper ID else if (saveData->authors.get("id", -1) == 0 || saveData->authors.get("id", -1) == -1) { - auto gameSave = currentSave->TakeGameSave(); - gameSave->authors["id"] = currentSave->id; - currentSave->SetGameSave(std::move(gameSave)); + auto gameSave = currentSave.saveInfo->TakeGameSave(); + gameSave->authors["id"] = currentSave.saveInfo->id; + currentSave.saveInfo->SetGameSave(std::move(gameSave)); } Client::Ref().OverwriteAuthorInfo(saveData->authors); } @@ -806,7 +808,7 @@ std::unique_ptr GameModel::TakeSaveFile() void GameModel::SetSaveFile(std::unique_ptr newSave, bool invertIncludePressure) { currentFile = std::move(newSave); - currentSave.reset(); + currentSave = {}; if (currentFile && currentFile->GetGameSave()) { diff --git a/src/gui/game/GameModel.h b/src/gui/game/GameModel.h index b7bf36b20..0aa1fa65d 100644 --- a/src/gui/game/GameModel.h +++ b/src/gui/game/GameModel.h @@ -51,7 +51,6 @@ struct HistoryEntry class GameModel { - std::unique_ptr execVoteRequest; private: std::vector notifications; @@ -74,7 +73,13 @@ private: int activeMenu; int currentBrush; std::vector> brushList; - std::unique_ptr currentSave; + struct SaveInfoWrapper + { + std::unique_ptr saveInfo; + std::optional queuedVote; + std::unique_ptr execVoteRequest; + }; + SaveInfoWrapper currentSave; std::unique_ptr currentFile; Tool *lastTool = nullptr; Tool **activeTools = nullptr; @@ -130,7 +135,6 @@ private: void SaveToSimParameters(const GameSave &saveData); - std::optional queuedVote; bool threadedRendering = false; GameView *view;