fa
fa copied to clipboard
FAForever
Overhaul stun logic to match the stun radius with the damage radius
As discussed in #4004 the stun radius does not match the damage radius. As a result we have #3478 and https://github.com/FAForever/fa/pull/3987 that try and fix this radius. But to no avail, as the query is fundamentally different:
- For damage the query is a sphere -> collision box intersection
- For stun the query was a sphere -> unit center intersection
As a result, the stun radius will always be off depending on the unit that you're applying the stun to. This pull request introduces a new damage type (coined 'Stun') that is ignored by entities except for units that the stun is applicable to.
For clarity, this pull requests fixes:
- Stun being applied through shields
- Stun not being able to match the damage radius
And because of that:
- Shielded units (personal shield or covered in shield from damage source) no longer get stunned
- Units are in general a lot easier to stun, especially naval units
Closes #3997
I like the overhaul but would it be possible to still stun targets that have a personal shield? Otherwise, it would make Obsidians really strong and an Aeon vs Aeon match-up with Chrono would be awkward, to say the least.
The only unit that is skipped is the chrono field, as that should be tackled with #3918 once that is more finalized.
It looks like you've flattened the stun durations for the different tech levels. I know adding more buffs with different target restrictions is a yucky way to it, but I wonder if we could change that with this PR by adding different stun durations for different tech levels in the same buff, and then pull that stun value out of the buff depending on the affected unit tech level in the OnDamage method. It'd definitely reduce the amount of fine-tuning you could do with TargetAllow and TargetDisallow though.
I flattened them because they felt unintuitive from a user perspective. The UI doesn't indicate the duration of the stun, yet some units are 'unstunned' sooner than others.
I'd be interested in having some tech-based curve on stuns if it is easy to understand for the user. But until that day, I'd rather the stun duration to be 'reliable' for the user: it either stuns, or doesn't stun. And if it stuns, it stuns for the same duration for each instance.
Only thing wrong I found was the sacu does not explode.
This is due to an issue unrelated to this repository, it is fixed on FAF Develop already!
I don't think that the stun duration should be flattened for different tech tiers. I understand your UX concern but I think that the balance reasons outweigh it as with a flat number you either need to keep it low enough to not make the stun OP vs higher tech tiers and then it's pretty useless vs tech 1 and feels underwhelming over all. Personally I don't think it's a big deal for UX nor is it super unintuitive. Higher tech units are more advanced and therefore less vulnerable to the EMP.
The only two units that are not flat already are the Medusa (url0103) and the SACU (url0301), there are (in my opinion) better ways of applying a curve than trying to stun multiple times in an area. Especially when we're talking about the overhead in terms of performance 😄 .
Note that typically higher tech units do not get stunned at all. As an example, the Medusa doesn't stun tech 3+ units.
Would we be interested in a global stun curve of something like
(don't worry, I already have code for it)
We would then set the area buff code to apply stun time based on the curve (and setup stun resistances for either the tech levels or tech level differences, and also set stun falloffs for the launchers), instead of creating multiple area buffs with different stun times and specific targets.
-- I've left most of the code unoptimized, except for adding memoization (a critical piece for performance)
-- this function would ideally be optimized for various support parameter counts, and allow multiple returns
-- in particular, if it has only one argument, it doesn't use it as the key without turning it to a string,
-- so the inflection table will get all of the original values as string keys too
function Memoize(fn, cache)
cache = cache or {}
-- a table with a __index metafield might be a better alternative to this lookup function
return function(...)
local key = table.concat(args, ',')
local stored = cache[key]
if stored then
return stored
end
local value = fn(unpack(args))
cache[key] = value
return value
end
end
-- basis of the stun curve. the rest of the code just standardizes the curve with respect to the falloff
-- parameter by shifting and scaling by the inflection point
function RawStunCurve(falloff, resistance)
return math.pow(1 + falloff, -resistance) / (1 + math.pow(falloff, -resistance))
end
-- don't have a closed-form expression for the inflection point, so just plug
-- x = (p^(x + 1) - p^(-x) + p - 1)/(ln((p + 1)^(p - 1) p^(-2 p - 1)) + p^(x + 1) ln(p + 1) + ln(p/(1 + p))/p^x)
-- into your favorite equation solver to get a few values (where p is the falloff), and use logarithmic
-- interpolation for everything in-between
-- (it probably doesn't even need to be that accurate, since the values only range from -.59 to -.38)
local inflections = {
[1] = -0.584187070149687,
[1.25] = -0.583036548171388,
[1.5] = -0.580405674598629,
[2] = -0.573272758504428,
[3] = -0.557521806487837,
[4] = -0.542838508621603,
[5] = -0.529782219491405,
[6] = -0.518212959135337,
[7] = -0.507903184616362,
[10] = -0.482647062730128,
[15] = -0.452541624988572,
[20] = -0.430952063220271,
[23] = -0.420535274239504,
[30] = -0.401039455921266,
[40] = -0.380599943010585,
-- any higher falloff parameters are going to have a bad time being the base of a power function
}
function StunCurveInflection(falloff)
if inflections[falloff] then -- catch the original values the memoizer left because it wants a string key
return inflections[falloff]
end
local below, above = 1, 99999
local belowValue, aboveValue = inflections[1], 0
for k, v in inflections do
if k < falloff then
if k > below then
below = k
belowValue = v
end
else
if k < above then
above = k
aboveValue = v
end
end
end
-- logarithmic interpolation
local dx = (math.log(falloff) - math.log(below)) / (math.log(above) - math.log(below))
return dx * (aboveValue - belowValue) + belowValue
end
StunCurveInflection = Memoize(StunCurveInflection, inflections)
function StunCurveScale(falloff)
return RawStunCurve(falloff, StunCurveInflection(falloff))
end
StunCurveScale = Memoize(StunCurveScale)
function StunModifier(falloff, resistance)
-- if falloff is between 0 and 1, the curve begins to reverse its shape, so force the modifier at x1
-- also, I didn't include inflections below 1, so it'll just be NaN anyway
if falloff < 1 then
return 1
end
return RawStunCurve(falloff, resistance + StunCurveInflection(falloff)) / StunCurveScale(falloff)
end
StunModifier = Memoize(StunModifier)
function StunTime(dur, falloff, resistance)
return dur * StunModifier(falloff, resistance)
end
@Tagada14 we need to make a call on two things, one of them being whether we want curve-based damage, such as the suggestion of @Hdt80bro
And, even if it would make some fights awkward - for consistency, I think any unit protected by some shield should not be stunnable. After all - they are protected by a shield. It'd add a bit of depth to the game, and give units with a shield a clear buff (that they lack when gaining vet, as the shield isn't taken into account there).
Curve-based stun duration would be nice to have but it's not a necessity. Units with personal shields getting stunned are IMO a must-have. It would break balance a lot and in my opinion be more unintuitive (You play zoomed out and you see 3/4 of your units get stunned while a couple of tanks are not, only after a couple of seconds do you remember that those tanks are obsidian so they have shields thus they are not getting stunned)
@Garanas I think that in order to get this wonderful change into the works, we need to table the ancillary discussions about shield stunning and curve flattening for another time. Those are separate things we can come back to later if we'd like. For now, I think that means leaving those two behaviors alone in this PR--which, I know... means re-adding several inefficient area buffs to get that tiered stun effect. Just think of it as something to motivate a future solution, or something.
I'll pick this up when I'm back! I'm not looking forward to adding back the inefficient area buffs - @Tagada14 could you discuss with the balance team whether stuns should or should not pierce shields? The old implementation allowed them to pierce shields, this implementation prevents that for most shields, except for personal shields.
I think it intuitively makes sense for shields to block stuns coming from outside of the shields, and it seems like a nice feature for increased depth and strategic potential. Also, if necessary, I figure that units with shields could be rebalanced some to accommodate such a change.
Hi. What is needed to get this going again? Should I help to test or are there more changes needed?
At this point, I doubt the balance conversation will be resolved. So we need to remove all behavioral discrepancies this PR produces as best as possible and somehow convince the balance team that dome shields blocking stunning is a good idea.
Do we really need to update all the units at once? Would it be easier to get this through if it was just the t1 units, mainly the medusa? It gets used in any game that has a cybran player, gives up a large chunk of dps to have a stun that does not work half the time. A flat 2 seconds stun on t1 and t2 makes sense. Its on the lower side of its stun duration and will work when it should.
In my opinion, stuns should not be able to pierce Dome shields (eg. Static shield generators, mobile shields) but they should stun units with personal shields. I will discuss it with the rest of the team as well.
Ok, so to make it penetrates shields we need some AOE thing that ignores shields and applies damage to units at the same time. Actually, this is how allies aoe damage works right now. What if we use DamageArea() with dummy (or any) unit from opposite team as instigator and with damageFriendly = true?
add. Also in this case the stun damage will be also applied to allies units, because instigator is from enemy team, so one additional check will be needed (apply stun from "allies" and ignore from "enemies".)
