Allow mods to control whether to grant the achievement for winning without extra squadsize

[cannonfodder1]

The achievement "The Few and the Proud" requires the player to defeat the aliens on Commander or Legend difficulty without increasing their squad size. The evaluation logic in X2AchievementTracker checks specifically for the SquadSizeIUnlock and SquadSizeIIUnlock GTS upgrades. This can present a bit of a problem if mods remove these upgrades or add alternate methods to increase squad size. An event trigger with passed tuple should be added to X2AchievementTracker, so mods can override the default behaviour.

[Iridar]

I don't think this a concern, arguably attachments shouldn't even be allowed with mods.

[Xymanek]

arguably attachments shouldn't even be allowed with mods

I assume you meant achievements and IMO since FXS didn't introduce any such restriction it's not up to us to do so. Besides, I think that as long as the game isn't different to the point where it's unrecognizable, it's fine to keep the achievements

[Iridar]

Derp, achievements, yes. I don't really care about achievements either way, but I don't see a reason to bloat the codebase with features just for their sake, particularly when it's seemingly an "all or nothing" issue, seems like a slippery slope.

[Xymanek]

To be blunt - I do not share your concerns. Let's view it from a bit of a different angle: how should we handle the situation? Options:

• Do nothing. Players have already reported the behaviour as a bug and personally I'm against granting achievements by "accident" - if people want mods that grant achievements, that exists already
• Do some hacky stuff if the player purchased our version of the upgrades. Risks screwing up other mods
• Make this hook. Small hook, which runs once per campaign, and is absolutely not noticeable in the endless pool of other hooks that already exist

[Iridar]

Fair enough, just wanted to say my piece.

[robojumper]

Is one event for a single achievement the right approach? One event could cover all achievement triggers here:

function UnlockAchievement(EAchievementType Type)
{
    DevOnlineMsg("UnlockAchievement: " $ AreAchievementsEnabled() $ ": " $ Type);

    if (AreAchievementsEnabled())
    {       
        OnlineSub.PlayerInterface.UnlockAchievement(LocalUserIndex, Type);
    }

    CheckGameProgress(Type);
}

[Xymanek]

I would be cautious against adding an event to XComOnlineEventMgr::UnlockAchievement due to register in strategy/tactical/etc situation - it would need to be a DLCInfo hook or a delegate in CHHelpers

[cannonfodder1]

I could expand the event hook to cover everything in the FinalMissionOnSuccess() function. There are some others here that would be good to have control over, like “Who Needs Tygan?”

[Xymanek]

@cannonfodder1 robo's suggestion is about inserting a hook into the global "trigger achievement" function so that mods could filter each achievement trigger individually, to avoid the need to patch each trigger site individually

[cannonfodder1]

@cannonfodder1 robo's suggestion is about inserting a hook into the global "trigger achievement" function so that mods could filter each achievement trigger individually, to avoid the need to patch each trigger site individually

I thought you said that was a bad idea but after reading your earlier comment more carefully I see you were just giving suggestions on how to do it better. I’ll get to to work on that

[Xymanek]

Yeah, I just meant that a hook like that would need to be either a DLCInfo or a CHHelpers delegate (see EHLDelegateReturn and onwards in CHHelpers)

[Xymanek]

So I thought about this for a while and I think it's best to leave the current implementation as-is. XComOnlineEventMgr::UnlockAchievement is a great idea in theory but in practice it has several issues:

• Can interfere with mods like Achievement Unlocker and similar
• Can interfere with XComCheatManager::TestUnlockAchievement
• Mods need to be aware (e.g. additional checks) of whether the game is currently in shell/mp/strategy/tactical/etc. so that they don't read the history incorrectly by accident

As such, I think it's best to leave it at a specific trigger override, at least until we have an actionable use case for a wider implementation