diff --git a/d3xp/script/Script_Compiler.cpp b/d3xp/script/Script_Compiler.cpp index 6fac0a6..8e6e308 100644 --- a/d3xp/script/Script_Compiler.cpp +++ b/d3xp/script/Script_Compiler.cpp @@ -987,6 +987,25 @@ idVarDef *idCompiler::EmitFunctionParms( int op, idVarDef *func, int startarg, i // need arg size seperate since script object may be NULL statement_t &statement = gameLocal.program.GetStatement( gameLocal.program.NumStatements() - 1 ); statement.c = SizeConstant( func->value.functionPtr->parmTotal ); + // DG: before changes I did to ParseFunctionDef(), func->value.functionPtr->parmTotal was 0 + // if the function declaration/prototype has been parsed already, but the + // definition/implementation hadn't been parsed yet. That was wrong and sometimes + // (with debug game DLLs) lead to assertions in custom scripts, because the + // stack space reserved for function parameters was wrong. + // Now func->value.functionPtr->parmTotal is calculated when parsing the prototype, + // but func->value.functionPtr->parmSize[i] is still only calculated when parsing + // the implementation (as it's not needed before and so we can tell the cases apart here). + // However, savegames from before the change have script checksums + // (by idProgram::CalculateChecksum()) from statements with the wrong size, so + // loading them would fail as the checksum doesn't match. + // Setting this flag allows using the parmTotal argSize 0 when calculating the checksum + // so it matches the one from old savegames (unless something else has also changed in + // the script state so they really are incompatible). That's only done when actually + // loading old savegames (detected via BUILD_NUMBER/savegame.GetBuildNumber()) + if ( op == OP_OBJECTCALL && func->value.functionPtr->parmTotal > 0 + && func->value.functionPtr->parmSize.Num() == 0 ) { + statement.flags = statement_t::FLAG_OBJECTCALL_IMPL_NOT_PARSED_YET; + } } else { EmitOpcode( op, func, SizeConstant( size ) ); } @@ -2144,46 +2163,57 @@ void idCompiler::ParseFunctionDef( idTypeDef *returnType, const char *name ) { } } - // DG: make sure parmSize gets calculated when parsing prototype (not just when parsing - // implementation) so calling this function/method before implementation has been parsed + // DG: make sure parmTotal gets calculated when parsing prototype (not just when parsing + // implementation) so calling this function/method before the implementation has been parsed // works without getting Assertions in IdInterpreter::Execute() and ::LeaveFunction() - // ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase", see #303) + // ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase", see #303 and #344) // calculate stack space used by parms numParms = type->NumParameters(); - if( numParms != func->parmSize.Num() ) { // DG: make sure not to do this twice - - // if it hasn't been parsed yet, parmSize.Num() should be 0.. - assert( func->parmSize.Num() == 0 && "function had different number of arguments before?!" ); - - func->parmSize.SetNum( numParms ); - for( i = 0; i < numParms; i++ ) { - parmType = type->GetParmType( i ); - if ( parmType->Inherits( &type_object ) ) { - func->parmSize[ i ] = type_object.Size(); - } else { - func->parmSize[ i ] = parmType->Size(); - } - func->parmTotal += func->parmSize[ i ]; - } - - // define the parms - for( i = 0; i < numParms; i++ ) { - if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) { - Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) ); - } - gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false ); - } - } - - // DG: moved this down here so parmSize also gets calculated when parsing prototype - // check if this is a prototype or declaration if ( !CheckToken( "{" ) ) { // it's just a prototype, so get the ; and move on ExpectToken( ";" ); + // DG: BUT only after calculating the stack space for the arguments because this + // function might be called before the implementation is parsed (see #303 and #344) + // which otherwise causes Assertions in IdInterpreter::Execute() and ::LeaveFunction() + // ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase") + func->parmTotal = 0; + for( i = 0; i < numParms; i++ ) { + parmType = type->GetParmType( i ); + int size = parmType->Inherits( &type_object ) ? type_object.Size() : parmType->Size(); + func->parmTotal += size; + // NOTE: Don't set func->parmSize[] yet, the workaround to keep compatibility + // with old savegames checks for func->parmSize.Num() == 0 + // (see EmitFunctionParms() for more explanation of that workaround) + // Also not defining the parms yet, otherwise they're defined in a different order + // than before, so their .num is different which breaks compat with old savegames + } return; } - // DG end + + + int totalSize = 0; // DG: totalsize might already have been calculated for the prototype, see a few lines above + func->parmSize.SetNum( numParms ); + for( i = 0; i < numParms; i++ ) { + parmType = type->GetParmType( i ); + if ( parmType->Inherits( &type_object ) ) { + func->parmSize[ i ] = type_object.Size(); + } else { + func->parmSize[ i ] = parmType->Size(); + } + totalSize += func->parmSize[ i ]; + } + // DG: if parmTotal has been calculated before, it shouldn't have changed + assert((func->parmTotal == 0 || totalSize == func->parmTotal) && "function parameter sizes differ between protype vs implementation?!"); + func->parmTotal = totalSize; + + // define the parms + for( i = 0; i < numParms; i++ ) { + if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) { + Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) ); + } + gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false ); + } oldscope = scope; scope = def; diff --git a/d3xp/script/Script_Program.cpp b/d3xp/script/Script_Program.cpp index 716c23d..adbbb19 100644 --- a/d3xp/script/Script_Program.cpp +++ b/d3xp/script/Script_Program.cpp @@ -1634,7 +1634,9 @@ statement_t *idProgram::AllocStatement( void ) { if ( statements.Num() >= statements.Max() ) { throw idCompileError( va( "Exceeded maximum allowed number of statements (%d)", statements.Max() ) ); } - return statements.Alloc(); + statement_t* ret = statements.Alloc(); + ret->flags = 0; // DG: initialize the added flags (that are rarely set/used otherwise) to 0 + return ret; } /* @@ -2005,7 +2007,7 @@ void idProgram::Save( idSaveGame *savefile ) const { savefile->WriteByte( variables[i] ); } - int checksum = CalculateChecksum(); + int checksum = CalculateChecksum(false); savefile->WriteInt( checksum ); } @@ -2039,9 +2041,11 @@ bool idProgram::Restore( idRestoreGame *savefile ) { int saved_checksum, checksum; savefile->ReadInt( saved_checksum ); - checksum = CalculateChecksum(); + bool isOldSavegame = savefile->GetBuildNumber() <= 1304; + checksum = CalculateChecksum(isOldSavegame); if ( saved_checksum != checksum ) { + gameLocal.Warning( "WARNING: Real Script checksum didn't match the one from the savegame!"); result = false; } @@ -2053,7 +2057,7 @@ bool idProgram::Restore( idRestoreGame *savefile ) { idProgram::CalculateChecksum ================ */ -int idProgram::CalculateChecksum( void ) const { +int idProgram::CalculateChecksum( bool forOldSavegame ) const { int i, result; typedef struct { @@ -2069,6 +2073,17 @@ int idProgram::CalculateChecksum( void ) const { memset( statementList, 0, ( sizeof(statementBlock_t) * statements.Num() ) ); + // DG hack: get the vardef for the argSize == 0 constant for savegame-compat + int constantZeroNum = -1; + if ( forOldSavegame ) { + for( idVarDef* def = GetDefList( "" ); def != NULL; def = def->Next() ) { + if ( def->Type() == ev_argsize && def->value.argSize == 0 ) { + constantZeroNum = def->num; + break; + } + } + } + // Copy info into new list, using the variable numbers instead of a pointer to the variable for( i = 0; i < statements.Num(); i++ ) { statementList[i].op = statements[i].op; @@ -2084,7 +2099,15 @@ int idProgram::CalculateChecksum( void ) const { statementList[i].b = -1; } if ( statements[i].c ) { - statementList[i].c = statements[i].c->num; + // DG: old savegames wrongly assumed argSize 0 for some statements. + // So for the checksums to match we need to use the corresponding vardef num here + // See idCompiler::EmitFunctionParms() and ParseFunctionDef() for more details. + if ( forOldSavegame && statements[i].op == OP_OBJECTCALL + && statements[i].flags == statement_t::FLAG_OBJECTCALL_IMPL_NOT_PARSED_YET ) { + statementList[i].c = constantZeroNum; + } else { + statementList[i].c = statements[i].c->num; + } } else { statementList[i].c = -1; } diff --git a/d3xp/script/Script_Program.h b/d3xp/script/Script_Program.h index ca0d7a8..93a7eb8 100644 --- a/d3xp/script/Script_Program.h +++ b/d3xp/script/Script_Program.h @@ -331,7 +331,7 @@ class idVarDef { friend class idVarDefName; public: - int num; + int num; // global index/ID of variable varEval_t value; idVarDef * scope; // function, namespace, or object the var was defined in int numUsers; // number of users if this is a constant @@ -432,11 +432,19 @@ extern idVarDef def_boolean; typedef struct statement_s { unsigned short op; + unsigned short flags; // DG: added this for ugly hacks + enum { + // op is OP_OBJECTCALL and when the statement was created the function/method + // implementation hasn't been parsed yet (only the declaration/prototype) + // see idCompiler::EmitFunctionParms() and idProgram::CalculateChecksum() + FLAG_OBJECTCALL_IMPL_NOT_PARSED_YET = 1, + }; + // DG: moved linenumber and file up here to prevent wasting 8 bytes of padding on 64bit + unsigned short linenumber; + unsigned short file; idVarDef *a; idVarDef *b; idVarDef *c; - unsigned short linenumber; - unsigned short file; } statement_t; /*********************************************************************** @@ -488,7 +496,7 @@ public: // save games void Save( idSaveGame *savefile ) const; bool Restore( idRestoreGame *savefile ); - int CalculateChecksum( void ) const; // Used to insure program code has not + int CalculateChecksum( bool forOldSavegame ) const; // Used to insure program code has not // changed between savegames void Startup( const char *defaultScript ); diff --git a/framework/BuildVersion.h b/framework/BuildVersion.h index 8065815..0d4681e 100644 --- a/framework/BuildVersion.h +++ b/framework/BuildVersion.h @@ -25,4 +25,4 @@ If you have questions concerning this license or the applicable additional terms =========================================================================== */ -const int BUILD_NUMBER = 1304; +const int BUILD_NUMBER = 1305; diff --git a/game/Game_local.cpp b/game/Game_local.cpp index 524bc5d..bc3d642 100644 --- a/game/Game_local.cpp +++ b/game/Game_local.cpp @@ -427,6 +427,10 @@ void idGameLocal::SaveGame( idFile *f ) { savegame.WriteBuildNumber( BUILD_NUMBER ); + // DG: TODO: write other things (maybe internal savegame version so I don't have to bump BUILD_NUMBER AGAIN) + // and OS, architecture etc, see #344 + // TODO: adjust InitFromSavegame() accordingly! + // go through all entities and threads and add them to the object list for( i = 0; i < MAX_GENTITIES; i++ ) { ent = entities[i]; @@ -1220,6 +1224,12 @@ bool idGameLocal::InitFromSaveGame( const char *mapName, idRenderWorld *renderWo savegame.ReadBuildNumber(); + + if(savegame.GetBuildNumber() >= 1305) + { + // TODO: read stuff additionally written in SaveGame() + } + // Create the list of all objects in the game savegame.CreateObjects(); diff --git a/game/script/Script_Compiler.cpp b/game/script/Script_Compiler.cpp index 1ad2693..196d463 100644 --- a/game/script/Script_Compiler.cpp +++ b/game/script/Script_Compiler.cpp @@ -987,6 +987,25 @@ idVarDef *idCompiler::EmitFunctionParms( int op, idVarDef *func, int startarg, i // need arg size seperate since script object may be NULL statement_t &statement = gameLocal.program.GetStatement( gameLocal.program.NumStatements() - 1 ); statement.c = SizeConstant( func->value.functionPtr->parmTotal ); + // DG: before changes I did to ParseFunctionDef(), func->value.functionPtr->parmTotal was 0 + // if the function declaration/prototype has been parsed already, but the + // definition/implementation hadn't been parsed yet. That was wrong and sometimes + // (with debug game DLLs) lead to assertions in custom scripts, because the + // stack space reserved for function parameters was wrong. + // Now func->value.functionPtr->parmTotal is calculated when parsing the prototype, + // but func->value.functionPtr->parmSize[i] is still only calculated when parsing + // the implementation (as it's not needed before and so we can tell the cases apart here). + // However, savegames from before the change have script checksums + // (by idProgram::CalculateChecksum()) from statements with the wrong size, so + // loading them would fail as the checksum doesn't match. + // Setting this flag allows using the parmTotal argSize 0 when calculating the checksum + // so it matches the one from old savegames (unless something else has also changed in + // the script state so they really are incompatible). That's only done when actually + // loading old savegames (detected via BUILD_NUMBER/savegame.GetBuildNumber()) + if ( op == OP_OBJECTCALL && func->value.functionPtr->parmTotal > 0 + && func->value.functionPtr->parmSize.Num() == 0 ) { + statement.flags = statement_t::FLAG_OBJECTCALL_IMPL_NOT_PARSED_YET; + } } else { EmitOpcode( op, func, SizeConstant( size ) ); } @@ -2144,46 +2163,57 @@ void idCompiler::ParseFunctionDef( idTypeDef *returnType, const char *name ) { } } - // DG: make sure parmSize gets calculated when parsing prototype (not just when parsing - // implementation) so calling this function/method before implementation has been parsed + // DG: make sure parmTotal gets calculated when parsing prototype (not just when parsing + // implementation) so calling this function/method before the implementation has been parsed // works without getting Assertions in IdInterpreter::Execute() and ::LeaveFunction() - // ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase", see #303) + // ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase", see #303 and #344) // calculate stack space used by parms numParms = type->NumParameters(); - if( numParms != func->parmSize.Num() ) { // DG: make sure not to do this twice - - // if it hasn't been parsed yet, parmSize.Num() should be 0.. - assert( func->parmSize.Num() == 0 && "function had different number of arguments before?!" ); - - func->parmSize.SetNum( numParms ); - for( i = 0; i < numParms; i++ ) { - parmType = type->GetParmType( i ); - if ( parmType->Inherits( &type_object ) ) { - func->parmSize[ i ] = type_object.Size(); - } else { - func->parmSize[ i ] = parmType->Size(); - } - func->parmTotal += func->parmSize[ i ]; - } - - // define the parms - for( i = 0; i < numParms; i++ ) { - if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) { - Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) ); - } - gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false ); - } - } - - // DG: moved this down here so parmSize also gets calculated when parsing prototype - // check if this is a prototype or declaration if ( !CheckToken( "{" ) ) { // it's just a prototype, so get the ; and move on ExpectToken( ";" ); + // DG: BUT only after calculating the stack space for the arguments because this + // function might be called before the implementation is parsed (see #303 and #344) + // which otherwise causes Assertions in IdInterpreter::Execute() and ::LeaveFunction() + // ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase") + func->parmTotal = 0; + for( i = 0; i < numParms; i++ ) { + parmType = type->GetParmType( i ); + int size = parmType->Inherits( &type_object ) ? type_object.Size() : parmType->Size(); + func->parmTotal += size; + // NOTE: Don't set func->parmSize[] yet, the workaround to keep compatibility + // with old savegames checks for func->parmSize.Num() == 0 + // (see EmitFunctionParms() for more explanation of that workaround) + // Also not defining the parms yet, otherwise they're defined in a different order + // than before, so their .num is different which breaks compat with old savegames + } return; } - // DG end + + + int totalSize = 0; // DG: totalsize might already have been calculated for the prototype, see a few lines above + func->parmSize.SetNum( numParms ); + for( i = 0; i < numParms; i++ ) { + parmType = type->GetParmType( i ); + if ( parmType->Inherits( &type_object ) ) { + func->parmSize[ i ] = type_object.Size(); + } else { + func->parmSize[ i ] = parmType->Size(); + } + totalSize += func->parmSize[ i ]; + } + // DG: if parmTotal has been calculated before, it shouldn't have changed + assert((func->parmTotal == 0 || totalSize == func->parmTotal) && "function parameter sizes differ between protype vs implementation?!"); + func->parmTotal = totalSize; + + // define the parms + for( i = 0; i < numParms; i++ ) { + if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) { + Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) ); + } + gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false ); + } oldscope = scope; scope = def; diff --git a/game/script/Script_Program.cpp b/game/script/Script_Program.cpp index 716c23d..adbbb19 100644 --- a/game/script/Script_Program.cpp +++ b/game/script/Script_Program.cpp @@ -1634,7 +1634,9 @@ statement_t *idProgram::AllocStatement( void ) { if ( statements.Num() >= statements.Max() ) { throw idCompileError( va( "Exceeded maximum allowed number of statements (%d)", statements.Max() ) ); } - return statements.Alloc(); + statement_t* ret = statements.Alloc(); + ret->flags = 0; // DG: initialize the added flags (that are rarely set/used otherwise) to 0 + return ret; } /* @@ -2005,7 +2007,7 @@ void idProgram::Save( idSaveGame *savefile ) const { savefile->WriteByte( variables[i] ); } - int checksum = CalculateChecksum(); + int checksum = CalculateChecksum(false); savefile->WriteInt( checksum ); } @@ -2039,9 +2041,11 @@ bool idProgram::Restore( idRestoreGame *savefile ) { int saved_checksum, checksum; savefile->ReadInt( saved_checksum ); - checksum = CalculateChecksum(); + bool isOldSavegame = savefile->GetBuildNumber() <= 1304; + checksum = CalculateChecksum(isOldSavegame); if ( saved_checksum != checksum ) { + gameLocal.Warning( "WARNING: Real Script checksum didn't match the one from the savegame!"); result = false; } @@ -2053,7 +2057,7 @@ bool idProgram::Restore( idRestoreGame *savefile ) { idProgram::CalculateChecksum ================ */ -int idProgram::CalculateChecksum( void ) const { +int idProgram::CalculateChecksum( bool forOldSavegame ) const { int i, result; typedef struct { @@ -2069,6 +2073,17 @@ int idProgram::CalculateChecksum( void ) const { memset( statementList, 0, ( sizeof(statementBlock_t) * statements.Num() ) ); + // DG hack: get the vardef for the argSize == 0 constant for savegame-compat + int constantZeroNum = -1; + if ( forOldSavegame ) { + for( idVarDef* def = GetDefList( "" ); def != NULL; def = def->Next() ) { + if ( def->Type() == ev_argsize && def->value.argSize == 0 ) { + constantZeroNum = def->num; + break; + } + } + } + // Copy info into new list, using the variable numbers instead of a pointer to the variable for( i = 0; i < statements.Num(); i++ ) { statementList[i].op = statements[i].op; @@ -2084,7 +2099,15 @@ int idProgram::CalculateChecksum( void ) const { statementList[i].b = -1; } if ( statements[i].c ) { - statementList[i].c = statements[i].c->num; + // DG: old savegames wrongly assumed argSize 0 for some statements. + // So for the checksums to match we need to use the corresponding vardef num here + // See idCompiler::EmitFunctionParms() and ParseFunctionDef() for more details. + if ( forOldSavegame && statements[i].op == OP_OBJECTCALL + && statements[i].flags == statement_t::FLAG_OBJECTCALL_IMPL_NOT_PARSED_YET ) { + statementList[i].c = constantZeroNum; + } else { + statementList[i].c = statements[i].c->num; + } } else { statementList[i].c = -1; } diff --git a/game/script/Script_Program.h b/game/script/Script_Program.h index f8df2e9..29ee8a0 100644 --- a/game/script/Script_Program.h +++ b/game/script/Script_Program.h @@ -317,7 +317,7 @@ class idVarDef { friend class idVarDefName; public: - int num; + int num; // global index/ID of variable varEval_t value; idVarDef * scope; // function, namespace, or object the var was defined in int numUsers; // number of users if this is a constant @@ -418,11 +418,19 @@ extern idVarDef def_boolean; typedef struct statement_s { unsigned short op; + unsigned short flags; // DG: added this for ugly hacks + enum { + // op is OP_OBJECTCALL and when the statement was created the function/method + // implementation hasn't been parsed yet (only the declaration/prototype) + // see idCompiler::EmitFunctionParms() and idProgram::CalculateChecksum() + FLAG_OBJECTCALL_IMPL_NOT_PARSED_YET = 1, + }; + // DG: moved linenumber and file up here to prevent wasting 8 bytes of padding on 64bit + unsigned short linenumber; + unsigned short file; idVarDef *a; idVarDef *b; idVarDef *c; - unsigned short linenumber; - unsigned short file; } statement_t; /*********************************************************************** @@ -474,7 +482,7 @@ public: // save games void Save( idSaveGame *savefile ) const; bool Restore( idRestoreGame *savefile ); - int CalculateChecksum( void ) const; // Used to insure program code has not + int CalculateChecksum( bool forOldSavegame ) const; // Used to insure program code has not // changed between savegames void Startup( const char *defaultScript );