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

OpenMU/src/GameLogic/AttackableExtensions.cs

Lines 78 to 81 in 6b93306

	dmg = Math.Max(dmg, 0);
	dmg = (int)(dmg * defender.Attributes[Stats.DamageReceiveDecrement]);
	dmg = (int)(dmg * attacker.Attributes[Stats.AttackDamageIncrease]);

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?

OpenMU/src/Persistence/Initialization/VersionSeasonSix/Items/Wings.cs

Lines 160 to 165 in 5602541

	if (damageAbsorbInitial > 0)
	{
	var powerUp = this.CreateItemBasePowerUpDefinition(Stats.DamageReceiveDecrement, 0f - (damageAbsorbInitial / 100f));
	powerUp.BonusPerLevelTable = this._absorbByLevelTable;
	wing.BasePowerUpAttributes.Add(powerUp);
	}

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.