Engine does not restore current time while loading entities from save games

[SamVanheer]

When a save game is being loaded the entities loaded from the file will be recreated and have Restore called on them. Some entities use gpGlobals->time to set a delayed think function to finalize restore at a point where the player entity has been recreated (the player isn't fully recreated until after load has completed and the player has connected to the local server).

However the current time isn't assigned to gpGlobals->time when this happens. As a result all restore methods that use the current time use the wrong time value, causing the think functions to execute before the player has been connected and created.

The time value extracted from the save game should be assigned to sv.time before entities are loaded, and gpGlobals->time should be synchronized to that value every time the engine calls into the server dll, like it does with think functions:
https://github.com/id-Software/Quake/blob/bf4ac424ce754894ac8f1dae6a3981954bc9852d/WinQuake/sv_phys.c#L116-L144

In this case it should always use the current time, not the time in pev->nextthink.

One known case that is affected is trigger_playerfreeze in Blue Shift. It's used once, in ba_outro. Saving and loading while frozen lets the player move around.

#1014 is related to that case.

[SamVanheer]

Looks like this can be fixed in game code. The saved time value is accessible in this type:

halflife/engine/eiface.h

Lines 342 to 370 in c7240b9

	#ifdef _WIN32
	typedef
	#endif
	struct saverestore_s
	{
	char *pBaseData; // Start of all entity save data
	char *pCurrentData; // Current buffer pointer for sequential access
	int size; // Current data size
	int bufferSize; // Total space for data
	int tokenSize; // Size of the linear list of tokens
	int tokenCount; // Number of elements in the pTokens table
	char **pTokens; // Hash table of entity strings (sparse)
	int currentIndex; // Holds a global entity table ID
	int tableCount; // Number of elements in the entity table
	int connectionCount;// Number of elements in the levelList[]
	ENTITYTABLE *pTable; // Array of ENTITYTABLE elements (1 for each entity)
	LEVELLIST levelList[ MAX_LEVEL_CONNECTIONS ]; // List of connections from this level
	// smooth transition
	int fUseLandmark;
	char szLandmarkName[20];// landmark we'll spawn near in next level
	vec3_t vecLandmarkOffset;// for landmark transitions
	float time;
	char szCurrentMapName[32]; // To check global entities
	}
	#ifdef _WIN32
	SAVERESTOREDATA
	#endif

Which is passed down into this:

halflife/dlls/cbase.cpp

Lines 301 to 410 in c7240b9

	int DispatchRestore( edict_t *pent, SAVERESTOREDATA *pSaveData, int globalEntity )
	{
	CBaseEntity *pEntity = (CBaseEntity *)GET_PRIVATE(pent);
	if ( pEntity && pSaveData )
	{
	entvars_t tmpVars;
	Vector oldOffset;
	CRestore restoreHelper( pSaveData );
	if ( globalEntity )
	{
	CRestore tmpRestore( pSaveData );
	tmpRestore.PrecacheMode( 0 );
	tmpRestore.ReadEntVars( "ENTVARS", &tmpVars );
	// HACKHACK - reset the save pointers, we're going to restore for real this time
	pSaveData->size = pSaveData->pTable[pSaveData->currentIndex].location;
	pSaveData->pCurrentData = pSaveData->pBaseData + pSaveData->size;
	// -------------------
	const globalentity_t *pGlobal = gGlobalState.EntityFromTable( tmpVars.globalname );
	// Don't overlay any instance of the global that isn't the latest
	// pSaveData->szCurrentMapName is the level this entity is coming from
	// pGlobla->levelName is the last level the global entity was active in.
	// If they aren't the same, then this global update is out of date.
	if ( !FStrEq( pSaveData->szCurrentMapName, pGlobal->levelName ) )
	return 0;
	// Compute the new global offset
	oldOffset = pSaveData->vecLandmarkOffset;
	CBaseEntity *pNewEntity = FindGlobalEntity( tmpVars.classname, tmpVars.globalname );
	if ( pNewEntity )
	{
	// ALERT( at_console, "Overlay %s with %s\n", STRING(pNewEntity->pev->classname), STRING(tmpVars.classname) );
	// Tell the restore code we're overlaying a global entity from another level
	restoreHelper.SetGlobalMode( 1 ); // Don't overwrite global fields
	pSaveData->vecLandmarkOffset = (pSaveData->vecLandmarkOffset - pNewEntity->pev->mins) + tmpVars.mins;
	pEntity = pNewEntity;// we're going to restore this data OVER the old entity
	pent = ENT( pEntity->pev );
	// Update the global table to say that the global definition of this entity should come from this level
	gGlobalState.EntityUpdate( pEntity->pev->globalname, gpGlobals->mapname );
	}
	else
	{
	// This entity will be freed automatically by the engine. If we don't do a restore on a matching entity (below)
	// or call EntityUpdate() to move it to this level, we haven't changed global state at all.
	return 0;
	}
	}
	if ( pEntity->ObjectCaps() & FCAP_MUST_SPAWN )
	{
	pEntity->Restore( restoreHelper );
	pEntity->Spawn();
	}
	else
	{
	pEntity->Restore( restoreHelper );
	pEntity->Precache( );
	}
	// Again, could be deleted, get the pointer again.
	pEntity = (CBaseEntity *)GET_PRIVATE(pent);
	#if 0
	if ( pEntity && pEntity->pev->globalname && globalEntity )
	{
	ALERT( at_console, "Global %s is %s\n", STRING(pEntity->pev->globalname), STRING(pEntity->pev->model) );
	}
	#endif
	// Is this an overriding global entity (coming over the transition), or one restoring in a level
	if ( globalEntity )
	{
	// ALERT( at_console, "After: %f %f %f %s\n", pEntity->pev->origin.x, pEntity->pev->origin.y, pEntity->pev->origin.z, STRING(pEntity->pev->model) );
	pSaveData->vecLandmarkOffset = oldOffset;
	if ( pEntity )
	{
	UTIL_SetOrigin( pEntity->pev, pEntity->pev->origin );
	pEntity->OverrideReset();
	}
	}
	else if ( pEntity && pEntity->pev->globalname )
	{
	const globalentity_t *pGlobal = gGlobalState.EntityFromTable( pEntity->pev->globalname );
	if ( pGlobal )
	{
	// Already dead? delete
	if ( pGlobal->state == GLOBAL_DEAD )
	return -1;
	else if ( !FStrEq( STRING(gpGlobals->mapname), pGlobal->levelName ) )
	{
	pEntity->MakeDormant(); // Hasn't been moved to this level yet, wait but stay alive
	}
	// In this level & not dead, continue on as normal
	}
	else
	{
	ALERT( at_error, "Global Entity %s (%s) not in table!!!\n", STRING(pEntity->pev->globalname), STRING(pEntity->pev->classname) );
	// Spawned entities default to 'On'
	gGlobalState.EntityAdd( pEntity->pev->globalname, gpGlobals->mapname, GLOBAL_ON );
	}
	}
	}
	return 0;
	}

Setting gpGlobals->time here like this:

int DispatchRestore( edict_t *pent, SAVERESTOREDATA *pSaveData, int globalEntity )
{
	gpGlobals->time = pSaveData->time;

	CBaseEntity *pEntity = (CBaseEntity *)GET_PRIVATE(pent);

Works, as tested in Blue Shift. Though that particular script is set up in such a way that the player is unfrozen anyway if they load a save game due to a trigger_auto that isn't set to be removed on fire. Seems that map has scripting that's sensitive to saving and loading.

For the sake of completeness this should also be done in DispatchSave:

void DispatchSave( edict_t *pent, SAVERESTOREDATA *pSaveData )
{
	gpGlobals->time = pSaveData->time;

	CBaseEntity *pEntity = (CBaseEntity *)GET_PRIVATE(pent);

To ensure any save code that needs the current time works as it should.