ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.65k stars 617 forks source link

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

Open SamVanheer opened 3 years ago

SamVanheer commented 3 years ago

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 commented 3 years ago

Looks like this can be fixed in game code. The saved time value is accessible in this type: https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/engine/eiface.h#L342-L370

Which is passed down into this: https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/cbase.cpp#L301-L410

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.