games/vcmi
1
0
Fork 0
mirror of https://github.com/vcmi/vcmi.git synced 2024-12-24 22:14:36 +02:00
Code Issues Releases Activity

Merge pull request #3964 from IvanSavenko/stabilization

Browse Source 
[1.5.1] Stabilization
This commit is contained in:
Ivan Savenko 2024-05-14 16:22:55 +03:00 committed by GitHub
commit 7fb93613d5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 116 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ChangeLog.md
View File

@ -1,3 +1,14 @@
# 1.5.0 -> 1.5.1
### Stability
* Fixed possible crash on accessing faction description
* Fixed possible thread race on leaving to main menu
* Game will now show error message instead of silent crash on corrupted H3 data
* Fixed possible crash on double-deletion of quest artifacts placed by RMG
### Interface
* Fixed possible freeze on attempt to move hero when hero has non-zero movement points but not enough to reach first tile in path
# 1.4.5 -> 1.5.0
### General

2
android/vcmi-app/build.gradle
View File

@ -10,7 +10,7 @@ android {
applicationId "is.xyz.vcmi"
minSdk 19
targetSdk 33
versionCode 1510
versionCode 1511
versionName "1.5.1"
setProperty("archivesBaseName", "vcmi")
}

32
client/CMT.cpp
View File

@ -31,6 +31,7 @@
#include "../lib/CConfigHandler.h"
#include "../lib/CGeneralTextHandler.h"
#include "../lib/CThreadHelper.h"
#include "../lib/ExceptionsCommon.h"
#include "../lib/VCMIDirs.h"
#include "../lib/VCMI_Lib.h"
#include "../lib/filesystem/Filesystem.h"
@ -56,6 +57,7 @@ namespace po = boost::program_options;
namespace po_style = boost::program_options::command_line_style;
static std::atomic<bool> headlessQuit = false;
static std::optional<std::string> criticalInitializationError;
#ifndef VCMI_IOS
void processCommand(const std::string &message);
@ -69,9 +71,16 @@ static CBasicLogConfigurator *logConfig;
void init()
{
CStopWatch tmh;
loadDLLClasses();
CGI->setFromLib();
try
{
loadDLLClasses();
CGI->setFromLib();
}
catch (const DataLoadingException & e)
{
criticalInitializationError = e.what();
return;
}
logGlobal->info("Initializing VCMI_Lib: %d ms", tmh.getDiff());
@ -322,6 +331,11 @@ int main(int argc, char * argv[])
#endif // ANDROID
#endif // THREADED
if (criticalInitializationError.has_value())
{
handleFatalError(criticalInitializationError.value(), false);
}
if(!settings["session"]["headless"].Bool())
{
pomtime.getDiff();
@ -412,7 +426,7 @@ static void mainLoop()
}
}
[[noreturn]] static void quitApplicationImmediately()
[[noreturn]] static void quitApplicationImmediately(int error_code)
{
// Perform quick exit without executing static destructors and let OS cleanup anything that we did not
// We generally don't care about them and this leads to numerous issues, e.g.
@ -420,9 +434,9 @@ static void mainLoop()
// Android - std::quick_exit is available only starting from API level 21
// Mingw, macOS and iOS - std::quick_exit is unavailable (at least in current version of CI)
#if (defined(__ANDROID_API__) && __ANDROID_API__ < 21) || (defined(__MINGW32__)) || defined(VCMI_APPLE)
::exit(0);
::exit(error_code);
#else
std::quick_exit(0);
std::quick_exit(error_code);
#endif
}
@ -476,7 +490,7 @@ static void mainLoop()
}
std::cout << "Ending...\n";
quitApplicationImmediately();
quitApplicationImmediately(0);
}
void handleQuit(bool ask)
@ -500,7 +514,7 @@ void handleQuit(bool ask)
// proper solution would be to abort init thread (or wait for it to finish)
if (!CCS->curh)
{
quitApplicationImmediately();
quitApplicationImmediately(0);
}
if (LOCPLINT)
@ -521,5 +535,5 @@ void handleFatalError(const std::string & message, bool terminate)
if (terminate)
throw std::runtime_error(message);
else
exit(1);
quitApplicationImmediately(1);
}

11
client/CServerHandler.cpp
View File

@ -119,7 +119,10 @@ CServerHandler::~CServerHandler()
if (serverRunner)
serverRunner->wait();
serverRunner.reset();
threadNetwork.join();
{
auto unlockInterface = vstd::makeUnlockGuard(GH.interfaceMutex);
threadNetwork.join();
}
}
catch (const std::runtime_error & e)
{
@ -421,6 +424,7 @@ void CServerHandler::sendClientDisconnecting()
networkConnection->close();
networkConnection.reset();
logicConnection.reset();
waitForServerShutdown();
}
void CServerHandler::setCampaignState(std::shared_ptr<CampaignState> newCampaign)
@ -901,6 +905,8 @@ void CServerHandler::onPacketReceived(const std::shared_ptr<INetworkConnection>
void CServerHandler::onDisconnected(const std::shared_ptr<INetworkConnection> & connection, const std::string & errorMessage)
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if (connection != networkConnection)
{
// ServerHandler already closed this connection on its own
@ -919,8 +925,6 @@ void CServerHandler::onDisconnected(const std::shared_ptr<INetworkConnection> &
return;
}
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
logNetwork->error("Lost connection to server! Connection has been closed");
if(client)
@ -955,7 +959,6 @@ void CServerHandler::waitForServerShutdown()
}
else
{
boost::mutex::scoped_lock interfaceLock(GH.interfaceMutex);
if (getState() == EClientState::CONNECTING)
{
showServerError(CGI->generaltexth->translate("vcmi.server.errors.existingProcess"));

5
client/Client.cpp
View File

@ -519,7 +519,10 @@ void CClient::handlePack(CPack * pack)
{
apply->applyOnClBefore(this, pack);
logNetwork->trace("\tMade first apply on cl: %s", typeid(*pack).name());
gs->apply(pack);
{
boost::unique_lock<boost::shared_mutex> lock(CGameState::mutex);
gs->apply(pack);
}
logNetwork->trace("\tApplied on gs: %s", typeid(*pack).name());
apply->applyOnClAfter(this, pack);
logNetwork->trace("\tMade second apply on cl: %s", typeid(*pack).name());

2
client/adventureMap/AdventureMapShortcuts.cpp
View File

@ -456,7 +456,7 @@ bool AdventureMapShortcuts::optionHeroSelected()
bool AdventureMapShortcuts::optionHeroCanMove()
{
const auto * hero = LOCPLINT->localState->getCurrentHero();
return optionInMapView() && hero && hero->movementPointsRemaining() != 0 && LOCPLINT->localState->hasPath(hero);
return optionInMapView() && hero && LOCPLINT->localState->hasPath(hero) && LOCPLINT->localState->getPath(hero).nextNode().turns == 0;
}
bool AdventureMapShortcuts::optionHasNextHero()

8
client/widgets/Images.cpp
View File

@ -334,8 +334,12 @@ CShowableAnim::CShowableAnim(int x, int y, const AnimationPath & name, ui8 Flags
anim->loadGroup(group);
last = anim->size(group);
pos.w = anim->getImage(0, group)->width();
pos.h = anim->getImage(0, group)->height();
auto image = anim->getImage(0, group);
if (!image)
throw std::runtime_error("Failed to load group " + std::to_string(group) + " of animation file " + name.getOriginalName());
pos.w = image->width();
pos.h = image->height();
pos.x+= x;
pos.y+= y;

2
client/widgets/MiscWidgets.cpp
View File

@ -466,7 +466,7 @@ void CInteractableTownTooltip::init(const CGTownInstance * town)
if(town->id == townId && town->builtBuildings.count(BuildingID::TAVERN))
LOCPLINT->showTavernWindow(town, nullptr, QueryID::NONE);
}
}, [&]{
}, [town]{
if(!town->town->faction->getDescriptionTranslated().empty())
CRClickPopup::createAndPush(town->town->faction->getDescriptionTranslated());
});

2
client/windows/CCastleInterface.cpp
View File

@ -1351,7 +1351,7 @@ void CCastleInterface::recreateIcons()
{
if(town->builtBuildings.count(BuildingID::TAVERN))
LOCPLINT->showTavernWindow(town, nullptr, QueryID::NONE);
}, [&]{
}, [this]{
if(!town->town->faction->getDescriptionTranslated().empty())
CRClickPopup::createAndPush(town->town->faction->getDescriptionTranslated());
});

9
lib/CArtHandler.cpp
View File

@ -12,6 +12,7 @@
#include "ArtifactUtils.h"
#include "CGeneralTextHandler.h"
#include "ExceptionsCommon.h"
#include "GameSettings.h"
#include "mapObjects/MapObjects.h"
#include "constants/StringConstants.h"
@ -354,7 +355,13 @@ std::vector<JsonNode> CArtHandler::loadLegacyData()
artData["slot"].Vector().emplace_back(artSlot);
}
}
artData["class"].String() = classes.at(parser.readString()[0]);
std::string artClass = parser.readString();
if (classes.count(artClass[0]))
artData["class"].String() = classes.at(artClass[0]);
else
throw DataLoadingException("File ARTRAITS.TXT is invalid or corrupted! Please reinstall Heroes III data files");
artData["text"]["description"].String() = parser.readString();
parser.endLine();

1
lib/CMakeLists.txt
View File

@ -660,6 +660,7 @@ set(lib_MAIN_HEADERS
CStack.h
CStopWatch.h
CTownHandler.h
ExceptionsCommon.h
ExtraOptionsInfo.h
FunctionList.h
GameCallbackHolder.h

16
lib/ExceptionsCommon.h Normal file
View File

@ -0,0 +1,16 @@
/*
* ExceptionsCommon.h, part of VCMI engine
*
* Authors: listed in file AUTHORS in main folder
*
* License: GNU General Public License v2.0 or later
* Full text of license available in license.txt file, in main folder
*
*/
#pragma once
class DLL_LINKAGE DataLoadingException: public std::runtime_error
{
public:
using std::runtime_error::runtime_error;
};

42
lib/MetaString.cpp
View File

@ -144,33 +144,33 @@ DLL_LINKAGE std::string MetaString::toString() const
switch(elem)
{
case EMessage::APPEND_RAW_STRING:
dst += exactStrings[exSt++];
dst += exactStrings.at(exSt++);
break;
case EMessage::APPEND_LOCAL_STRING:
dst += getLocalString(localStrings[loSt++]);
dst += getLocalString(localStrings.at(loSt++));
break;
case EMessage::APPEND_TEXTID_STRING:
dst += VLC->generaltexth->translate(stringsTextID[textID++]);
dst += VLC->generaltexth->translate(stringsTextID.at(textID++));
break;
case EMessage::APPEND_NUMBER:
dst += std::to_string(numbers[nums++]);
dst += std::to_string(numbers.at(nums++));
break;
case EMessage::REPLACE_RAW_STRING:
boost::replace_first(dst, "%s", exactStrings[exSt++]);
boost::replace_first(dst, "%s", exactStrings.at(exSt++));
break;
case EMessage::REPLACE_LOCAL_STRING:
boost::replace_first(dst, "%s", getLocalString(localStrings[loSt++]));
boost::replace_first(dst, "%s", getLocalString(localStrings.at(loSt++)));
break;
case EMessage::REPLACE_TEXTID_STRING:
boost::replace_first(dst, "%s", VLC->generaltexth->translate(stringsTextID[textID++]));
boost::replace_first(dst, "%s", VLC->generaltexth->translate(stringsTextID.at(textID++)));
break;
case EMessage::REPLACE_NUMBER:
boost::replace_first(dst, "%d", std::to_string(numbers[nums++]));
boost::replace_first(dst, "%d", std::to_string(numbers.at(nums++)));
break;
case EMessage::REPLACE_POSITIVE_NUMBER:
if (dst.find("%+d") != std::string::npos)
{
int64_t value = numbers[nums];
int64_t value = numbers.at(nums);
if (value > 0)
boost::replace_first(dst, "%+d", '+' + std::to_string(value));
else
@ -179,7 +179,7 @@ DLL_LINKAGE std::string MetaString::toString() const
nums++;
}
else
boost::replace_first(dst, "%d", std::to_string(numbers[nums++]));
boost::replace_first(dst, "%d", std::to_string(numbers.at(nums++)));
break;
default:
logGlobal->error("MetaString processing error! Received message of type %d", static_cast<int>(elem));
@ -199,41 +199,41 @@ DLL_LINKAGE std::string MetaString::buildList() const
std::string lista;
for(int i = 0; i < message.size(); ++i)
{
if(i > 0 && (message[i] == EMessage::APPEND_RAW_STRING || message[i] == EMessage::APPEND_LOCAL_STRING))
if(i > 0 && (message.at(i) == EMessage::APPEND_RAW_STRING || message.at(i) == EMessage::APPEND_LOCAL_STRING))
{
if(exSt == exactStrings.size() - 1)
lista += VLC->generaltexth->allTexts[141]; //" and "
else
lista += ", ";
}
switch(message[i])
switch(message.at(i))
{
case EMessage::APPEND_RAW_STRING:
lista += exactStrings[exSt++];
lista += exactStrings.at(exSt++);
break;
case EMessage::APPEND_LOCAL_STRING:
lista += getLocalString(localStrings[loSt++]);
lista += getLocalString(localStrings.at(loSt++));
break;
case EMessage::APPEND_TEXTID_STRING:
lista += VLC->generaltexth->translate(stringsTextID[textID++]);
lista += VLC->generaltexth->translate(stringsTextID.at(textID++));
break;
case EMessage::APPEND_NUMBER:
lista += std::to_string(numbers[nums++]);
lista += std::to_string(numbers.at(nums++));
break;
case EMessage::REPLACE_RAW_STRING:
lista.replace(lista.find("%s"), 2, exactStrings[exSt++]);
lista.replace(lista.find("%s"), 2, exactStrings.at(exSt++));
break;
case EMessage::REPLACE_LOCAL_STRING:
lista.replace(lista.find("%s"), 2, getLocalString(localStrings[loSt++]));
lista.replace(lista.find("%s"), 2, getLocalString(localStrings.at(loSt++)));
break;
case EMessage::REPLACE_TEXTID_STRING:
lista.replace(lista.find("%s"), 2, VLC->generaltexth->translate(stringsTextID[textID++]));
lista.replace(lista.find("%s"), 2, VLC->generaltexth->translate(stringsTextID.at(textID++)));
break;
case EMessage::REPLACE_NUMBER:
lista.replace(lista.find("%d"), 2, std::to_string(numbers[nums++]));
lista.replace(lista.find("%d"), 2, std::to_string(numbers.at(nums++)));
break;
default:
logGlobal->error("MetaString processing error! Received message of type %d", int(message[i]));
logGlobal->error("MetaString processing error! Received message of type %d", int(message.at(i)));
}
}
return lista;

5
lib/gameState/CGameState.cpp
View File

@ -75,8 +75,6 @@ public:
void applyOnGS(CGameState *gs, CPack * pack) const override
{
T *ptr = static_cast<T*>(pack);
boost::unique_lock<boost::shared_mutex> lock(CGameState::mutex);
ptr->applyGs(gs);
}
};
@ -1101,6 +1099,9 @@ BattleField CGameState::battleGetBattlefieldType(int3 tile, CRandomGenerator & r
if(map->isCoastalTile(tile)) //coastal tile is always ground
return BattleField(*VLC->identifiers()->getIdentifier("core", "battlefield.sand_shore"));
if (t.terType->battleFields.empty())
throw std::runtime_error("Failed to find battlefield for terrain " + t.terType->getJsonKey());
return BattleField(*RandomGeneratorUtil::nextItem(t.terType->battleFields, rand));
}

2
lib/network/NetworkConnection.cpp
View File

@ -92,7 +92,7 @@ void NetworkConnection::onPacketReceived(const boost::system::error_code & ec, u
if (readBuffer.size() < expectedPacketSize)
{
throw std::runtime_error("Failed to read packet!");
throw std::runtime_error("Failed to read packet! " + std::to_string(readBuffer.size()) + " bytes read, but " + std::to_string(expectedPacketSize) + " bytes expected!");
}
std::vector<std::byte> message(expectedPacketSize);

16
lib/networkPacks/NetPacksLib.cpp
View File

@ -1579,7 +1579,7 @@ void ChangeStackCount::applyGs(CGameState * gs)
{
auto * srcObj = gs->getArmyInstance(army);
if(!srcObj)
logNetwork->error("[CRITICAL] ChangeStackCount: invalid army object %d, possible game state corruption.", army.getNum());
throw std::runtime_error("ChangeStackCount: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption.");
if(absoluteValue)
srcObj->setStackCount(slot, count);
@ -1591,7 +1591,7 @@ void SetStackType::applyGs(CGameState * gs)
{
auto * srcObj = gs->getArmyInstance(army);
if(!srcObj)
logNetwork->error("[CRITICAL] SetStackType: invalid army object %d, possible game state corruption.", army.getNum());
throw std::runtime_error("SetStackType: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption.");
srcObj->setStackType(slot, type);
}
@ -1600,7 +1600,7 @@ void EraseStack::applyGs(CGameState * gs)
{
auto * srcObj = gs->getArmyInstance(army);
if(!srcObj)
logNetwork->error("[CRITICAL] EraseStack: invalid army object %d, possible game state corruption.", army.getNum());
throw std::runtime_error("EraseStack: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption.");
srcObj->eraseStack(slot);
}
@ -1609,11 +1609,11 @@ void SwapStacks::applyGs(CGameState * gs)
{
auto * srcObj = gs->getArmyInstance(srcArmy);
if(!srcObj)
logNetwork->error("[CRITICAL] SwapStacks: invalid army object %d, possible game state corruption.", srcArmy.getNum());
throw std::runtime_error("SwapStacks: invalid army object " + std::to_string(srcArmy.getNum()) + ", possible game state corruption.");
auto * dstObj = gs->getArmyInstance(dstArmy);
if(!dstObj)
logNetwork->error("[CRITICAL] SwapStacks: invalid army object %d, possible game state corruption.", dstArmy.getNum());
throw std::runtime_error("SwapStacks: invalid army object " + std::to_string(dstArmy.getNum()) + ", possible game state corruption.");
CStackInstance * s1 = srcObj->detachStack(srcSlot);
CStackInstance * s2 = dstObj->detachStack(dstSlot);
@ -1627,18 +1627,18 @@ void InsertNewStack::applyGs(CGameState *gs)
if(auto * obj = gs->getArmyInstance(army))
obj->putStack(slot, new CStackInstance(type, count));
else
logNetwork->error("[CRITICAL] InsertNewStack: invalid army object %d, possible game state corruption.", army.getNum());
throw std::runtime_error("InsertNewStack: invalid army object " + std::to_string(army.getNum()) + ", possible game state corruption.");
}
void RebalanceStacks::applyGs(CGameState * gs)
{
auto * srcObj = gs->getArmyInstance(srcArmy);
if(!srcObj)
logNetwork->error("[CRITICAL] RebalanceStacks: invalid army object %d, possible game state corruption.", srcArmy.getNum());
throw std::runtime_error("RebalanceStacks: invalid army object " + std::to_string(srcArmy.getNum()) + ", possible game state corruption.");
auto * dstObj = gs->getArmyInstance(dstArmy);
if(!dstObj)
logNetwork->error("[CRITICAL] RebalanceStacks: invalid army object %d, possible game state corruption.", dstArmy.getNum());
throw std::runtime_error("RebalanceStacks: invalid army object " + std::to_string(dstArmy.getNum()) + ", possible game state corruption.");
StackLocation src(srcObj, srcSlot);
StackLocation dst(dstObj, dstSlot);

3
lib/rmg/CMapGenerator.cpp
View File

@ -126,6 +126,9 @@ std::unique_ptr<CMap> CMapGenerator::generate()
fillZones();
//updated guarded tiles will be calculated in CGameState::initMapObjects()
map->getZones().clear();
// undo manager keeps pointers to object that might be removed during gameplay. Remove them to prevent any hanging pointer after gameplay
map->getEditManager()->getUndoManager().clearAll();
}
catch (rmgException &e)
{