Guardian Angel damage absorption works in reverse

[halflumi]

Describe the bug Guardian Angel makes characters take 20% more damage instead of 20% less.

To Reproduce Steps to reproduce the behavior:

1. Get hit by a mob with nothing equipped and take 80 points of damage.
2. Get hit by a mob again with Guardian Angel equipped and take 119 points of damage.

Expected behavior Should take less damage with Guardian Angel equipped.

Additional context

https://github.com/MUnique/OpenMU/blob/6b933062f3de667ac67c279355e15fbc6640812b/src/GameLogic/AttackableExtensions.cs#L78-L81

Stats.DamageReceiveDecrement seems to be working fine. During the repro, defender.Attributes[Stats.DamageReceiveDecrement] has a value of 0.44617185 without equipping Guardian Angel and 0.60272336 with it equipped.

Modifying Guardian Angel's Damage Receive Multiplier value from 0.2 to -0.2 seems to fix the issue. Now Stats.DamageReceiveDecrement is changed from 0.44617185 to 0.28962034 when equipping Guardian Angel.

The issue seems to be universal to items with positive Damage Receive Multiplier modifiers. For example, Horn of Dinorant is supposed to absorb 10% of the damage, but it changes Stats.DamageReceiveDecrement from 1.0 to 1.1.

To that end, I guess the sensible fix is to change all positive Damage Receive Multiplier to negative for all erroneous items. Though I'm not sure if that's the best fix. Any thoughts on this?

[sven-n]

Hmm, I think the solution looks different. For the Guardian Angel the value should be +0.8, and the InputOperator needs to be changed to Multiply. 0.44617185 * 0.8 = 0,35693748

[halflumi]

That does look more correct.

I've printed out all the references of Stats.DamageReceiveDecrement (relevant parts attached at the bottom). Looks like we would be in the clear if the value of Guardian Angel, Horn of Dinorant, and Spirit of Guardian is changed to accommodate the multiplicative calculation.

To that end, I guess we gotta update the Harmony option that affects Stats.DamageReceiveDecrement as well, so changing 1.03f, 1.04f, 1.05f, 1.06f, 1.07f to 0.97f, 0.96f, 0.95f, 0.94f, 0.93f.

As for wings, they seem to be taking the negative addition approach. Shall we update wings to use the positive multiplicative calculation as well?

https://github.com/MUnique/OpenMU/blob/5602541937f9a50fbebbe203562f6c6f2734e370/src/Persistence/Initialization/VersionSeasonSix/Items/Wings.cs#L160-L165

src\Persistence\Initialization\CharacterClasses\CharacterClassInitialization.cs:135
    baseAttributeValues.Add(this.CreateConstValueAttribute(1, Stats.DamageReceiveDecrement));
src\Persistence\Initialization\Items\ExcellentOptions.cs:124
    definition.PossibleOptions.Add(this.CreateExcellentOption(4, Stats.DamageReceiveDecrement, 0.96f, AggregateType.Multiplicate, ItemOptionDefinitionNumbers.ExcellentDefense));
src\Persistence\Initialization\Skills\DefenseEffectInitializer.cs:42
    powerUpDefinition.TargetAttribute = Stats.DamageReceiveDecrement.GetPersistent(this.GameConfiguration);
src\Persistence\Initialization\Skills\SoulBarrierEffectInitializer.cs:39
    powerUpDefinition.TargetAttribute = Stats.DamageReceiveDecrement.GetPersistent(this.GameConfiguration);
src\Persistence\Initialization\VersionSeasonSix\Items\HarmonyOptions.cs:123
    definition.PossibleOptions.Add(this.CreateHarmonyOptions(7, ItemOptionDefinitionNumbers.HarmonyDefense, Stats.DamageReceiveDecrement, AggregateType.Multiplicate, 9, 1.03f, 1.04f, 1.05f, 1.06f, 1.07f));
src\Persistence\Initialization\VersionSeasonSix\Items\Pets.cs:37
    this.CreatePet(0, 0, 1, 1, "Guardian Angel", 23, true, true, (Stats.DamageReceiveDecrement, 0.2f), (Stats.MaximumHealth, 50f));
src\Persistence\Initialization\VersionSeasonSix\Items\Pets.cs:4
    var dinorant = this.CreatePet(3, SkillNumber.FireBreath, 1, 1, "Horn of Dinorant", 110, false, true, (Stats.DamageReceiveDecrement, 0.1f), (Stats.AttackDamageIncrease, 0.15f), (Stats.CanFly, 1.0f));
src\Persistence\Initialization\VersionSeasonSix\Items\Pets.cs:58
    this.CreatePet(65, 0, 1, 1, "Spirit of Guardian", 1, false, true, (Stats.DamageReceiveDecrement, 0.3f), (Stats.MaximumHealth, 50f));
src\Persistence\Initialization\VersionSeasonSix\Items\Pets.cs:224
    dinoOptionDefinition.PossibleOptions.Add(this.CreateOption(ItemOptionTypes.Excellent, 1, Stats.DamageReceiveDecrement, 0.95f, AggregateType.Multiplicate, ItemOptionDefinitionNumbers.Dino));
src\Persistence\Initialization\VersionSeasonSix\Items\Pets.cs:247
    fenrirOptionDefinition.PossibleOptions.Add(this.CreateOption(ItemOptionTypes.BlueFenrir, 2, Stats.DamageReceiveDecrement, 0.95f, AggregateType.Multiplicate, ItemOptionDefinitionNumbers.Fenrir));
src\Persistence\Initialization\VersionSeasonSix\Items\SocketSystem.cs:349
    definition.PossibleOptions.Add(this.CreateSocketOption(3, SocketSubOptionType.Water, Stats.DamageReceiveDecrement, AggregateType.Multiplicate, 0.96f, 0.95f, 0.94f, 0.93f, 0.92f));
src\Persistence\Initialization\VersionSeasonSix\Items\Wings.cs:162
    var powerUp = this.CreateItemBasePowerUpDefinition(Stats.DamageReceiveDecrement, 0f - (damageAbsorbInitial / 100f));

src\Persistence\Initialization\VersionSeasonSix\TestAccounts\Level300.cs... // The rest seems to be correct.

EDIT: Fix typos.

[sven-n]

I'm not sure how it's done at the original servers, but I think it makes sense to change that for wings as well. Then the behavior is constistent to the rest 😄

To fix that, not only the Initialization-code has to be changed. We also need update-code for existing databases. For these updates, we usually create plugins which implement IConfigurationUpdatePlugIn in the MUnique.OpenMU.Persistence.Initialization.Updates namespace. In this case we'd need three separate plugins for the supported game versions (0.75, 0.95d, Season 6).

[halflumi]

Thanks for the heads-up. I'd agree that we can at least keep things consistent in OpenMU. I'll get a proper fix later.

[Niter88]

Hello @halflumi Is this still being looked over?

[sven-n]

@Niter88 Feel free to work on it. I doubt he is working on that a half year later :)

[halflumi]

@Niter88 Definitely take it if ya feel like it. I don't think I'll get the chance to work on OpenMU before the end of this year.

[sven-n]

Hey, I looked after this issue today. Changing the calculation of the wings and the soul barrier skill was a bit more complicated than I thought before. I had to extend the attribute system for that. My current state is at PR #456 😅