long-war-2 / lwotc

Port of Long War 2 to XCOM 2's War of the Chosen expansion
341 stars 89 forks source link

IsModInstalled checks in hot code #1604

Closed Iridar closed 1 year ago

Iridar commented 1 year ago

LWOTC has a few IsModInstalled() checks, including in hot code, like GetToHitAsTargetModifiers() in X2Effect_SmokeFlankingCritProtection.

This is suboptimal to the point where it can cause stuttering in tactical, for example when switching between active units.

Relevant discord report.

The suggested solution is to do IsModInstalled() checks in OPTC or somewhere else similar, and cache the result of the check in a class variable, for example:

class X2Effect_SmokeFlankingCritProtection extends X2Effect_Persistent config(Anything);

var config bSmokeStopsFlanksActive;
var config bImprovedSmokeDefenseActive;

function GetToHitAsTargetModifiers(XComGameState_Effect EffectState, XComGameState_Unit Attacker, XComGameState_Unit Target, XComGameState_Ability AbilityState, class<X2AbilityToHitCalc> ToHitType, bool bMelee, bool bFlanking, bool bIndirectFire, out array<ShotModifierInfo> ShotModifiers)
{
    local ShotModifierInfo ShotMod;

    // If the Reliable Smoke or Improved Smoke Defense mods are enabled,
    // let them handle flanking crit protection, otherwise we'll end up
    // doubling the effect.
    if (default.bSmokeStopsFlanksActive || default.bImprovedSmokeDefenseActive)
        return;

    // If the target is affected by smoke and is flanked, then add a
    // negative crit chance to negate the flanking bonus.
    if (bFlanking && Target.IsInWorldEffectTile(class'X2Effect_ApplySmokeGrenadeToWorld'.default.Class.Name))
    {
        ShotMod.ModType = eHit_Crit;
        ShotMod.Reason = FriendlyName;
        ShotMod.Value = -Attacker.GetCurrentStat(eStat_FlankingCritChance);

        ShotModifiers.AddItem(ShotMod);
    }
}

And in OPTC:

static event OnPostTemplatesCreated()
{
    class'X2Effect_SmokeFlankingCritProtection'.default.bSmokeStopsFlanksActive = class'Helpers_LW'.static.IsModInstalled("SmokeStopsFlanks");
    class'X2Effect_SmokeFlankingCritProtection'.default.bImprovedSmokeDefenseActive = class'Helpers_LW'.static.IsModInstalled("ImprovedSmokeDefense")
}

The check can also be accelerated by adding final to IsModInstalled() function definition.

Ideally this kind of caching should be done to all instances of IsModInstalled().

The slowest part of the check seems to be class'Helpers'.static.GetInstalledModNames(). So the alternative universal solution is to cache its output into a config array<name>, and using the cached array for any check that follows the first.

remcoros commented 1 year ago

Just found this propagates to other mods as well, resulting in noticeable frame freezes.

This trace shows the use of 'TACTICALRULES.UnitHasActionsAvailable' from a mod (Select Soldier Icons Redux): The three spikes in frame time all involve calls to 'IsModInstalled'

image

remcoros commented 1 year ago

Another fix without the need for every caller of IsModInstalled to remember to cache it, is to lazy cache any requested mod names:

this will fix all usages of IsModInstalled, also for 3rd party mods using it.

Iridar commented 1 year ago

this will fix all usages of IsModInstalled, also for 3rd party mods using it.

IsModInstalled is not a globally implemented function. Meaning each mod that wants to do this check includes a copy of this function. It's not possible for LWOTC to change how it works in any other mod.

remcoros commented 1 year ago

IsModInstalled is not a globally implemented function.

I see, that's my lack of Unreal(Script) knowledge :)

I see this snippet in some mods:

static function bool IsModInstalled(name X2DCIName)
{
    local X2DownloadableContentInfo Mod;

    foreach `ONLINEEVENTMGR.m_cachedDLCInfos (Mod)
    {
        if (Mod.Class.Name == X2DCIName)
        {
            return true;
        }
    }
    return false;
}

Seems that CHL already has a cached list of dlc infos. might not even be worth caching at all, and just use this, since checking a list of let's say 1000 mods should not be noticeable (but again, do not know enough about UnrealScript to back that up).

Iridar commented 1 year ago

This method cannot be used universally, unfortunately, because it checks specifically for X2DownloadableContentInfo script files, and any mod can include any number of these, potentially none, and their name is not set in stone the same way the mod name is.