nwn-dotnet / Anvil

A Neverwinter Nights Server API
https://nwn-dotnet.github.io/Anvil
MIT License
24 stars 9 forks source link

Effect.DamageIncrease only accepts int for damage amount instead of a DamageBonus type. #772

Open pyro7r34d opened 2 months ago

pyro7r34d commented 2 months ago

Effect.DamageIncrease only accepts int for damage amount instead of a DamageBonus type.

You have to cast it as an int. Effect divine = Effect.DamageIncrease((int)DamageBonus.Plus2d6, DamageType.Divine);

https://nwnlexicon.com/index.php/EffectDamageIncrease

zeroark commented 2 months ago

It looks like this is happening because the signature of the current method only supports int as a parameter.

/// <summary>
/// Creates an effect that applies a bonus to a specified damage type.
/// </summary>
/// <param name="bonus">The damage bonus to apply.</param>
/// <param name="damageType">The damage type to apply the bonus to.</param>
public static Effect DamageIncrease(int bonus, DamageType damageType = DamageType.Magical)
{
  return NWScript.EffectDamageIncrease(bonus, (int)damageType)!;
}

Would creating a separate overload work? Probably something like this:

/// <summary>
/// Creates an effect that applies a bonus to a specified damage type.
/// </summary>
/// <param name="bonus">The damage bonus to apply.</param>
/// <param name="damageType">The damage type to apply the bonus to.</param>
public static Effect DamageIncrease(DamageBonus bonus, DamageType damageType = DamageType.Magical)
{
  return NWScript.EffectDamageIncrease((int) bonus, (int)damageType)!;
}

This way we would prevent breaking up the previous method usage and ensure backwards compatibility, while adding the new feature to add a method that supports the DamageBonus enum as a parameter.

I'm new to this repo and this change seemed easy enough to add but I'm not sure if there's anything else I should consider.