Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions src/AttributeSystem/ComposableAttribute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,18 +68,10 @@ private float GetAndCacheValue()
var maxValues = this.Elements.Where(e => e.AggregateType == AggregateType.Maximum).MaxBy(e => e.Value)?.Value ?? 0;
rawValues += maxValues;

if (multiValues == 0 && this.Elements.All(e => e.AggregateType != AggregateType.Multiplicate))
{
multiValues = 1;
}
Comment on lines -71 to -74
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an impossible situation since in line 66 an enumerable with "1" is concatenated.

else if (rawValues == 0 && multiValues != 0 && this.Elements.All(e => e.AggregateType != AggregateType.AddRaw))
if (this.Elements.All(e => e.AggregateType == AggregateType.Multiplicate))
{
rawValues = 1;
}
Comment on lines +71 to 74
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The new logic for defaulting rawValues to 1 is more restrictive than the previous implementation. By checking if all elements are of type Multiplicate, it fails to handle scenarios where the attribute contains other non-raw elements (like AddFinal or Maximum) but no AddRaw elements. In Mu Online's attribute system, multipliers should generally apply to a base of 1 if no explicit raw additions are present. The previous check this.Elements.All(e => e.AggregateType != AggregateType.AddRaw) correctly identified the absence of a base value regardless of other non-raw elements.

        if (this.Elements.All(e => e.AggregateType != AggregateType.AddRaw))
        {
            rawValues = 1;
        }

Copy link
Copy Markdown
Contributor Author

@ze-dom ze-dom May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would fail if we have only AggregateType.AddFinal values. Suppose we only have a +3 AggregateType.AddFinal. Then, we get: 1*1 + 3 = 4, instead of 3.

In case the attribute has different values, like AggregatyeType.AddFinal and AggregateType.Multiplicate values, I think it makes more sense to explicitly add a AggregateType.AddRaw value of 1 (this seems to be the standard anyway), otherwise it becomes too messy with assumptions.

else
{
// nothing to do
}

var newValue = (rawValues * multiValues) + finalValues;
if (this._maximumValue.HasValue)
Expand Down
24 changes: 21 additions & 3 deletions src/GameLogic/AttackableExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@ public static class AttackableExtensions
{
private const short ExplosionMagicEffectNumber = 75; // 0x4B

private const short StunnedMagicEffectNumber = 61; // 0x3D

private static readonly IDictionary<AttributeDefinition, AttributeDefinition> ReductionModifiers =
new Dictionary<AttributeDefinition, AttributeDefinition>
{
{ Stats.CurrentMana, Stats.ManaUsageReduction },
{ Stats.CurrentAbility, Stats.AbilityUsageReduction },
};

private static MagicEffectDefinition? stunEffectDefinition;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The static field stunEffectDefinition is shared across all instances and is not initialized in a thread-safe manner. In environments with multiple GameContext instances (e.g., multi-tenant servers or unit tests), this could lead to cross-context contamination if different configurations are used. Additionally, the lazy initialization ??= is subject to race conditions. Consider retrieving the definition from the attacker.GameContext directly within the method or using a thread-safe caching mechanism if performance is a concern.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to make sense. Remove static, @sven-n ?


extension(IAttackable attackable)
{
/// <summary>
Expand Down Expand Up @@ -327,7 +331,7 @@ public static async ValueTask ApplyMagicEffectAsync(this IAttackable target, IAt
}

float chance = target is Player ? skillEntry.PowerUpChancePvp!.Value : skillEntry.PowerUpChance!.Value;
if (!Rand.NextRandomBool(Convert.ToDouble(chance)))
if (!Rand.NextRandomBool(chance))
{
return;
}
Expand Down Expand Up @@ -597,6 +601,20 @@ public static double CalculateBaseExperience(this IAttackable killedObject, floa
return Math.Max(tempExperience, 0) * 1.25;
}

/// <summary>
/// Applies the mace mastery stun effect to the specified attackable.
/// </summary>
/// <param name="attacker">The attacker.</param>
/// <param name="attackable">The attackable.</param>
/// <returns>A task representing the asynchronous operation.</returns>
public static async ValueTask ApplyMaceMasteryStunEffectAsync(this Player attacker, IAttackable attackable)
{
stunEffectDefinition ??= attacker.GameContext.Configuration.MagicEffects.First(m => m.Number == StunnedMagicEffectNumber);
var powerUp = attackable.Attributes.CreateElement(stunEffectDefinition.PowerUpDefinitions.First(pu => pu.TargetAttribute == Stats.IsStunned));
var magicEffect = new MagicEffect(TimeSpan.FromSeconds(2), stunEffectDefinition, [new MagicEffect.ElementWithTarget(powerUp, Stats.IsStunned)]);
await attackable.MagicEffectList.AddEffectAsync(magicEffect).ConfigureAwait(false);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private static bool IsAttackSuccessfulTo(this IAttacker attacker, IAttackable defender)
{
var hitChance = attacker.GetHitChanceTo(defender);
Expand Down Expand Up @@ -863,8 +881,8 @@ private static async ValueTask ApplyMagicEffectAsync(this IAttackable target, IA
}
else if (magicEffectDefinition.PowerUpDefinitions.Any(e => e.TargetAttribute == Stats.IsStunned))
{
var stunChancePowerUp = powerUps.FirstOrDefault(p => p.Target == Stats.StunChance);
if (stunChancePowerUp.Boost is null || !Rand.NextRandomBool(Convert.ToDouble(stunChancePowerUp.Boost.Value)))
var stunChancePowerUp = powerUps.FirstOrDefault(p => p.Target == Stats.MasteryStunChance);
if (stunChancePowerUp.Boost is null || !Rand.NextRandomBool(stunChancePowerUp.Boost.Value))
{
return;
}
Expand Down
23 changes: 15 additions & 8 deletions src/GameLogic/Attributes/PowerUpWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ public sealed class PowerUpWrapper : IElement, IDisposable
{
private readonly IElement _element;

private readonly AggregateType? _aggregateType;

private ComposableAttribute? _parentAttribute;

/// <summary>
Expand All @@ -22,7 +24,8 @@ public sealed class PowerUpWrapper : IElement, IDisposable
/// <param name="element">The element.</param>
/// <param name="targetAttribute">The target attribute.</param>
/// <param name="attributeHolder">The attribute holder.</param>
public PowerUpWrapper(IElement element, AttributeDefinition targetAttribute, AttributeSystem attributeHolder)
/// <param name="aggregateType">The aggregate type that should override the <paramref name="element"/>'s.</param>
public PowerUpWrapper(IElement element, AttributeDefinition targetAttribute, AttributeSystem attributeHolder, AggregateType? aggregateType = null)
{
this._parentAttribute = attributeHolder.GetComposableAttribute(targetAttribute);
if (this._parentAttribute is null)
Expand All @@ -31,6 +34,7 @@ public PowerUpWrapper(IElement element, AttributeDefinition targetAttribute, Att
}

this._element = element;
this._aggregateType = aggregateType;
this._parentAttribute.AddElement(this);
this._element.ValueChanged += this.OnValueChanged;
}
Expand All @@ -42,33 +46,36 @@ public PowerUpWrapper(IElement element, AttributeDefinition targetAttribute, Att
public float Value => this._element.Value;

/// <inheritdoc/>
public AggregateType AggregateType => this._element.AggregateType;
public AggregateType AggregateType => this._aggregateType ?? this._element.AggregateType;

/// <summary>
/// Creates elements by a <see cref="PowerUpDefinition"/>.
/// </summary>
/// <param name="powerUpDef">The power up definition.</param>
/// <param name="attributeHolder">The attribute holder.</param>
/// <param name="aggregateType">The specific aggregate type. If not specified, the aggregate type of the <paramref name="powerUpDef"/>'s constant value will be used.</param>
/// <returns>The elements which represent the power-up.</returns>
public static IEnumerable<PowerUpWrapper> CreateByPowerUpDefinition(PowerUpDefinition powerUpDef, AttributeSystem attributeHolder)
public static IEnumerable<PowerUpWrapper> CreateByPowerUpDefinition(PowerUpDefinition powerUpDef, AttributeSystem attributeHolder, AggregateType? aggregateType = null)
{
if (powerUpDef.Boost?.ConstantValue != null)
{
yield return new PowerUpWrapper(
powerUpDef.Boost.ConstantValue,
powerUpDef.TargetAttribute ?? throw Error.NotInitializedProperty(powerUpDef, nameof(PowerUpDefinition.TargetAttribute)),
attributeHolder);
attributeHolder,
aggregateType);
}

if (powerUpDef.Boost?.RelatedValues != null)
{
foreach (var relationship in powerUpDef.Boost.RelatedValues)
{
var aggregateType = powerUpDef.Boost?.ConstantValue.AggregateType ?? AggregateType.AddRaw;
var aggregType = powerUpDef.Boost?.ConstantValue.AggregateType ?? AggregateType.AddRaw;
yield return new PowerUpWrapper(
attributeHolder.CreateRelatedAttribute(relationship, attributeHolder, aggregateType),
attributeHolder.CreateRelatedAttribute(relationship, attributeHolder, aggregType),
powerUpDef.TargetAttribute ?? throw Error.NotInitializedProperty(powerUpDef, nameof(PowerUpDefinition.TargetAttribute)),
attributeHolder);
attributeHolder,
aggregateType);
}
}
}
Expand All @@ -88,7 +95,7 @@ public void Dispose()
/// <inheritdoc/>
public override string? ToString()
{
return $"{this._parentAttribute?.Definition.Designation}: {this._element}";
return $"{this._parentAttribute?.Definition.Designation}: {this._element}{(this._aggregateType is { } aggreg ? $"->({aggreg})" : string.Empty)}";
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that one can see that this aggregate type has been overriden.
E.g.: {Physical Base Damage: 20 (AddRaw)->(AddFinal)}

}

private void OnValueChanged(object? sender, EventArgs eventArgs)
Expand Down
50 changes: 45 additions & 5 deletions src/GameLogic/Attributes/Stats.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,14 @@ public class Stats
/// <summary>
/// Gets the min and max physical base DMG attribute definition.
/// </summary>
/// <remarks>
/// <see cref="AggregateType.AddRaw"/> values include:
/// Weapon item option; excellent lvl/20 option (weapons)
/// <see cref="AggregateType.Multiplicate"/> values include:
/// Double wield halving (averaging).
/// <see cref="AggregateType.AddFinal"/> values include:
/// Excellent lvl/20 option (pendant); wings damage option; harmony damage (min and max) option; gold fenrir damage bonus; <see cref="BaseDamageBonus"/>.
/// </remarks>
public static AttributeDefinition PhysicalBaseDmg { get; } = new(new Guid("DD1E13E4-BFFD-45B5-9B91-9080710324B2"), "Physical Base Damage (min and max)", string.Empty);

/// <summary>
Expand Down Expand Up @@ -421,7 +429,12 @@ public class Stats
/// <summary>
/// Gets the physical base (min and max) damage increase attribute definition>.
/// </summary>
/// <remarks>Includes excellent 2% physical increase option, ammunition damage increase, and the double wield multiplier (55%).</remarks>
/// <remarks>
/// <see cref="AggregateType.AddRaw"/> values include:
/// Excellent 2% increase option (double wield weapons); ammunition damage increase.
/// <see cref="AggregateType.Multiplicate"/> values include:
/// Double wield halving (averaging); double wield bonus multiplier (55%); excellent 2% increase option (pendant and other weapons).
/// </remarks>
public static AttributeDefinition PhysicalBaseDmgIncrease { get; } = new(new Guid("104B4DAA-C507-4CBB-AF38-D53DDBB4817E"), "Physical Base Damage Increase", string.Empty);

/// <summary>
Expand Down Expand Up @@ -1130,14 +1143,41 @@ public class Stats
public static AttributeDefinition DoubleDamageChance { get; } = new(new Guid("2B8A03E6-1CC2-48A0-8633-3F36E17050F4"), "Double Damage Chance", string.Empty);

/// <summary>
/// Gets the stun chance attribute definition.
/// Gets the MST stun chance attribute definition.
/// </summary>
/// <remarks>Bucket attribute for the master skills: wind tome (book of neil) mastery, fire burst mastery and earthshake mastery.</remarks>
public static AttributeDefinition MasteryStunChance { get; } = new(new Guid("610D3259-1158-424A-8738-9EB7A71DE600"), "Mastery Stun Chance (MST)", "A generic master tree stun chance attribute, which serves as a bucket for \"mastery\" skills.");

/// <summary>
/// Gets the MST mace mastery stun chance attribute definition.
/// </summary>
public static AttributeDefinition MaceMasteryStunChance { get; } = new(new Guid("6E3A9F2D-5B7C-4D8E-A1F3-2C9E5B7D4F6A"), "Mace Mastery Stun Chance (MST)", string.Empty);

/// <summary>
/// Gets the MST target move chance attribute definition.
/// </summary>
/// <remarks>Bucket attribute for the master skills: lightning tome (book of lagle) mastery and twisting slash mastery.</remarks>
public static AttributeDefinition MasteryMoveTargetChance { get; } = new(new Guid("6F9619FF-8B86-D011-B42D-00C04FC964FF"), "Mastery Move Target Chance (MST)", "A generic master tree move target chance attribute, which serves as a bucket for \"mastery\" skills.");

/// <summary>
/// Gets the rageful blow mastery decrease durability MST chance attribute definition.
/// </summary>
public static AttributeDefinition RagefulBlowMasteryDurabilityDecChance { get; } = new(new Guid("2F8A5D3B-9C7E-4F1A-B6D2-8E3C5A7F9B1D"), "Rageful Blow Mastery Durability Decrease Chance (MST)", string.Empty);

/// <summary>
/// Gets the spear mastery double damage chance attribute definition.
/// </summary>
public static AttributeDefinition SpearMasteryDoubleDamageChance { get; } = new(new Guid("5D9E2A7B-3C4F-8E1A-B5D6-2E7F9C4A1B8D"), "Spear Mastery Double Damage Chance (MST)", string.Empty);

/// <summary>
/// Gets the swell life skill health increase attribute definition.
/// </summary>
public static AttributeDefinition StunChance { get; } = new(new Guid("610D3259-1158-424A-8738-9EB7A71DE600"), "Stun Chance", string.Empty);
public static AttributeDefinition SwellLifeHealthIncrease { get; } = new(new Guid("9C4E7B2A-F1D6-4A3E-B8C5-1D7F2E9A3B4C"), "Swell Life Health Increase", string.Empty);

/// <summary>
/// Gets the pollution skill MST target move chance, which rises with lightning tome mastery.
/// Gets the swell life skill mana increase attribute definition.
/// </summary>
public static AttributeDefinition PollutionMoveTargetChance { get; } = new(new Guid("6F9619FF-8B86-D011-B42D-00C04FC964FF"), "Pollution Move Target Chance (MST)", "The pollution skill (book of lagle) move chance, which rises with lightning tome mastery.");
public static AttributeDefinition SwellLifeManaIncrease { get; } = new(new Guid("8B4F1C6D-9A2E-4F7B-A3D5-1E9C7F2A4B6D"), "Swell Life Mana Increase", string.Empty);

/// <summary>
/// Gets the mana after monster kill attribute definition.
Expand Down
12 changes: 12 additions & 0 deletions src/GameLogic/ItemExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,18 @@ public static bool IsJewelry(this Item item)
return item.ItemSlot >= InventoryConstants.PendantSlot && item.ItemSlot <= InventoryConstants.Ring2Slot;
}

/// <summary>
/// Determines whether this item is an armor item.
/// </summary>
/// <param name="item">The item.</param>
/// <returns>
/// <c>true</c> if the specified item is armor; otherwise, <c>false</c>.
/// </returns>
public static bool IsArmorItem(this Item item)
{
return item.ItemSlot >= InventoryConstants.HelmSlot && item.ItemSlot <= InventoryConstants.BootsSlot;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/// <summary>
/// Determines whether this instance is a is weapon which deals physical damage.
/// </summary>
Expand Down
30 changes: 29 additions & 1 deletion src/GameLogic/ItemPowerUpFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,28 @@ private IEnumerable<PowerUpWrapper> GetPowerUpsOfItemOptions(Item item, Attribut
continue;
}

foreach (var wrapper in PowerUpWrapper.CreateByPowerUpDefinition(powerUp, attributeHolder))
AggregateType? aggregateType = null;
if (option.OptionType == ItemOptionTypes.Excellent)
{
if (item.ItemSlot == InventoryConstants.PendantSlot
&& option.PowerUpDefinition?.TargetAttribute == Stats.PhysicalBaseDmg)
{
// Pendant options are not subject to double wield averaging
aggregateType = AggregateType.AddFinal;
}
else if (option.PowerUpDefinition?.TargetAttribute == Stats.PhysicalBaseDmgIncrease
&& item.Definition!.BasePowerUpAttributes.Any(pu => pu.TargetAttribute == Stats.DoubleWieldWeaponCount))
{
// This needs special treatment, since this option is averaged when double wielding
aggregateType = AggregateType.AddRaw;
}
else
{
// the normal aggregate type should be used
}
Comment on lines +277 to +294
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent weapon options should be averaged, as they only apply to their "side" in the sources, whereas the pendant excellent options add to both sides: zTeamS6.3, emu.
I used this aggregateType override to get that right 😃

}

foreach (var wrapper in PowerUpWrapper.CreateByPowerUpDefinition(powerUp, attributeHolder, aggregateType))
{
yield return wrapper;
}
Expand Down Expand Up @@ -344,6 +365,13 @@ private IEnumerable<PowerUpWrapper> CreateExcellentAndAncientBasePowerUpWrappers
yield return new PowerUpWrapper(new SimpleElement(ancientBonus, AggregateType.AddRaw), minDmgAttribute, attributeHolder);
yield return new PowerUpWrapper(new SimpleElement(ancientBonus, AggregateType.AddRaw), maxDmgAttribute, attributeHolder);
}

if (itemIsExcellent
&& item.ItemOptions.Any(io => io.ItemOption?.PowerUpDefinition?.TargetAttribute == Stats.PhysicalBaseDmgIncrease)
&& item.Definition!.BasePowerUpAttributes.Any(pu => pu.TargetAttribute == Stats.DoubleWieldWeaponCount))
{
yield return new PowerUpWrapper(new SimpleElement(-1, AggregateType.AddRaw), Stats.PhysicalBaseDmgIncrease, attributeHolder);
}
}

if (item.IsWizardryWeapon(out var staffRise) && item.ItemSlot == InventoryConstants.LeftHandSlot)
Expand Down
14 changes: 7 additions & 7 deletions src/GameLogic/MapInitializer.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="MapInitializer.cs" company="MUnique">
// <copyright file="MapInitializer.cs" company="MUnique">
// Licensed under the MIT License. See LICENSE file in the project root for full license information.
// </copyright>

Expand Down Expand Up @@ -88,7 +88,7 @@ public async ValueTask InitializeStateAsync(GameMap createdMap)
_ = this.PlugInManager ?? throw new InvalidOperationException("PlugInManager must be set first");
_ = this.PathFinderPool ?? throw new InvalidOperationException("PathFinderPool must be set first");

this._logger.LogDebug("Start creating monster instances for map {createdMap}", createdMap);
this._logger.LogDebug("Start creating monster instances for map {createdMap}", createdMap.Definition.Name);
var automaticSpawns = createdMap.Definition.MonsterSpawns
.Where(m => m.MonsterDefinition is not null)
.Where(m => m.SpawnTrigger is SpawnTrigger.Automatic);
Expand Down Expand Up @@ -117,7 +117,7 @@ public async ValueTask InitializeStateAsync(GameMap createdMap)
this._spawnedMonsters.AddOrUpdate(spawnArea, spawnArea.Quantity, (_, _) => spawnArea.Quantity);
});

this._logger.LogDebug("Finished creating monster instances for map {createdMap}", createdMap);
this._logger.LogDebug("Finished creating monster instances for map {createdMap}", createdMap.Definition.Name);
}

/// <summary>
Expand All @@ -130,7 +130,7 @@ public async ValueTask InitializeNpcsOnEventStartAsync(GameMap createdMap, IEven
_ = this.PlugInManager ?? throw new InvalidOperationException("PlugInManager must be set first");
_ = this.PathFinderPool ?? throw new InvalidOperationException("PathFinderPool must be set first");

this._logger.LogDebug("Start creating event monster instances for map {createdMap}", createdMap);
this._logger.LogDebug("Start creating event monster instances for map {createdMap}", createdMap.Definition.Name);
var eventSpawns = createdMap.Definition.MonsterSpawns
.Where(m => m.MonsterDefinition is not null)
.Where(m => m.SpawnTrigger is SpawnTrigger.OnceAtEventStart or SpawnTrigger.AutomaticDuringEvent);
Expand All @@ -143,7 +143,7 @@ public async ValueTask InitializeNpcsOnEventStartAsync(GameMap createdMap, IEven
}
}

this._logger.LogDebug("Finished creating event monster instances for map {createdMap}", createdMap);
this._logger.LogDebug("Finished creating event monster instances for map {createdMap}", createdMap.Definition.Name);
}

/// <inheritdoc />
Expand All @@ -152,7 +152,7 @@ public async ValueTask InitializeNpcsOnWaveStartAsync(GameMap createdMap, IEvent
_ = this.PlugInManager ?? throw new InvalidOperationException("PlugInManager must be set first");
_ = this.PathFinderPool ?? throw new InvalidOperationException("PathFinderPool must be set first");

this._logger.LogDebug("Start creating event monster instances for map {createdMap}", createdMap);
this._logger.LogDebug("Start creating event monster instances for map {createdMap}", createdMap.Definition.Name);
var waveSpawns = createdMap.Definition.MonsterSpawns
.Where(m => m.MonsterDefinition is not null)
.Where(m => m.SpawnTrigger is SpawnTrigger.AutomaticDuringWave or SpawnTrigger.OnceAtWaveStart)
Expand All @@ -166,7 +166,7 @@ public async ValueTask InitializeNpcsOnWaveStartAsync(GameMap createdMap, IEvent
}
}

this._logger.LogDebug("Finished creating event monster instances for map {createdMap}", createdMap);
this._logger.LogDebug("Finished creating event monster instances for map {createdMap}", createdMap.Definition.Name);
}

/// <inheritdoc />
Expand Down
Loading