Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UITacticalHUD_Enemies UpdateVisibleEnemies should use cached hit chances when sorting final targets array #1233

Open
remcoros opened this issue Aug 5, 2023 · 3 comments · May be fixed by #1234

Comments

@remcoros
Copy link
Contributor

remcoros commented Aug 5, 2023

UpdateVisibleEnemies of UITacticalHUD_Enemies sorts the array of visible enemies by hit chance using a comparer delegate.

This sorting delegate determines the hit change, by calling GetHitChanceForObjectRef on object A and B, which is relatively slow, and calls it multiple times for the same objects, until the array is fully sorted.

The hit chance for the same object reference cannot change during one sorting pass, so pre-calculating the hit chances for visible enemies first. Then using that in the sorting delegate would decrease the number of calls drastically.

Before:
image

After:
image

Note that I used the override of UITacticalHUD_Enemies from 'WOTC_DisplayHitChange' to do my test, which was easier for now than to build a custom CHL.

The profiling was done on the exact same save, same conditions, same move. Reduced the number of calls in that one frame, from 378 to 48.

struct StateObjectReferenceWrapper
{
	var StateObjectReference Object;
	var int HitChance;
};

simulated function UpdateVisibleEnemies(int HistoryIndex)
{
	local array<StateObjectReferenceWrapper> CachedHitChanges;
	local StateObjectReferenceWrapper HitChangeWrapper;

	// ... snip

	if (kActiveUnit != none)
	{
		// ... snip

		iNumVisibleEnemies = m_arrTargets.Length;

		CachedHitChanges.Length = 0;
		for (i = 0; i < m_arrTargets.Length; ++i)
		{
			HitChangeWrapper.Object = m_arrTargets[i];
			HitChangeWrapper.HitChance = GetHitChanceForObjectRef(HitChangeWrapper.Object);
			CachedHitChanges.AddItem(HitChangeWrapper);
		}

		CachedHitChanges.Sort(SortEnemies2);

		m_arrTargets.Length = 0;
		for (i = 0; i < CachedHitChanges.Length; ++i)
		{
			m_arrTargets.AddItem(CachedHitChanges[i].Object);
		}

		UpdateVisuals(HistoryIndex);
	}
}

private function int SortEnemies2(StateObjectReferenceWrapper ObjectA, StateObjectReferenceWrapper ObjectB)
{
    // you get the point :)
}	

Not sure if the above implementation is ideal, that's the best I could do with my limited UnrealScript knowledge.
Maybe it's also possible with a class variable, then using CachedHitChanges.Find('Object', ObjectA) (which is a hashtable lookup?) inside SortEnemies.

discord: https://discord.com/channels/165245941664710656/165245941664710656/1137333029837209630

@remcoros remcoros changed the title UITacticalHUD_Enemies UpdateVisibleEnemies should use cached hit changes when sorting final targets array UITacticalHUD_Enemies UpdateVisibleEnemies should use cached hit chances when sorting final targets array Aug 5, 2023
@robojumper
Copy link
Member

If I may suggest a modification -- in addition to the GetHitChanceForObjectRef slowness, SortEnemies currently constantly fetches state objects from the History in every sorting call to push destructibles to the end, so the function that maps the StateObjectReference to structs could also pre-check whether the state object is a XComGameState_Destructible and also store that in the struct (or just assign a -1000 hit chance to them) for sorting purposes. That would simplify the comparison delegate.

remcoros added a commit to remcoros/X2WOTCCommunityHighlander that referenced this issue Aug 5, 2023
remcoros added a commit to remcoros/X2WOTCCommunityHighlander that referenced this issue Aug 5, 2023
remcoros added a commit to remcoros/X2WOTCCommunityHighlander that referenced this issue Aug 5, 2023
@remcoros
Copy link
Contributor Author

As a reminder for myself: The same optimization can be applied to "GatherAbilityTargets" of XComGameState_Ability and XComGameState_Ability_CH.
The sorting delegate there also does calls to GetGameState and GetShotBreakDown

@robojumper
Copy link
Member

Should be noted that the default implementation of GatherAbilityTargets is native and only in some edge cases for very specific abilities, the implementation in XComGameState_Ability_CH is used. See the comments in that file for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants