Added observability events and reviewed docs and code.

Forge.Tests/Statescript/Resolvers/ConcatenateResolverTests.cs → Forge.Tests/Statescript/Resolvers/QuaternionConcatenateResolverTests.cs

@@ -7,13 +7,13 @@
 
 namespace Gamesmiths.Forge.Tests.Statescript.Resolvers;
 
 public class ConcatenateResolverTests
 {
 	[Fact]
 	[Trait("Resolver", "Concatenate")]
 	public void Concatenate_resolver_value_type_is_quaternion()
 	{
-		var resolver = new ConcatenateResolver(
+		var resolver = new QuaternionConcatenateResolver(
 			new VariantResolver(new Variant128(Quaternion.Identity), typeof(Quaternion)),
 			new VariantResolver(new Variant128(Quaternion.Identity), typeof(Quaternion)));
 
@@ -26,7 +26,7 @@
 	{
 		var left = Quaternion.CreateFromAxisAngle(Vector3.UnitY, 0.5f);
 		var right = Quaternion.CreateFromAxisAngle(Vector3.UnitX, -0.25f);
-		var resolver = new ConcatenateResolver(
+		var resolver = new QuaternionConcatenateResolver(
 			new VariantResolver(new Variant128(left), typeof(Quaternion)),
 			new VariantResolver(new Variant128(right), typeof(Quaternion)));
 
@@ -44,7 +44,7 @@
 	public void Concatenate_resolver_throws_for_non_quaternion_left_operand()
 	{
 #pragma warning disable CA1806 // Do not ignore method results
-		Action act = () => new ConcatenateResolver(
+		Action act = () => new QuaternionConcatenateResolver(
 			new VariantResolver(new Variant128(Vector3.UnitX), typeof(Vector3)),
 			new VariantResolver(new Variant128(Quaternion.Identity), typeof(Quaternion)));
 #pragma warning restore CA1806 // Do not ignore method results
@@ -57,7 +57,7 @@
 	public void Concatenate_resolver_throws_for_non_quaternion_right_operand()
 	{
 #pragma warning disable CA1806 // Do not ignore method results
-		Action act = () => new ConcatenateResolver(
+		Action act = () => new QuaternionConcatenateResolver(
 			new VariantResolver(new Variant128(Quaternion.Identity), typeof(Quaternion)),
 			new VariantResolver(new Variant128(Vector3.UnitX), typeof(Vector3)));
 #pragma warning restore CA1806 // Do not ignore method results

Forge.Tests/Tags/EntityTagsEventTests.cs

@@ -0,0 +1,108 @@
+// Copyright © Gamesmiths Guild.
+
+using FluentAssertions;
+using Gamesmiths.Forge.Cues;
+using Gamesmiths.Forge.Effects;
+using Gamesmiths.Forge.Effects.Components;
+using Gamesmiths.Forge.Effects.Duration;
+using Gamesmiths.Forge.Tags;
+using Gamesmiths.Forge.Tests.Helpers;
+
+namespace Gamesmiths.Forge.Tests.Tags;
+
+public class EntityTagsEventTests(TagsAndCuesFixture tagsAndCuesFixture) : IClassFixture<TagsAndCuesFixture>
+{
+	private readonly TagsManager _tagsManager = tagsAndCuesFixture.TagsManager;
+	private readonly CuesManager _cuesManager = tagsAndCuesFixture.CuesManager;
+
+	[Fact]
+	[Trait("Event", null)]
+	public void An_effect_granting_a_modifier_tag_raises_the_event_with_the_updated_tags()
+	{
+		TestEntity target = CreateEntity();
+
+		int raised = 0;
+		TagContainer? reported = null;
+
+		target.Tags.OnTagsChanged += tags =>
+		{
+			raised++;
+			reported = tags;
+		};
+
+		ApplyModifierTagsEffect(target, "color.red");
+
+		raised.Should().Be(1);
+		reported.Should().BeSameAs(target.Tags.AllTags);
+		reported!.HasTagExact(Tag.RequestTag(_tagsManager, "color.red")).Should().BeTrue();
+	}
+
+	[Fact]
+	[Trait("Event", null)]
+	public void Removing_the_effect_raises_the_event_with_the_tag_already_gone()
+	{
+		TestEntity target = CreateEntity();
+		ActiveEffectHandle? handle = ApplyModifierTagsEffect(target, "color.red");
+
+		bool hadTagWhenRaised = true;
+		target.Tags.OnTagsChanged += tags =>
+			hadTagWhenRaised = tags.HasTagExact(Tag.RequestTag(_tagsManager, "color.red"));
+
+		target.EffectsManager.RemoveEffect(handle!);
+
+		hadTagWhenRaised.Should().BeFalse();
+	}
+
+	[Fact]
+	[Trait("Event", null)]
+	public void A_second_effect_granting_the_same_tag_raises_nothing()
+	{
+		TestEntity target = CreateEntity();
+		ApplyModifierTagsEffect(target, "color.red");
+
+		int raised = 0;
+		target.Tags.OnTagsChanged += _ => raised++;
+
+		// The reference count goes up but AllTags does not change, so there is nothing to report.
+		ActiveEffectHandle? second = ApplyModifierTagsEffect(target, "color.red");
+		raised.Should().Be(0);
+
+		// Nor when the first of the two goes away, since the tag is still held by the other.
+		target.EffectsManager.RemoveEffect(second!);
+		raised.Should().Be(0);
+	}
+
+	[Fact]
+	[Trait("Event", null)]
+	public void The_event_only_reports_changes_on_its_own_entity()
+	{
+		TestEntity watched = CreateEntity();
+		TestEntity other = CreateEntity();
+
+		int raised = 0;
+		watched.Tags.OnTagsChanged += _ => raised++;
+
+		ApplyModifierTagsEffect(other, "color.red");
+
+		raised.Should().Be(0);
+	}
+
+	private TestEntity CreateEntity()
+	{
+		return new TestEntity(_tagsManager, _cuesManager);
+	}
+
+	private ActiveEffectHandle? ApplyModifierTagsEffect(TestEntity target, params string[] tagKeys)
+	{
+		var effectData = new EffectData(
+			"Tag Granting Effect",
+			new DurationData(DurationType.Infinite),
+			effectComponents:
+			[
+				new ModifierTagsEffectComponent(
+					new TagContainer(_tagsManager, TestUtils.StringToTag(_tagsManager, tagKeys)))
+			]);
+
+		return target.EffectsManager.ApplyEffect(new Effect(effectData, new EffectOwnership(target, target)));
+	}
+}

Forge/Abilities/Ability.cs

@@ -146,6 +146,7 @@ internal bool TryActivateAbility(
 			return true;
 		}
 
+		Owner.Abilities.NotifyAbilityActivationFailed(Handle, failureFlags);
 		return false;
 	}
 
@@ -161,6 +162,7 @@ internal bool TryActivateAbility<TData>(
 			return true;
 		}
 
+		Owner.Abilities.NotifyAbilityActivationFailed(Handle, failureFlags);
 		return false;
 	}
 
@@ -571,16 +573,28 @@ private void Activate(IForgeEntity? abilityTarget, float magnitude)
 	{
 		AbilityInstance instance = CreateInstance(abilityTarget);
 		_activeInstances.Add(instance);
+		NotifyActivated();
 		instance.Start(magnitude);
 	}
 
 	private void Activate<TData>(IForgeEntity? abilityTarget, TData data, float magnitude)
 	{
 		AbilityInstance instance = CreateInstance(abilityTarget);
 		_activeInstances.Add(instance);
+		NotifyActivated();
 		instance.Start(data, magnitude);
 	}
 
+	private void NotifyActivated()
+	{
+		if (_activeInstances.Count != 1)
+		{
+			return;
+		}
+
+		Owner.Abilities.NotifyAbilityActivated(Handle);
+	}
+
 	private AbilityInstance CreateInstance(IForgeEntity? abilityTarget)
 	{
 		// Cancel conflicting abilities before we start this one. An empty container means this ability conflicts with

Forge/Core/EntityAbilities.cs

@@ -16,14 +16,65 @@ namespace Gamesmiths.Forge.Core;
 public class EntityAbilities(IForgeEntity owner)
 {
 	private readonly Dictionary<Ability, List<IAbilityGrantSource>> _grantSources = [];
+	private readonly HashSet<AbilityHandle> _grantedAbilities = [];
 	private Action<Ability>? _removeAbility;
 	private Action<Ability>? _inhibitAbility;
 
+	/// <summary>
+	/// Event invoked when an ability is granted to the entity, carrying its handle.
+	/// </summary>
+	/// <remarks>
+	/// Raised once per <see cref="Ability"/>, after the grant has settled, so the handle already reports its level and
+	/// inhibition state. Granting an ability the entity already has adds a grant source instead of a second ability,
+	/// and raises <see cref="OnAbilityChanged"/> if that changed anything.
+	/// </remarks>
+	public event Action<AbilityHandle>? OnAbilityGranted;
+
+	/// <summary>
+	/// Event invoked when a granted ability's level or inhibition changes, carrying its handle.
+	/// </summary>
+	/// <remarks>
+	/// A change that resolves to the same values — a repeat grant that neither overrides the level nor flips
+	/// inhibition — raises nothing.
+	/// </remarks>
+	public event Action<AbilityHandle>? OnAbilityChanged;
+
+	/// <summary>
+	/// Event invoked when an ability is removed from the entity, carrying its handle.
+	/// </summary>
+	/// <remarks>
+	/// Raised once the last grant source is gone, before the handle is invalidated, so the handle can still be read
+	/// inside the handler and reports <see cref="AbilityHandle.IsValid"/> as <see langword="false"/> afterwards.
+	/// Losing one of several grant sources keeps the ability and raises <see cref="OnAbilityChanged"/> instead.
+	/// </remarks>
+	public event Action<AbilityHandle>? OnAbilityRemoved;
+
+	/// <summary>
+	/// Event invoked when an ability becomes active, carrying its handle.
+	/// </summary>
+	/// <remarks>
+	/// The exact counterpart of <see cref="OnAbilityEnded"/>: both track the ability, not its instances, so a second
+	/// concurrent instance of a per-execution ability raises neither. Raised as the ability becomes active and before
+	/// its behavior starts, so it always precedes the matching <see cref="OnAbilityEnded"/> — including for a behavior
+	/// that finishes synchronously.
+	/// </remarks>
+	public event Action<AbilityHandle>? OnAbilityActivated;
+
 	/// <summary>
 	/// Event invoked when an ability ends.
 	/// </summary>
 	public event Action<AbilityEndedData>? OnAbilityEnded;
 
+	/// <summary>
+	/// Event invoked when an activation attempt is refused, carrying the ability's handle and every reason it failed.
+	/// </summary>
+	/// <remarks>
+	/// The direct callers of the activation API already receive these flags as an out parameter. This event covers the
+	/// activations nobody is holding the result of — those driven by <see cref="AbilityTriggerData"/> tags and events,
+	/// and by the Statescript activation nodes — which are otherwise silent.
+	/// </remarks>
+	public event Action<AbilityHandle, AbilityActivationFailures>? OnAbilityActivationFailed;
+
 	/// <summary>
 	/// Gets the owner of this effects manager.
 	/// </summary>
@@ -32,7 +83,13 @@ public class EntityAbilities(IForgeEntity owner)
 	/// <summary>
 	/// Gets the set of abilities currently granted to the entity.
 	/// </summary>
-	public HashSet<AbilityHandle> GrantedAbilities { get; } = [];
+	/// <remarks>
+	/// Read-only: the manager keeps this set in step with the grant sources behind each ability, so grant and removal
+	/// go through <see cref="GrantAbilityPermanently"/>, <c>GrantAbilityAndActivateOnce</c> and the effect components
+	/// rather than through this set. The collection is live, so a handle removed while it is being enumerated
+	/// invalidates the enumeration; copy it first when the loop body can remove abilities.
+	/// </remarks>
+	public IReadOnlyCollection<AbilityHandle> GrantedAbilities => _grantedAbilities;
 
 	/// <summary>
 	/// Gets the tags that block abilities from being used.
@@ -298,6 +355,9 @@ public AbilityHandle GrantAbilityPermanently(
 
 		if (existingAbility is not null && existingAbility.SourceEntity == sourceEntity)
 		{
+			bool wasInhibited = existingAbility.IsInhibited;
+			int previousLevel = existingAbility.Level;
+
 			_grantSources[existingAbility].Add(new PermanentGrantSource());
 
 			// If the ability was fully inhibited, this permanent grant should re-enable it.
@@ -313,13 +373,17 @@ public AbilityHandle GrantAbilityPermanently(
 				existingAbility.Level = abilityLevel;
 			}
 
+			NotifyAbilityChanged(existingAbility, wasInhibited, previousLevel);
+
 			return existingAbility.Handle;
 		}
 
 		var newAbility = new Ability(Owner, abilityData, abilityLevel, sourceEntity);
-		GrantedAbilities.Add(newAbility.Handle);
+		_grantedAbilities.Add(newAbility.Handle);
 		_grantSources[newAbility] = [new PermanentGrantSource()];
 
+		OnAbilityGranted?.Invoke(newAbility.Handle);
+
 		return newAbility.Handle;
 	}
 
@@ -348,6 +412,9 @@ internal AbilityHandle GrantAbility(
 
 		if (existingAbility is not null && existingAbility.SourceEntity == sourceEntity)
 		{
+			bool wasInhibited = existingAbility.IsInhibited;
+			int previousLevel = existingAbility.Level;
+
 			// Ability already granted, just add the new source to the mapping.
 			_grantSources[existingAbility].Add(grantSource);
 
@@ -364,15 +431,21 @@ internal AbilityHandle GrantAbility(
 				existingAbility.Level = abilityLevel;
 			}
 
+			NotifyAbilityChanged(existingAbility, wasInhibited, previousLevel);
+
 			return existingAbility.Handle;
 		}
 
 		var newAbility = new Ability(Owner, abilityData, abilityLevel, sourceEntity);
-		GrantedAbilities.Add(newAbility.Handle);
+		_grantedAbilities.Add(newAbility.Handle);
 		_grantSources[newAbility] = [grantSource];
 
+		// Set before announcing the grant, so the handle already reports its settled inhibition state and an inhibited
+		// grant never reads as a change to an ability nobody has been told about yet.
 		newAbility.IsInhibited = grantSource.IsInhibited;
 
+		OnAbilityGranted?.Invoke(newAbility.Handle);
+
 		return newAbility.Handle;
 	}
 
@@ -449,6 +522,16 @@ internal void NotifyAbilityEnded(AbilityEndedData abilityEndedData)
 		OnAbilityEnded?.Invoke(abilityEndedData);
 	}
 
+	internal void NotifyAbilityActivated(AbilityHandle abilityHandle)
+	{
+		OnAbilityActivated?.Invoke(abilityHandle);
+	}
+
+	internal void NotifyAbilityActivationFailed(AbilityHandle abilityHandle, AbilityActivationFailures failureFlags)
+	{
+		OnAbilityActivationFailed?.Invoke(abilityHandle, failureFlags);
+	}
+
 	private static bool MatchesTags(Ability ability, TagContainer tagsToActivate)
 	{
 		return ability.AbilityData.AbilityTags?.HasAny(tagsToActivate) == true;
@@ -533,8 +616,12 @@ private void RemoveAbility(Ability abilityToRemove)
 		}
 
 		abilityToRemove.Cleanup();
+
+		// Raised before the handle is freed, so handlers can still read what went away.
+		OnAbilityRemoved?.Invoke(abilityToRemove.Handle);
+
 		abilityToRemove.Handle.Free();
-		GrantedAbilities.Remove(abilityToRemove.Handle);
+		_grantedAbilities.Remove(abilityToRemove.Handle);
 	}
 
 	private void InhibitAbility(Ability abilityToInhibit)
@@ -545,7 +632,23 @@ private void InhibitAbility(Ability abilityToInhibit)
 			_inhibitAbility = null;
 		}
 
+		bool wasInhibited = abilityToInhibit.IsInhibited;
+
 		abilityToInhibit.IsInhibited = CheckIsInhibited(abilityToInhibit);
+
+		NotifyAbilityChanged(abilityToInhibit, wasInhibited, abilityToInhibit.Level);
+	}
+
+	private void NotifyAbilityChanged(Ability ability, bool wasInhibited, int previousLevel)
+	{
+		// Announces a change only when one of the two observable pieces of ability state actually moved: the grant
+		// paths run unconditionally, and a repeat grant that overrides nothing must stay silent.
+		if (ability.IsInhibited == wasInhibited && ability.Level == previousLevel)
+		{
+			return;
+		}
+
+		OnAbilityChanged?.Invoke(ability.Handle);
 	}
 
 	private bool CheckIsInhibited(Ability ability)