Skip to content
Open
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 ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's just theoretical, but it's used just in one special code path. So, just retrieve it locally.


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
35 changes: 31 additions & 4 deletions src/GameLogic/Attributes/Stats.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1130,14 +1130,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>
public static AttributeDefinition StunChance { get; } = new(new Guid("610D3259-1158-424A-8738-9EB7A71DE600"), "Stun Chance", string.Empty);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why was StunChance removed / replaced by MasterStunChance? There is a castle siege skill to stun opponents, too.
How about renaming MasterStunChance back to StunChance ?

Copy link
Copy Markdown
Contributor Author

@ze-dom ze-dom May 14, 2026

Choose a reason for hiding this comment

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

Yeah. I checked the CS "Stun" skill and it has 100% chance, so it will not use this Stat: zTeamS6.3, emu.
So, the only skills that use this Stat are actually "mastery" skills.

/// <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)", string.Empty);

/// <summary>
/// Gets the pollution skill MST target move chance, which rises with lightning tome mastery.
/// Gets the MST mace mastery stun chance 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 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 SwellLifeHealthIncrease { get; } = new(new Guid("9C4E7B2A-F1D6-4A3E-B8C5-1D7F2E9A3B4C"), "Swell Life Health Increase", string.Empty);

/// <summary>
/// Gets the swell life skill mana increase attribute definition.
/// </summary>
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
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
5 changes: 5 additions & 0 deletions src/GameLogic/NPC/AttackableNpcBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ public int Health
if (attacker is Player player)
{
await player.AfterHitTargetAsync().ConfigureAwait(false);

if (player.Attributes?[Stats.IsMaceEquipped] > 0 && Rand.NextRandomBool(player.Attributes[Stats.MaceMasteryStunChance]))
{
await player.ApplyMaceMasteryStunEffectAsync(this).ConfigureAwait(false);
}
Comment on lines +130 to +134
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this code be replaced with a conditional attribute relationship?
Something like this in the character class initializer:
result.AttributeCombinations.Add(this.CreateConditionalRelationship(Stats.MaceMasteryStunChance, Stats.IsMaceEquipped, Stats.StunChance));

}

if (attacker as IPlayerSurrogate is { } playerSurrogate)
Expand Down
30 changes: 24 additions & 6 deletions src/GameLogic/Player.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// <copyright file="Player.cs" company="MUnique">
// <copyright file="Player.cs" company="MUnique">
// Licensed under the MIT License. See LICENSE file in the project root for full license information.
// </copyright>

Expand Down Expand Up @@ -756,7 +756,7 @@ public ValueTask ShowBlueMessageAsync(string message)
}

await this.HitAsync(hitInfo, attacker, skill?.Skill, isFinalStreakHit).ConfigureAwait(false);
await this.DecreaseItemDurabilityAfterHitAsync(hitInfo).ConfigureAwait(false);
await this.DecreaseItemDurabilityAfterHitAsync(hitInfo, attacker.Attributes[Stats.RagefulBlowMasteryDurabilityDecChance]).ConfigureAwait(false);
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

Directly accessing attacker.Attributes is unsafe here. If the attacker is an entity without initialized attributes (which can happen with certain NPCs or in specific edge cases), this will throw a NullReferenceException. It's safer to use a null-conditional access with a default value.

        await this.DecreaseItemDurabilityAfterHitAsync(hitInfo, attacker.Attributes?[Stats.RagefulBlowMasteryDurabilityDecChance] ?? 0).ConfigureAwait(false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this now applying the rageful blow mastery value to all kind of skills?

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.

🤦 😅


if (attacker as IPlayerSurrogate is { } playerSurrogate)
{
Expand All @@ -766,6 +766,11 @@ public ValueTask ShowBlueMessageAsync(string message)
if (attacker is Player attackerPlayer)
{
await attackerPlayer.AfterHitTargetAsync().ConfigureAwait(false);

if (attackerPlayer.Attributes?[Stats.IsMaceEquipped] > 0 && Rand.NextRandomBool(attackerPlayer.Attributes[Stats.MaceMasteryStunChance]))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we just use Stats.StunChance ? When it's a conditional relationship, the if doesn't have to check if a mace is equipped.

{
await attackerPlayer.ApplyMaceMasteryStunEffectAsync(this).ConfigureAwait(false);
}
}

return hitInfo;
Expand Down Expand Up @@ -2659,12 +2664,25 @@ private async void OnAmmunitionAmountChanged(object? sender, EventArgs args)
}
}

private async ValueTask DecreaseItemDurabilityAfterHitAsync(HitInfo hitInfo)
private async ValueTask DecreaseItemDurabilityAfterHitAsync(HitInfo hitInfo, float ragefulBlowMasteryDurabilityDecChance = 0)
{
var randomArmorItem = this.Inventory?.EquippedItems.Where(ItemExtensions.IsDefensiveItem).SelectRandom();
if (randomArmorItem is { })
var randomDefensiveItem = this.Inventory?.EquippedItems.Where(ItemExtensions.IsDefensiveItem).SelectRandom();
if (randomDefensiveItem is { })
{
await this.DecreaseDefenseItemDurabilityAsync(randomDefensiveItem, hitInfo).ConfigureAwait(false);
}

if (Rand.NextRandomBool(ragefulBlowMasteryDurabilityDecChance))
{
await this.DecreaseDefenseItemDurabilityAsync(randomArmorItem, hitInfo).ConfigureAwait(false);
var randomArmorItem = this.Inventory?.EquippedItems.Where(ItemExtensions.IsArmorItem).SelectRandom();
if (randomArmorItem is { })
{
var durabilityFactor = 1.1 - this.Attributes![Stats.ItemDurationIncrease]; // 10% decrease
if (durabilityFactor > 0 && randomArmorItem.DecreaseDurability(randomArmorItem.GetMaximumDurabilityOfOnePiece() * durabilityFactor))
{
await this.InvokeViewPlugInAsync<IItemDurabilityChangedPlugIn>(p => p.ItemDurabilityChangedAsync(randomArmorItem, false)).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.

zTeamS6.3, emu
The sources seem to be bugged here, because they use the attacker's instead of the defender's durability reduction master skill to attenuate the effect.

}

if (this.Inventory?.GetItem(InventoryConstants.PetSlot) is { Durability: > 0.0 } pet)
Expand Down
11 changes: 8 additions & 3 deletions src/GameLogic/PlayerActions/Skills/AreaSkillAttackAction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,16 @@ private async ValueTask ApplySkillAsync(Player player, SkillEntry skillEntry, IA
await target.AttackByAsync(player, skillEntry, isCombo, 1, hit == skill.NumberOfHitsPerAttack).ConfigureAwait(false);
}

var baseSkill = skillEntry.GetBaseSkill();

if (player.GameContext.PlugInManager.GetStrategy<short, IAreaSkillPlugIn>(baseSkill.Number) is { } strategy)
if (player.GameContext.PlugInManager.GetStrategy<short, IAreaSkillPlugIn>(skillEntry.Skill.Number) is { } strategy)
{
await strategy.AfterTargetGotAttackedAsync(player, target, skillEntry, targetAreaCenter, hitInfo).ConfigureAwait(false);
return;
}

var baseSkill = skillEntry.GetBaseSkill();
if (player.GameContext.PlugInManager.GetStrategy<short, IAreaSkillPlugIn>(baseSkill.Number) is { } baseSkillStrategy)
{
await baseSkillStrategy.AfterTargetGotAttackedAsync(player, target, skillEntry, targetAreaCenter, hitInfo).ConfigureAwait(false);
}
Comment thread
sven-n marked this conversation as resolved.
}
}
2 changes: 0 additions & 2 deletions src/GameLogic/PlayerActions/Skills/EarthShakeSkillPlugIn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public async ValueTask AfterTargetGotAttackedAsync(IAttacker attacker, IAttackab
direction = (Direction)Rand.NextInt(1, 9);
}

var currentDistance = startingPoint.EuclideanDistanceTo(currentTarget);
for (int i = 0; i < 3; i++)
{
var nextTarget = currentTarget.CalculateTargetPoint(direction);
Expand All @@ -51,7 +50,6 @@ public async ValueTask AfterTargetGotAttackedAsync(IAttacker attacker, IAttackab
}

currentTarget = nextTarget;
currentDistance = startingPoint.EuclideanDistanceTo(currentTarget);
}

await movableTarget.MoveAsync(currentTarget).ConfigureAwait(false);
Expand Down
2 changes: 1 addition & 1 deletion src/GameLogic/PlayerActions/Skills/PollutionSkillPlugIn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public async ValueTask AfterTargetGotAttackedAsync(IAttacker attacker, IAttackab
if (!target.IsAlive
|| target is not IMovable movableTarget
|| target.CurrentMap is not { } currentMap
|| !Rand.NextRandomBool(Convert.ToDouble(attacker.Attributes[Stats.PollutionMoveTargetChance])))
|| !Rand.NextRandomBool(attacker.Attributes[Stats.MasteryMoveTargetChance]))
{
return;
}
Expand Down
6 changes: 3 additions & 3 deletions src/GameLogic/PlayerActions/Skills/RequiemSkillPlugIn.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ public async ValueTask AfterTargetGotAttackedAsync(IAttacker attacker, IAttackab
{
this._stunEffectDefinition ??= ((Player)attacker).GameContext.Configuration.MagicEffects.First(m => m.Number == StunnedMagicEffectNumber);

if (!target.IsAlive || !Rand.NextRandomBool(Convert.ToDouble(attacker.Attributes[Stats.StunChance])))
if (!target.IsAlive || !Rand.NextRandomBool(attacker.Attributes[Stats.MasteryStunChance]))
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

Unsafe access to attacker.Attributes. If the attacker is not a player or has no attributes, this will throw a NullReferenceException. Use null-conditional access.

        if (!target.IsAlive || !Rand.NextRandomBool(attacker.Attributes?[Stats.MasteryStunChance] ?? 0))

{
return;
}

var powerUp = attacker.Attributes.CreateElement(this._stunEffectDefinition.PowerUpDefinitions.First());
var magicEffect = new MagicEffect(powerUp, this._stunEffectDefinition, TimeSpan.FromSeconds(3));
var powerUp = attacker.Attributes.CreateElement(this._stunEffectDefinition.PowerUpDefinitions.First(pu => pu.TargetAttribute == Stats.IsStunned));
var magicEffect = new MagicEffect(TimeSpan.FromSeconds(3), this._stunEffectDefinition, [new MagicEffect.ElementWithTarget(powerUp, Stats.IsStunned)]);
await target.MagicEffectList.AddEffectAsync(magicEffect).ConfigureAwait(false);

if (target is ISupportWalk walkSupporter && walkSupporter.IsWalking)
Expand Down
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.

Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// <copyright file="TwistingSlashMasterySkillPlugIn.cs" company="MUnique">
// Licensed under the MIT License. See LICENSE file in the project root for full license information.
// </copyright>

namespace MUnique.OpenMU.GameLogic.PlayerActions.Skills;

using System.Runtime.InteropServices;
using MUnique.OpenMU.GameLogic.Attributes;
using MUnique.OpenMU.GameLogic.NPC;
using MUnique.OpenMU.GameLogic.PlugIns;
using MUnique.OpenMU.Pathfinding;
using MUnique.OpenMU.PlugIns;

/// <summary>
/// Handles the twisting slash mastery skill of the dark knight class. Based on a chance, it may push the targets 2 squares away from the attacker.
/// </summary>
[PlugIn]
[Display(Name = nameof(PlugInResources.TwistingSlashMasterySkillPlugIn_Name), Description = nameof(PlugInResources.TwistingSlashMasterySkillPlugIn_Description), ResourceType = typeof(PlugInResources))]
[Guid("9F4B2C1D-E7A6-4B3C-8D9E-0FAB12C3D4E5")]
public class TwistingSlashMasterySkillPlugIn : IAreaSkillPlugIn
{
/// <inheritdoc />
public short Key => 332;

/// <inheritdoc />
public async ValueTask AfterTargetGotAttackedAsync(IAttacker attacker, IAttackable target, SkillEntry skillEntry, Point targetAreaCenter, HitInfo? hitInfo)
{
if (!target.IsAlive
|| target is not IMovable movableTarget
|| target.CurrentMap is not { } currentMap
|| !Rand.NextRandomBool(attacker.Attributes[Stats.MasteryMoveTargetChance]))
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

Unsafe access to attacker.Attributes. If the attacker is not a player or has no attributes, this will throw a NullReferenceException. Use null-conditional access.

            || !Rand.NextRandomBool(attacker.Attributes?[Stats.MasteryMoveTargetChance] ?? 0))

{
return;
}

var startingPoint = attacker.Position;
var currentTarget = target.Position;
var direction = startingPoint.GetDirectionTo(currentTarget);
if (direction == Direction.Undefined)
{
direction = (Direction)Rand.NextInt(1, 9);
}

for (int i = 0; i < 2; i++)
{
var nextTarget = currentTarget.CalculateTargetPoint(direction);
if (!currentMap.Terrain.WalkMap[nextTarget.X, nextTarget.Y]
|| (target is NonPlayerCharacter && target.CurrentMap.Terrain.SafezoneMap[nextTarget.X, nextTarget.Y]))
Comment on lines +47 to +48
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

Accessing WalkMap and SafezoneMap using nextTarget.X and nextTarget.Y without verifying that these coordinates are within the terrain's boundaries can lead to an IndexOutOfRangeException. This is particularly likely if a target is pushed near the edges of the map. Please add a bounds check before accessing the terrain arrays.

{
// we don't want to push the target into a non-reachable area, through walls or monsters into the safe zone.
break;
}

currentTarget = nextTarget;
}

await movableTarget.MoveAsync(currentTarget).ConfigureAwait(false);
}
}
Loading