reznok / GMCAbilitySystem

An Unreal Engine plugin that adds an ability system to the General Movement Component plugin
MIT License
28 stars 13 forks source link

Adding Attribute Modifiers #27

Closed reznok closed 4 months ago

reznok commented 4 months ago

This is my initial stab at doing attribute modifiers.

The following modifiers exist:

Add
Multiply
Divide

The Attribute now keeps track of its BaseValue, an AdditiveMultiplier, a MultiplyModifier, and a DivideModifier.

Additive Multiplier starts at 0, Multiply and Divide start at 1.

When you apply an Add effect, it just adds or subtracts from the Additive value.

For Multiply/Divide, the same is true. If you apply an effect with a Multiply type and a value of 1, it will add 1 to the MultiplyModifier. This would result in doubling the value. Which is weird if you don't know it, but I think it's the easiest way to add this.

The actual Value calc is done with:

Value = (AdditiveModifier + (BaseValue * MultiplyModifier)) / DivideModifier;

(There are checks that happen before the calc to make sure Multiply/Divide modifiers aren't negative, and prevent divide by 0)

So say you have 100 base value, and you have a +1 mod to multiply and divide, you would get a total value of BaseValue.

Value = (0 + (100 * 2)) / 2
Value = 200 / 2
Value = 100

I think there's a few ways that Value calc can be done, and I'm not sure any one is objectively correct, but this is my first stab at it.

Packetdancer commented 4 months ago

Obviously this works differently than my attribute modifier stacks that we've discussed in the past, but the one thing I think would be worth taking from those and applying here would be the optional MinValue and MaxValue I put on attributes.

While it's unlikely in most general use cases that this will become a problem, if anyone is doing something like "Attribute X is your stacking weapon power per-headshot, and then we take Attribute X's value and use it as the multiply modifier in the Damage attribute," you can definitely run into unexpected scenarios where a value gets outside of the range the devs expected it to live within. Having a MinValue and MaxValue which can be set basically just gives some easily-applied guardrails at the attribute level, rather than requiring it be handled case-by-case in the game logic itself.

Otherwise, this looks like a straightforward implementation. 👍

reznok commented 4 months ago

Closes #19

LeoFabre commented 4 months ago

I agree with @Packetdancer about having boundaries to attributes instead of having to clamp them manually using OnAttributeChanged, that will make users code cleaner.

Thanks for the modifiers ! 🕺

reznok commented 4 months ago

It's not a good place to clamp, as if you want to Clamp to a value based on an Attribute, it gets messy. It is good to clamp for "breaking" values, like overflows.

LeoFabre commented 4 months ago

Oh yeah I read that wrong, I see 👍