Skip to content

Commit

Permalink
World|SaveGame: Restore and check state of as many objects as possible
Browse files Browse the repository at this point in the history
A discrepancy during object state restoration does not halt the entire
process, but only causes a warning to be logged.

Fixes a crash in Hexen where the Mage's lightning weapon mobjs had their
lastEnemy pointer restored to an unmangled pointer value (that had been
truncated from 64 to 32 bits, no less), causing illegal memory access
when switching maps.

Suspiciously, some objects are still being reported as having lost their
targets, which does warrant further investigation.

IssueID #2275
  • Loading branch information
skyjake committed Aug 1, 2018
1 parent 274ec6d commit 985a15f
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 19 deletions.
40 changes: 24 additions & 16 deletions doomsday/apps/client/src/world/base/map.cpp
Expand Up @@ -3135,9 +3135,12 @@ String Map::objectsDescription() const
void Map::restoreObjects(Info const &objState, IThinkerMapping const &thinkerMapping) const
{
/// @todo Generalize from mobjs to all thinkers?
LOG_AS("Map::restoreObjects");

if (!gx.MobjStateAsInfo || !gx.MobjRestoreState) return;

bool problemsDetected = false;

// Look up all the mobjs.
QList<thinker_t const *> mobjs;
thinkers().forAll(0x3, [&mobjs] (thinker_t *th) {
Expand All @@ -3148,10 +3151,9 @@ void Map::restoreObjects(Info const &objState, IThinkerMapping const &thinkerMap
// Check that all objects are found in the state description.
if (objState.root().contents().size() != mobjs.size())
{
throw Error("Map::restoreObjects",
String::format("Incorrect number of objects: %i in map, %i in description",
mobjs.size(),
objState.root().contents().size()));
LOGDEV_MAP_WARNING("Different number of objects: %i in map, but got %i in restore data")
<< mobjs.size()
<< objState.root().contents().size();
}

// Check the cross-references.
Expand All @@ -3172,9 +3174,8 @@ void Map::restoreObjects(Info const &objState, IThinkerMapping const &thinkerMap
// Restore the state according to the serialized info.
gx.MobjRestoreState(found->as<MobjThinkerData>().mobj(), state);

#if defined (DENG2_DEBUG)
// Verify that the state is now correct.
{
// Verify that the state is now correct.
Info const currentDesc(gx.MobjStateAsInfo(found->as<MobjThinkerData>().mobj()));
Info::BlockElement const &currentState = currentDesc.root().contentsInOrder()
.first()->as<Info::BlockElement>();
Expand All @@ -3183,25 +3184,32 @@ void Map::restoreObjects(Info const &objState, IThinkerMapping const &thinkerMap
{
if (state.keyValue(key).text != currentState.keyValue(key).text)
{
throw Error("Map::restoreObjects",
String("Object %1 has mismatching '%2' (current:%3 != arch:%4)")
.arg(privateId)
.arg(key)
.arg(currentState.keyValue(key).text)
.arg(state.keyValue(key).text));
problemsDetected = true;
const String msg = String("Object %1 has mismatching '%2' (current:%3 != arch:%4)")
.arg(privateId)
.arg(key)
.arg(currentState.keyValue(key).text)
.arg(state.keyValue(key).text);
LOGDEV_MAP_WARNING("%s") << msg;
}
}
}
#endif
}
else
{
throw Error("Map::restoreObjects",
String::format("Failed to find thinker matching ID 0x%x", privateId));
LOGDEV_MAP_ERROR("Failed to find thinker matching ID 0x%x") << privateId;
}
}

LOGDEV_MSG("State of map objects has been restored");
if (problemsDetected)
{
LOG_MAP_WARNING("Map objects were not fully restored " DENG2_CHAR_MDASH
" gameplay may be affected (enable Developer log entries for details)");
}
else
{
LOGDEV_MAP_MSG("State of map objects has been restored");
}
}

void Map::serializeInternalState(Writer &to) const
Expand Down
10 changes: 7 additions & 3 deletions doomsday/apps/plugins/common/src/world/mobj.cpp
Expand Up @@ -648,7 +648,7 @@ void mobj_s::write(MapStateWriter *msw) const
break;
}

Writer_WriteInt32(writer, PTR2INT(mo->lastEnemy));
Writer_WriteInt32(writer, 0); //PTR2INT(mo->lastEnemy));
#elif __JHERETIC__
Writer_WriteInt16(writer, msw->serialIdFor(mo->generator));
#endif
Expand Down Expand Up @@ -887,12 +887,16 @@ int mobj_s::read(MapStateReader *msr)
#if __JHEXEN__
if(ver >= 4)
{
tracer = INT2PTR(mobj_t, Reader_ReadInt32(reader));
// This value has not been mangled properly.
/*tracer = */INT2PTR(mobj_t, Reader_ReadInt32(reader));
tracer = nullptr;
}

if(ver >= 4)
{
lastEnemy = INT2PTR(mobj_t, Reader_ReadInt32(reader));
// This value has not been mangled properly.
/*lastEnemy =*/ INT2PTR(mobj_t, Reader_ReadInt32(reader));
lastEnemy = nullptr;
}
#else
if(ver >= 5)
Expand Down

0 comments on commit 985a15f

Please sign in to comment.