warrentyexpired/ModernUO
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(ninjitsu): gate Animal Form gump by skill and stop mana drain (#2452) (#2468)

Browse source 
## Issue

Fixes #2452. A player with 30 Ninjitsu reported that the Animal Form menu showed **every** form; selecting one above their skill (e.g. Dog, req 40) **consumed mana** and returned "you need at least 40 skill", and afterwards the **gump never reopened** — every recast silently re-attempted the unusable form and drained more mana.

## Root cause

Three linked bugs, all reproduced from the code:

1. **Gump not gated by skill.** `AnimalFormGump.BuildLayout` compared `Skill.Fixed` (which is `Value * 10`, so 30 skill → `300`) against the raw 0–100 `ReqSkill` (Dog = `40`). `300 >= 40` is always true, so all forms were shown. `Morph` itself correctly uses `.Value`.
2. **Mana charged on a no-skill cast.** `Morph` returns `MorphResult.NoSkill` for an under-skilled form, but both call sites (`OnCast`, `OnResponse`) only special-cased `MorphResult.Fail`; `NoSkill` fell through to the branch that deducts mana.
3. **Menu never reopened.** Per OSI ([uo.com](https://uo.com/wiki/ultima-online-wiki/skills/ninjitsu/), [uoguide](https://www.uoguide.com/Animal_Form)), casting while **standing still always opens the selection menu**, and casting while **moving** quick-transforms into the last selected form. ModernUO only opened the menu when `lastAnimalForm == -1`, so once any form was selected a stationary recast skipped the menu.

## Fix

- Add `AnimalForm.CanSelectEntry` (compares `Skill.Value` to `ReqSkill`, plus the talisman check) and use it for the gump's per-entry enable check.
- `OnCast`: standing still always opens the menu; moving quick-transforms into the last form. `NoSkill` no longer costs mana.
- `OnResponse`: handle `Success` / `Fail` / `NoSkill` explicitly so `NoSkill` costs no mana.

## Tests

Adds `AnimalFormTests`:
- `CanSelectEntry` rejects forms above skill, accepts forms at/below skill, and requires a talisman for talisman-gated forms.
- `Morph` returns `NoSkill` (without transforming) when under-skilled, and `Success` when sufficiently skilled.

Verified the gating test catches the regression (reintroducing `.Fixed` fails it). Full solution build is clean; the 5 new tests plus 284 other UOContent tests pass (the pathfinding/AI sequential tests were excluded only because they deadlock under concurrent local runs — they are unrelated to this change).
This commit is contained in:
Kamron Batman 2026-06-06 15:56:31 -07:00 • committed by GitHub
parent 122e20c954
commit 8e5e4f72c8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 120 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

52
Projects/UOContent/Spells/Ninjitsu/AnimalForm.cs
View file

@ -140,22 +140,25 @@ public class AnimalForm : NinjaSpell
RemoveContext(Caster, context);
Caster.Mana -= mana;
}
// On OSI, casting while standing still always opens the selection menu; casting while
// moving quick-transforms into the last selected form without the menu.
else if (Caster is PlayerMobile && !_wasMoving && !CasterIsMoving())
{
Caster.SendGump(new AnimalFormGump(Caster, Entries, this));
}
else
{
var lastAnimalForm = GetLastAnimalForm(Caster);
if (Caster is PlayerMobile && lastAnimalForm == -1 && !_wasMoving && !CasterIsMoving())
{
Caster.SendGump(new AnimalFormGump(Caster, Entries, this));
}
else if (Morph(Caster, lastAnimalForm) == MorphResult.Fail)
{
DoFizzle();
}
else
var result = Morph(Caster, GetLastAnimalForm(Caster));
if (result == MorphResult.Success)
{
Caster.FixedParticles(0x3728, 10, 13, 2023, EffectLayer.Waist);
Caster.Mana -= mana;
}
else if (result == MorphResult.Fail)
{
DoFizzle();
}
// MorphResult.NoSkill: Morph already messaged the requirement; consume no mana.
}
}
@ -164,6 +167,11 @@ public class AnimalForm : NinjaSpell
public static int GetLastAnimalForm(Mobile m) => _lastAnimalForms.GetValueOrDefault(m, -1);
// Whether the caster currently meets the requirements (Ninjitsu skill + talisman) to select this form.
// Note: ReqSkill is on the 0-100 scale, so this must compare against Skill.Value, not Skill.Fixed (value x10).
public static bool CanSelectEntry(Mobile m, AnimalFormEntry entry) =>
m.Skills.Ninjitsu.Value >= entry.ReqSkill && BaseFormTalisman.EntryEnabled(m, entry.Type);
public static MorphResult Morph(Mobile m, int entryID)
{
if (entryID < 0 || entryID >= Entries.Length)
@ -386,12 +394,11 @@ public class AnimalForm : NinjaSpell
builder.AddButton(10, 374, 0xFB1, 0xFB2, 0);
builder.AddHtmlLocalized(45, 376, 450, 20, 1011012, 0x7FFF); // CANCEL
var ninjitsu = _caster.Skills[SkillName.Ninjitsu].Fixed;
var current = 0;
for (var i = 0; i < _entries.Length; ++i)
{
var enabled = ninjitsu >= _entries[i].ReqSkill && BaseFormTalisman.EntryEnabled(_caster, _entries[i].Type);
var enabled = CanSelectEntry(_caster, _entries[i]);
var page = current / 10 + 1;
var pos = current % 10;
@ -461,16 +468,21 @@ public class AnimalForm : NinjaSpell
{
_caster.SendLocalizedMessage(1063108); // You cannot use this ability right now.
}
else if (Morph(_caster, entryID) == MorphResult.Fail)
{
_caster.LocalOverheadMessage(MessageType.Regular, 0x3B2, 502632); // The spell fizzles.
_caster.FixedParticles(0x3735, 1, 30, 9503, EffectLayer.Waist);
_caster.PlaySound(0x5C);
}
else
{
_caster.FixedParticles(0x3728, 10, 13, 2023, EffectLayer.Waist);
_caster.Mana -= mana;
var result = Morph(_caster, entryID);
if (result == MorphResult.Success)
{
_caster.FixedParticles(0x3728, 10, 13, 2023, EffectLayer.Waist);
_caster.Mana -= mana;
}
else if (result == MorphResult.Fail)
{
_caster.LocalOverheadMessage(MessageType.Regular, 0x3B2, 502632); // The spell fizzles.
_caster.FixedParticles(0x3735, 1, 30, 9503, EffectLayer.Waist);
_caster.PlaySound(0x5C);
}
// MorphResult.NoSkill: Morph already messaged the requirement; consume no mana.
}
}
}