Commits that were missing in #67

d3xp/Entity.cpp

@@ -4955,6 +4955,14 @@ idEntity::Event_StartSoundShader
 ================
 */
 void idEntity::Event_StartSoundShader( const char *soundName, int channel ) {
+	// DG: at least some map scripts in d3xp seem to use $ent.startSoundShader( "", SND_CHANNEL_whatever );
+	//     to stop a playing sound. special-casing this to avoid playing beep sound (if s_playDefaultSound 1)
+	if ( soundName == NULL || soundName[0] == '\0' ) {
+		StopSound( (s_channelType)channel, false );
+		idThread::ReturnFloat( 0.0f );
+		return;
+	}
+
 	int length;
 
 	bool netSync = false;

d3xp/script/Script_Compiler.cpp

@@ -988,6 +988,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 ) );
 	}
@@ -2145,46 +2164,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 );
+	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 );
-			if ( parmType->Inherits( &type_object ) ) {
-				func->parmSize[ i ] = type_object.Size();
-			} else {
-				func->parmSize[ i ] = parmType->Size();
-			}
-			func->parmTotal += func->parmSize[ 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;
+	}
 
-		// 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 );
+
+	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;
 
-	// 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( ";" );
-		return;
+	// 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 end
 
 	oldscope = scope;
 	scope = def;

d3xp/script/Script_Program.cpp

@@ -1640,7 +1640,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;
 }
 
 /*
@@ -2011,7 +2013,7 @@ void idProgram::Save( idSaveGame *savefile ) const {
 		savefile->WriteByte( variables[i] );
 	}
 
-	int checksum = CalculateChecksum();
+	int checksum = CalculateChecksum(false);
 	savefile->WriteInt( checksum );
 }
 
@@ -2045,9 +2047,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;
 	}
 
@@ -2059,7 +2063,7 @@ bool idProgram::Restore( idRestoreGame *savefile ) {
 idProgram::CalculateChecksum
 ================
 */
-int idProgram::CalculateChecksum( void ) const {
+int idProgram::CalculateChecksum( bool forOldSavegame ) const {
 	int i, result;
 
 	typedef struct {
@@ -2075,6 +2079,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( "<IMMEDIATE>" ); 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;
@@ -2090,7 +2105,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;
 		}

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 @@ class idProgram {
 	// 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 );

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;

game/Entity.cpp

@@ -4874,6 +4874,14 @@ idEntity::Event_StartSoundShader
 ================
 */
 void idEntity::Event_StartSoundShader( const char *soundName, int channel ) {
+	// DG: at least some map scripts in d3xp seem to use $ent.startSoundShader( "", SND_CHANNEL_whatever );
+	//     to stop a playing sound. special-casing this to avoid playing beep sound (if s_playDefaultSound 1)
+	if ( soundName == NULL || soundName[0] == '\0' ) {
+		StopSound( (s_channelType)channel, false );
+		idThread::ReturnFloat( 0.0f );
+		return;
+	}
+
 	int length;
 
 

game/script/Script_Compiler.cpp

@@ -988,6 +988,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 ) );
 	}
@@ -2145,46 +2164,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 );
+	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 );
-			if ( parmType->Inherits( &type_object ) ) {
-				func->parmSize[ i ] = type_object.Size();
-			} else {
-				func->parmSize[ i ] = parmType->Size();
-			}
-			func->parmTotal += func->parmSize[ 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;
+	}
 
-		// 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 );
+
+	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;
 
-	// 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( ";" );
-		return;
+	// 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 end
 
 	oldscope = scope;
 	scope = def;