UE1 - UT Tips on optimization

[Zur]

Hi,

I just finished testing a mutator and would like to know how the following code could be optimized.

It's the tick event and it runs for each player so the code inside needs to be as concise as possible.

Perhaps other Actors could be used to take advantage of UT's threading.

The rest of the code can be seen in the zip file attached below.

P.S: I'm guessing the "simulated" is probably not needed (I'm still having trouble understanding replication).

simulated function Tick( float DeltaTime )
{
	local int playerID, lastYaw, lastPitch, deltaYaw, deltaPitch, maxDeltaYaw, maxDeltaPitch;
	local Pawn p;

	// Check if game has started
	if( getTimeSinceGameStart() == 0 )
		return;

	p = Level.PawnList; // get start of linked list of Pawns

	while( p != none )
	{
		if( isValidPlayer( p ) )
		{
			playerID = p.PlayerReplicationInfo.PlayerID;

			// Get data from arrays
			lastYaw = lastYawArray[ playerID ];
			lastPitch = lastPitchArray[ playerID ];

			if( p.bJustTeleported ) // Pitch is reset when entering a teleporter
				lastPitch = p.ViewRotation.Pitch;

			// When a player dies, the Pawn is hidden and tweened while a carcass is spawned in it's place
			if( p.GetStateName() == 'Dying' || p.bHidden )
			{
				lastYaw = p.ViewRotation.Yaw;
				lastPitch = p.ViewRotation.Pitch;
			}

			// Refer to ModifyPlayer() for an explanation of why this test is carried out
			if( pawnArray[ playerID ] == none || pawnArray[ playerID ] != p )
				lastYaw = p.ViewRotation.Yaw;

			// Use absolute function as Yaw values can be negative
			deltaYaw = abs( abs( p.ViewRotation.Yaw ) -  abs( lastYaw ) );

			// Calculate Pitch difference ( max up 18000<-0/65536->49152 max down )
			if( p.ViewRotation.Pitch > 0 && p.ViewRotation.Pitch <= 18000 )
			{
					if( lastPitch > 0 && lastPitch <= 18000 )
						deltaPitch = abs( p.ViewRotation.Pitch - lastPitch );
					else if( lastPitch >= 49152 && lastPitch <= 65536 )
						deltaPitch = p.ViewRotation.Pitch + ( 65536 - lastPitch );

				else if( p.ViewRotation.Pitch >= 49152 && p.ViewRotation.Pitch <= 65536 )
				{
					if( lastPitch > 0 && lastPitch <= 18000 )
						deltaPitch = ( 65536 - p.ViewRotation.Pitch ) + lastPitch;
					else if( lastPitch >= 49152 && lastPitch <= 65536 )
						deltaPitch = abs( p.ViewRotation.Pitch - lastPitch );
				}

				if( deltaYaw > maxDeltaYaw )
					maxDeltaYaw = deltaYaw;

				if( deltaPitch > maxDeltaPitch )
					maxDeltaPitch = deltaPitch;

				if( maxDeltaYaw > maxYawLimit || maxDeltaPitch > maxPitchLimit )
				{
					maxDeltaYawArray[ playerID ] = maxDeltaYaw;
					maxDeltaPitchArray[ playerID ] = maxDeltaPitch;

					SaveConfig();

					handlePlayer( p );
				}
			}

			lastYaw = p.ViewRotation.Yaw;
			lastPitch = p.ViewRotation.Pitch;

			// Save data to arrays
			lastYawArray[ playerID ] = lastYaw;
			lastPitchArray[ playerID ] = lastPitch;
			deltaYawArray[ playerID ] = deltaYaw;
			deltaPitchArray[ playerID ] = deltaPitch;
		}

		p = p.nextPawn;
	}
}

 

[Wormbo]

The construct "p = ...; while (p) { ...; p = next; }" should really be written as a for loop. The bytecode is absolutely identical and you will know what's going on when you come back to your code after a few weeks.

The "simulated" keyword can be dropped for all actors that are either only spawned on a client or have RemoteRole = ROLE_None when spawned on a server. This doesn't affect performance in any way, though.

General optimization includes doing cheat stuff first if it helps ruling out more expensive stuff. For example, it might be slightly more efficient to do "if (p.bHidden || p.GetStateName() == 'Dying')" than the other way around because p.bHidden is faster than calling p.GetStateName() and comparing its result to the literal 'Dying'. However, if it's more likely that a player might be in state Dying without being hidden, then your order might be the better choice.

A more effective optimization might be to prevent redundant assignments. For example, dead players are unlikely to just have teleported. In fact, you don't have to care whether a dead player just teleported because his pitch was reset anyway. Same goes for that other check for lastYaw. You could probably optimize that part by adding an else to the "if (dead)" and put the "if (teleported)" and "if (array thingy") statements in there. Speaking of the array thingy: p is known to be not none, so if you find a value to be none, you automatically know it's not p, thus you can drop "pawnArray[playerID] == none" without replacement.

SaveConfig() - does that really need to be there? What if that part happens often, or even multiple times in the same tick? Consider starting a Timer instead if it's not already running (something like "if (TimerRate == 0) SetTimer(0.1, false);") and do the SaveConfig() there. Or at least make sure it only happens once per tick by replacing SaveConfig() with bShouldSaveConfig=True and at the end of the Tick body do "if (bShouldSaveConfig) SaveConfig();" You need to bShouldSaveConfig as local bool variable.


"Perhaps other Actors could be used to take advantage of UT's threading." - UnrealScript is strictly single-threaded. In UE1/2 the only multi-threading occurs when you use the Resolve() function of Tcp/UdpLink, where the DNS->IP resolving is done in a separate thread so the game doesn't block.

 

[Zur]

For example, it might be slightly more efficient to do "if (p.bHidden || p.GetStateName() == 'Dying')" than the other way around because p.bHidden is faster than calling p.GetStateName() and comparing its result to the literal 'Dying'.

I'm not sure if bHidden is needed. It is used while a player respawns but it might be redundant with respect to the checks in ModifyPlayer.

A more effective optimization might be to prevent redundant assignments. For example, dead players are unlikely to just have teleported. In fact, you don't have to care whether a dead player just teleported because his pitch was reset anyway.

Ok, that makes sense. Depending on what the player is doing or his state, he can be completely ignored. ViewRotation seems to be reset before Health <= 0 though so the StateName might be the only option.

Same goes for that other check for lastYaw. You could probably optimize that part by adding an else to the "if (dead)" and put the "if (teleported)" and "if (array thingy") statements in there.

The checks done before the difference in yaw (=deltayaw) is calculated simply set lastYaw to the current ViewRotationYaw. So deltaYaw will always be zero. Maybe a bool, say bSkip, can be used to ignore the deltaYaw and deltaPitch calculation. Using continue or getting the next pawn would probably be a bad idea in this case.

Speaking of the array thingy: p is known to be not none, so if you find a value to be none, you automatically know it's not p, thus you can drop "pawnArray[playerID] == none" without replacement.

That's true. Both conditions can be condensed into "pawnArray[playerID] != p".

The conditions test if ModifyPlayer has had a chance to kick in at least once. This is to avoid calculating the first deltaYaw when lastYawArray[ playerID ] equals zero. This is something you've probably figured out.

It looks like tick has a chance to run at least once before ModifyPlayer is called. The following code in GameInfo might also explain what causes a big difference in Yaw :

	// In not found, spawn a new player.
	if( NewPlayer==None )
	{
		// Make sure this kind of player is allowed.
		if ( (bHumansOnly || Level.bHumansOnly) && !SpawnClass.Default.bIsHuman
			&& !ClassIsChildOf(SpawnClass, class'Spectator') )
			SpawnClass = DefaultPlayerClass;

		NewPlayer = Spawn(SpawnClass,,,StartSpot.Location,StartSpot.Rotation);
		if( NewPlayer!=None )
			NewPlayer.ViewRotation = StartSpot.Rotation;
	}

	// Handle spawn failure.
	if( NewPlayer == None )
	{
		log("Couldn't spawn player at "$StartSpot);
		Error = FailedSpawnMessage;
		return None;
	}

	NewPlayer.static.SetMultiSkin(NewPlayer, InSkin, InFace, InTeam);

	// Set the player's ID.
	NewPlayer.PlayerReplicationInfo.PlayerID = CurrentID++;

	// Init player's information.
	NewPlayer.ClientSetRotation(NewPlayer.Rotation);

SaveConfig() - does that really need to be there? What if that part happens often, or even multiple times in the same tick? Consider starting a Timer instead if it's not already running (something like "if (TimerRate == 0) SetTimer(0.1, false);") and do the SaveConfig() there. Or at least make sure it only happens once per tick by replacing SaveConfig() with bShouldSaveConfig=True and at the end of the Tick body do "if (bShouldSaveConfig) SaveConfig();" You need to bShouldSaveConfig as local bool variable.

Not really. SaveConfig() could probably be removed because there's no need to save maxDeltaYawArray to the ini file since it will be meaningless when a new game starts. The difference in Yaw is written to a log anyway when it's exceeded the limits.

However, this idea of letting Tick do it's work and letting the Timer do a less critical task is one to keep in mind.

"Perhaps other Actors could be used to take advantage of UT's threading." - UnrealScript is strictly single-threaded. In UE1/2 the only multi-threading occurs when you use the Resolve() function of Tcp/UdpLink, where the DNS->IP resolving is done in a separate thread so the game doesn't block.

You've probably read the document below a number of times. It's only recently that the following paragraph caught my attention. One guy at unrealadmin seems to think custom PRIs can be used to achieve some sort of parallelism (at least, that's the impression I got). What do you think ?

http://unreal.epicgames.com/UnrealScript.htm

In traditional programming terms, UnrealScript acts as if each actor in a level has its own "thread" of execution. Internally, Unreal does not use Windows threads, because that would be very inefficient (Windows 95 and Windows NT do not handle thousands of simultaneous threads efficiently). Instead, UnrealScript simulates threads. This fact is transparent to UnrealScript code, but becomes very apparent when you write C++ code which interacts with UnrealScript.