X2CommunityCore / X2WOTCCommunityHighlander

https://steamcommunity.com/workshop/filedetails/?id=1134256495
MIT License
60 stars 68 forks source link

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

Open remcoros opened 1 year ago

remcoros commented 1 year ago

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

robojumper commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.