X2CommunityCore / X2WOTCCommunityHighlander

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

Integrate Avenger Defense Bondmates Fix #1059

Open Iridar opened 3 years ago

Iridar commented 3 years ago

A cousin of #1029

Standalone mod that fixes the problem:

https://steamcommunity.com/sharedfiles/filedetails/?id=1176368634

pledbrook commented 3 years ago

The issue seems to be in X2Condition_Bondmate.CanEverBeValid(), which uses XComHQ.IsUnitInSquad(BondmateRef) to check whether the bondmate is in the mission. But this doesn't apply for units that are spawned from the Avenger.

I think the check should be removed from the condition since it can't ever reliably determine whether the ability "can ever be valid". I'm just not sure what the impact of removing it would be.

Iridar commented 3 years ago

I'm just not sure what the impact of removing it would be.

Marginal performance reduction due to the game validating this condition even if the soldier's bondmate is not on the mission and might never be.

pledbrook commented 3 years ago

Another thing to consider is whether this is an actual bug/oversight. It might actually be intentional and fixing it changes intended behaviour. That's outside the scope of the Community Highlander. I'm fairly sure this was an oversight and bondmates on the same mission should always have access to their abilities, regardless of whether they were in the original squad or not. But I'm not certain.

Iridar commented 3 years ago

I don't think soldiers forgetting about being best-friends-forever if they didn't walk out the Avenger/Skyranger together can be reasonably called as being an intentional design decision.

Xymanek commented 3 years ago

Instead of clearing X2Condition_Bondmate.CanEverBeValid, maybe it's possible to approach this more intelligently and force-reinit all abilities that use X2Condition_Bondmate after spawning unit(s) from the avenger?

Iridar commented 3 years ago

Instead of clearing X2Condition_Bondmate.CanEverBeValid, maybe it's possible to approach this more intelligently and force-reinit all abilities that use X2Condition_Bondmate after spawning unit(s) from the avenger?

This seems like a more complex solution with no upside. Is there a specific case you want to guard against?

EDIT: Oh, I see, what you mean. Add abilities to bondmates when their mate enters tactical. That could work.

pledbrook commented 3 years ago

I don't know if that's "more intelligently". If you're forcing initialisation of bondmate abilities on units entering tactical, you're implicitly saying that CanEverBeValid() is wrong. I can see taking such an approach if simply fixing CanEverBeValid() has undesirable consequences, but we don't know if there are any yet.

Iridar commented 3 years ago

I can see taking such an approach if simply fixing CanEverBeValid() has undesirable consequences, but we don't know if there are any yet.

I have a pretty good grasp on CanEverBeValid(). It's a simple bool return function. If it returns false when the unit enters tactical combat, the ability is not added to the unit. That's it.

So if a unit enters tactical without their bondmate, CanEverBeValid() will fail, and the unit won't have their bondmate abilities at all.

Then if the units bondmate enters combat, they will still be missing these abilities, because they were never added.

Completely removing CanEverBeValid() will simply mean the abilities are always added to the unit, regardless if their bondmate is currently in squad or not.

The player will still be unable to use the abilities, because the condition itself will fail.

The only downside to completely removing CanEverBeValid() is the increased performance cost caused by evaluating the condition alongside all other unit's abilities. Not absolutely optimal, but overall negligible in the grand scheme of things, since the same condition evaluations happen when the unit is with their bondmate on the mission anyway.

pledbrook commented 3 years ago

The player will still be unable to use the abilities, because the condition itself will fail.

The thing I was uncertain about was whether the icon appears in the ability bar at all if the bond mate is not on the mission, or whether it appears disabled.

Iridar commented 3 years ago

The player will still be unable to use the abilities, because the condition itself will fail.

The thing I was uncertain about was whether the icon appears in the ability bar at all if the bond mate is not on the mission, or whether it appears disabled.

That will depend on how the ability template is set up. Technically it's in our grasp to change.

Iridar commented 1 year ago

I checked, the only active bondmate ability, the one that gives an action point, uses eAbilityIconBehavior_AlwaysShow, which means if we do the proposed change to X2Condition_Bondmate, then the ability will still show up on the bar if the bondmate is not on the mission, which is a bit annoying.

The standalone bugfix uses FinalizeAbilitiesForDLCInit hook when it's called for Bondmate B to call TACTICALRULES.InitAbilityForUnit() for Bondmate A that was already on the mission. Kinda hacky, but it works. I don't think a similar fix is applicable within Highlander, though.

I think the most graceful solution here is to adjust the X2Condition and use the OverrideAbilityAvailabilityFn on the the bondmate ability template to not show up if the bondmate is not on the mission. Using eAbilityIconBehavior_HideSpecificErrors with 'AA_NoTargets' in HideErrors wouldn't be good enough, since it would modify the UI behavior of the ability.

Though the approach Xym suggested would work too, and it's basically what the standalone bugfix does anyway.

Iridar commented 1 year ago

Thought about it some more, and I want to expand on two solutions that have been suggested so far:

Xym's Solution

When Bondmate B enters tactical (gets their abilities initialized), also initialized bondmate abilities for Bondmate A that was already on the mission.

This is problematic, because there's no good way for us to find bondmate abilities to init other than using a config list or hardcoded list of vanilla bondmate abilities.

We can't just blindly initialize all abilities that have an X2Condition_Bondmate, because we can't tell tell if those abilities were not previously initialized specifically and only because of that condition.

My Solution

Modify X2Condition_Bondmate so it no longer does IsUnitInSquad() check, so bondmate abilities are always initialized for Bondmate A regardless if Bondmate B is on the mission or not, so when/if Bondmate B does enter the mission, they can just work.

This has downsides: at the very least, the Teamwork ability will always appear in UI, even if the Bondmate B is not on the mission.

We can potentially modify the OverrideAbilityAvailabilityFn delegate on its ability template so the ability doesn't show up if the bondmate is not in Squad. However, the passive bondmate abilities will still show up on the unit in the lower left corner, we can't do much about that.

This solution would also necessarily apply only to vanilla bondmate abilities.

Conclusion

It seems the best we can do is basically copy paste the existing bugfix mod into the Highlander, including the config array of valid bondmate abilities, which seems like a crappy half measure.