feat(skills): SkillEvents.SkillUsed for cross-assembly subscribers; InternalsVisibleTo ModernSpawner.Tests (#2636)
## Summary
Two small additive changes that an external content assembly (ModernSpawner) needs, as separable commits.
**1. `SkillEvents.SkillUsed`** (`Projects/UOContent/Skills/SkillEvents.cs`, namespace `Server.Misc`): a plain C# event `Action<Mobile, Skill, bool success>` raised once per skill attempt from each of the four `Mobile_SkillCheck*` handlers, with the handler's own result. Attempts the handler resolves without a roll (too difficult, no challenge) raise too, so a grandmaster's trivial success and a guaranteed combat roll are observable. Not raised when the mobile lacks the skill. Each handler keeps its logic in a private core method and raises on the way out, so there is exactly one raise per attempt and `CheckSkill` itself is unchanged.
- **Why a plain event and not a `[GeneratedEvent]`:** generated events are compile-time static dispatch inside the UOContent compilation, so a subscriber in another assembly cannot use `[OnEvent]`. Shape follows `HelpEvents`.
- **Why "used", not "gained":** this is the XmlSpawner skill-trigger semantic (it wrapped the same four handlers and passed their result as `success`; its grammar was `Skill[+/-]` for success-only or failure-only). Gains are already observable through the existing skill-change notification on `Mobile`.
- **Cost:** one delegate null-check per attempt when nothing is subscribed; no boxing, no closure, no allocation. The handlers sit on the combat swing path.
- **Exception contract:** subscriber exceptions propagate, matching `EventSink`/`HelpEvents`; no try/catch by design.
**2. `InternalsVisibleTo("ModernSpawner.Tests")`** on `Server.csproj`, beside the existing `Server.Tests`/`UOContent.Tests` entries, so an external test host can seed `Core._now` the way the engine's own test initializers do. Separable; a public test seam on `Core` would serve the same need without naming a downstream assembly.
## Open question
The payload is the `Skill` object plus a positional `bool`. A `readonly struct` args type passed `in` would leave room to add `chance` or the target later without breaking subscribers. Happy to change before merge.
## Test plan
- [x] `UOContent.Tests`: 4 tests — a rolled attempt raises once with the returned outcome; each short-circuit path (no challenge, too difficult, on both the direct and value-window handlers) raises with the handler's result; a direct `CheckSkill` call does not raise; no subscriber does not throw. Full suite green.
- [x] `Server` and `UOContent` build clean with `TreatWarningsAsErrors`.
- [ ] CI
This commit is contained in:
parent
7deb8b082e
commit
309fcfeb27
5 changed files with 189 additions and 0 deletions
|
|
@ -50,5 +50,8 @@
|
|||
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">
|
||||
<_Parameter1>UOContent.Tests</_Parameter1>
|
||||
</AssemblyAttribute>
|
||||
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">
|
||||
<_Parameter1>ModernSpawner.Tests</_Parameter1>
|
||||
</AssemblyAttribute>
|
||||
</ItemGroup>
|
||||
</Project>
|
||||
|
|
|
|||
124
Projects/UOContent.Tests/Tests/Skills/SkillEventsTests.cs
Normal file
124
Projects/UOContent.Tests/Tests/Skills/SkillEventsTests.cs
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
using Server;
|
||||
using Server.Misc;
|
||||
using Xunit;
|
||||
|
||||
namespace UOContent.Tests;
|
||||
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class SkillEventsTests
|
||||
{
|
||||
private sealed class Recorder
|
||||
{
|
||||
public Mobile From;
|
||||
public Skill Skill;
|
||||
public bool Success;
|
||||
public int Calls;
|
||||
|
||||
public void Handle(Mobile from, Skill skill, bool success)
|
||||
{
|
||||
From = from;
|
||||
Skill = skill;
|
||||
Success = success;
|
||||
Calls++;
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void DirectTarget_RolledAttempt_RaisesOnceWithTheReturnedOutcome()
|
||||
{
|
||||
var from = new Mobile();
|
||||
var skill = from.Skills[SkillName.Mining];
|
||||
var recorder = new Recorder();
|
||||
|
||||
SkillEvents.SkillUsed += recorder.Handle;
|
||||
try
|
||||
{
|
||||
var rolled = SkillCheck.Mobile_SkillCheckDirectTarget(from, SkillName.Mining, null, 0.5);
|
||||
Assert.Equal(1, recorder.Calls);
|
||||
Assert.Same(from, recorder.From);
|
||||
Assert.Same(skill, recorder.Skill);
|
||||
Assert.Equal(rolled, recorder.Success);
|
||||
|
||||
Assert.False(SkillCheck.Mobile_SkillCheckDirectTarget(from, SkillName.Mining, null, 0.0));
|
||||
Assert.Equal(2, recorder.Calls);
|
||||
Assert.False(recorder.Success);
|
||||
}
|
||||
finally
|
||||
{
|
||||
SkillEvents.SkillUsed -= recorder.Handle;
|
||||
from.Delete();
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ShortCircuits_StillRaise_WithTheHandlerOutcome()
|
||||
{
|
||||
var from = new Mobile();
|
||||
var recorder = new Recorder();
|
||||
|
||||
SkillEvents.SkillUsed += recorder.Handle;
|
||||
try
|
||||
{
|
||||
Assert.True(SkillCheck.Mobile_SkillCheckDirectLocation(from, SkillName.Mining, 1.0));
|
||||
Assert.Equal(1, recorder.Calls);
|
||||
Assert.True(recorder.Success);
|
||||
|
||||
Assert.False(SkillCheck.Mobile_SkillCheckDirectTarget(from, SkillName.Mining, null, -0.1));
|
||||
Assert.Equal(2, recorder.Calls);
|
||||
Assert.False(recorder.Success);
|
||||
|
||||
Assert.False(SkillCheck.Mobile_SkillCheckLocation(from, SkillName.Mining, 50.0, 100.0));
|
||||
Assert.Equal(3, recorder.Calls);
|
||||
Assert.False(recorder.Success);
|
||||
|
||||
Assert.True(SkillCheck.Mobile_SkillCheckTarget(from, SkillName.Mining, null, 0.0, 0.0));
|
||||
Assert.Equal(4, recorder.Calls);
|
||||
Assert.True(recorder.Success);
|
||||
}
|
||||
finally
|
||||
{
|
||||
SkillEvents.SkillUsed -= recorder.Handle;
|
||||
from.Delete();
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CheckSkill_Direct_DoesNotRaise()
|
||||
{
|
||||
var from = new Mobile();
|
||||
var skill = from.Skills[SkillName.Mining];
|
||||
var recorder = new Recorder();
|
||||
|
||||
SkillEvents.SkillUsed += recorder.Handle;
|
||||
try
|
||||
{
|
||||
SkillCheck.CheckSkill(from, skill, null, 1.0);
|
||||
Assert.Equal(0, recorder.Calls);
|
||||
}
|
||||
finally
|
||||
{
|
||||
SkillEvents.SkillUsed -= recorder.Handle;
|
||||
from.Delete();
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void NoSubscriber_DoesNotThrow()
|
||||
{
|
||||
var from = new Mobile();
|
||||
var recorder = new Recorder();
|
||||
|
||||
try
|
||||
{
|
||||
SkillEvents.SkillUsed += recorder.Handle;
|
||||
SkillEvents.SkillUsed -= recorder.Handle;
|
||||
|
||||
Assert.True(SkillCheck.Mobile_SkillCheckDirectLocation(from, SkillName.Mining, 1.0));
|
||||
Assert.Equal(0, recorder.Calls);
|
||||
}
|
||||
finally
|
||||
{
|
||||
from.Delete();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -48,6 +48,13 @@ public static class SkillCheck
|
|||
return false;
|
||||
}
|
||||
|
||||
var success = CheckLocation(from, skill, minSkill, maxSkill);
|
||||
SkillEvents.InvokeSkillUsed(from, skill, success);
|
||||
return success;
|
||||
}
|
||||
|
||||
private static bool CheckLocation(Mobile from, Skill skill, double minSkill, double maxSkill)
|
||||
{
|
||||
var value = skill.Value;
|
||||
|
||||
if (value < minSkill)
|
||||
|
|
@ -76,6 +83,13 @@ public static class SkillCheck
|
|||
return false;
|
||||
}
|
||||
|
||||
var success = CheckDirectLocation(from, skill, chance);
|
||||
SkillEvents.InvokeSkillUsed(from, skill, success);
|
||||
return success;
|
||||
}
|
||||
|
||||
private static bool CheckDirectLocation(Mobile from, Skill skill, double chance)
|
||||
{
|
||||
if (chance < 0.0)
|
||||
{
|
||||
return false; // Too difficult
|
||||
|
|
@ -156,6 +170,13 @@ public static class SkillCheck
|
|||
return false;
|
||||
}
|
||||
|
||||
var success = CheckTarget(from, skill, target, minSkill, maxSkill);
|
||||
SkillEvents.InvokeSkillUsed(from, skill, success);
|
||||
return success;
|
||||
}
|
||||
|
||||
private static bool CheckTarget(Mobile from, Skill skill, object target, double minSkill, double maxSkill)
|
||||
{
|
||||
var value = skill.Value;
|
||||
|
||||
if (value < minSkill)
|
||||
|
|
@ -182,6 +203,13 @@ public static class SkillCheck
|
|||
return false;
|
||||
}
|
||||
|
||||
var success = CheckDirectTarget(from, skill, target, chance);
|
||||
SkillEvents.InvokeSkillUsed(from, skill, success);
|
||||
return success;
|
||||
}
|
||||
|
||||
private static bool CheckDirectTarget(Mobile from, Skill skill, object target, double chance)
|
||||
{
|
||||
if (chance < 0.0)
|
||||
{
|
||||
return false; // Too difficult
|
||||
|
|
|
|||
23
Projects/UOContent/Skills/SkillEvents.cs
Normal file
23
Projects/UOContent/Skills/SkillEvents.cs
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
using System;
|
||||
using System.Runtime.CompilerServices;
|
||||
|
||||
namespace Server.Misc;
|
||||
|
||||
/// <summary>
|
||||
/// Skill system events. Plain C# events so other assemblies can subscribe; generated events cannot be
|
||||
/// subscribed across assemblies.
|
||||
/// </summary>
|
||||
public static class SkillEvents
|
||||
{
|
||||
/// <summary>
|
||||
/// Raised once per skill attempt from the four <c>Mobile_SkillCheck*</c> handlers with the attempt's
|
||||
/// outcome, including attempts the handler resolves without a roll (too difficult, no challenge).
|
||||
/// Not raised when the mobile lacks the skill. Fires for every <see cref="Mobile" />, including
|
||||
/// creatures. Subscribers must not block or allocate.
|
||||
/// </summary>
|
||||
public static event Action<Mobile, Skill, bool> SkillUsed;
|
||||
|
||||
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
||||
public static void InvokeSkillUsed(Mobile from, Skill skill, bool success) =>
|
||||
SkillUsed?.Invoke(from, skill, success);
|
||||
}
|
||||
|
|
@ -242,6 +242,17 @@ public static void HandlePlayerLogin(PlayerMobile player)
|
|||
|
||||
---
|
||||
|
||||
## Static Content Events
|
||||
|
||||
Generated events dispatch statically inside `UOContent`; content that other assemblies must observe exposes a
|
||||
plain `static event` instead (shape: `Projects/UOContent/Engines/Help/HelpEvents.cs`).
|
||||
|
||||
- `SkillEvents.SkillUsed` -- `Action<Mobile, Skill, bool success>`, raised once per skill attempt from the
|
||||
four `Mobile_SkillCheck*` handlers with the attempt's outcome, including attempts resolved without a roll
|
||||
(too difficult, no challenge). Not raised when the mobile lacks the skill. Fires for every `Mobile`.
|
||||
|
||||
---
|
||||
|
||||
## Event Args Pooling Pattern
|
||||
|
||||
Some EventArgs use object pooling to avoid allocation in hot paths:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue