Added per operation cue changes mechanism.

Forge.Tests/Cues/CueMagnitudeReentrancyTests.cs

@@ -18,23 +18,28 @@
 namespace Gamesmiths.Forge.Tests.Cues;
 
 /// <summary>
-/// An <see cref="CueMagnitudeType.AttributeValueChange"/> cue reads the deltas an effect has left pending on its
-/// target, and any effect landing on that same target flushes them. A hook that runs between the effect's attribute
-/// writes and its cues — an executed hook raising an event that activates an ability, a changed hook applying
-/// threshold effects — can therefore land such an effect, and the cue must still report what its own effect did.
-/// The cue handlers themselves are arbitrary code too, so they keep running after the hooks: reading early is what
-/// protects the magnitudes, not dispatching early.
+/// An <see cref="CueMagnitudeType.AttributeValueChange"/> cue and the modifier-success check read the changes the
+/// effect's own operation made to its target, tallied apart from every other operation on that entity. A hook that
+/// runs between the effect's attribute writes and its cues — an executed hook raising an event that activates an
+/// ability, a changed hook applying threshold effects — can land another effect on the same target, and neither that
+/// effect's cues nor the original's may read each other's changes. The cue handlers themselves are arbitrary code
+/// too, so they keep running after the hooks: closing the tally early is what protects the magnitudes, not
+/// dispatching early.
 /// </summary>
 /// <param name="tagsAndCuesFixture">The fixture providing the <see cref="TagsManager"/> and <see cref="CuesManager"/>.
 /// </param>
 public class CueMagnitudeReentrancyTests(TagsAndCuesFixture tagsAndCuesFixture) : IClassFixture<TagsAndCuesFixture>
 {
 	private const string CueAttribute = "TestAttributeSet.Attribute90";
 	private const string SideAttribute = "TestAttributeSet.Attribute1";
+	private const string KeptAttribute = "TestAttributeSet.Attribute1000";
+	private const string ArrivingAttribute = "VitalAttributeSet.CurrentHealth";
 
 	private readonly TagsManager _tagsManager = tagsAndCuesFixture.TagsManager;
 	private readonly CuesManager _cuesManager = tagsAndCuesFixture.CuesManager;
 	private readonly TestCue _cue = tagsAndCuesFixture.TestCueInstances[0];
+	private readonly TestCue _sideCue = tagsAndCuesFixture.TestCueInstances[1];
+	private readonly TestCue _otherCue = tagsAndCuesFixture.TestCueInstances[2];
 
 	[Fact]
 	[Trait("Execute", null)]
@@ -134,12 +139,156 @@ public void Update_cue_handlers_run_after_the_changed_hooks_so_a_handler_removin
 		}
 	}
 
-	private static Effect CreateSideEffect(TestEntity target)
+	[Fact]
+	[Trait("Execute", null)]
+	public void A_nested_effects_cues_read_the_changes_of_its_own_execution_and_not_the_outer_ones()
+	{
+		var target = new TestEntity(_tagsManager, _cuesManager);
+		_cue.Reset();
+		_sideCue.Reset();
+		_otherCue.Reset();
+
+		// The side effect lands while the outer hit's change to Attribute90 is still pending on the entity. Its cue on
+		// that attribute must read nothing, and only its cue on the attribute it touched must read its own change.
+		target.Events.Subscribe(
+			EventTag(),
+			_ => target.EffectsManager.ApplyEffect(CreateSideEffect(
+				target,
+				cue: CreateCue("test.cue2"),
+				otherCue: CreateCue("test.cue3", SideAttribute))));
+
+		target.EffectsManager.ApplyEffect(CreateInstantEffect(target));
+
+		_cue.ExecuteData.Value.Should().Be(-10);
+		_sideCue.ExecuteData.Count.Should().Be(1);
+		_sideCue.ExecuteData.Value.Should().Be(0);
+		_otherCue.ExecuteData.Count.Should().Be(1);
+		_otherCue.ExecuteData.Value.Should().Be(1);
+	}
+
+	[Fact]
+	[Trait("Execute", null)]
+	public void Modifier_success_is_judged_on_the_effects_own_execution()
+	{
+		var target = new TestEntity(_tagsManager, _cuesManager);
+		_cue.Reset();
+		_sideCue.Reset();
+
+		// A nested effect whose modifier changes nothing must not have its cues fired by the outer hit's pending
+		// change.
+		target.Events.Subscribe(
+			EventTag(),
+			_ => target.EffectsManager.ApplyEffect(CreateSideEffect(
+				target,
+				magnitude: 0,
+				cue: CreateCue("test.cue2"),
+				requireModifierSuccess: true)));
+
+		target.EffectsManager.ApplyEffect(CreateInstantEffect(target));
+
+		_cue.ExecuteData.Value.Should().Be(-10);
+		_sideCue.ExecuteData.Count.Should().Be(0);
+	}
+
+	[Fact]
+	[Trait("Execute", null)]
+	public void Every_cue_of_an_execution_is_read_before_its_hooks_and_handlers_can_move_the_attributes()
+	{
+		var target = new TestEntity(_tagsManager, _cuesManager);
+		_cue.Reset();
+		_sideCue.Reset();
+
+		// Both an executed hook and the first cue's own handler take another 5 off the attribute; the second cue, read
+		// after the execution's writes and before either ran, still reports the 80 this execution left.
+		target.Events.Subscribe(
+			EventTag(),
+			_ => target.EffectsManager.ApplyEffect(CreateSideEffect(target, CueAttribute, -5)));
+
+		void ReenterFromCue()
+		{
+			target.EffectsManager.ApplyEffect(CreateSideEffect(target, CueAttribute, -5));
+		}
+
+		_cue.OnExecuted += ReenterFromCue;
+
+		try
+		{
+			target.EffectsManager.ApplyEffect(CreateInstantEffect(
+				target,
+				CreateCue("test.cue2", CueAttribute, CueMagnitudeType.AttributeCurrentValue)));
+
+			target.PlayerAttributeSet.Attribute90.CurrentValue.Should().Be(70);
+			_cue.ExecuteData.Value.Should().Be(-10);
+			_sideCue.ExecuteData.Value.Should().Be(80);
+		}
+		finally
+		{
+			_cue.OnExecuted -= ReenterFromCue;
+		}
+	}
+
+	[Fact]
+	[Trait("Update", null)]
+	public void Update_cues_after_an_attribute_set_leaves_report_the_modifier_it_took_with_it()
+	{
+		var target = new TestEntity(_tagsManager, _cuesManager);
+		var vitalSet = new VitalAttributeSet();
+		target.Attributes.AddAttributeSet(vitalSet);
+		_cue.Reset();
+		_sideCue.Reset();
+
+		target.EffectsManager.ApplyEffect(CreateCrossSetEffect(target));
+		target.Attributes[ArrivingAttribute].CurrentValue.Should().Be(90);
+
+		// The departing attribute is detached before the update cues run, but the unapply that gave it its 10 back was
+		// this effect's doing and is still reported, the way its listeners still hear that last change.
+		target.Attributes.RemoveAttributeSet(vitalSet);
+
+		_cue.UpdateData.Count.Should().Be(1);
+		_cue.UpdateData.Value.Should().Be(10);
+		_sideCue.UpdateData.Value.Should().Be(0);
+	}
+
+	[Fact]
+	[Trait("Update", null)]
+	public void Update_cues_after_an_attribute_set_arrives_report_what_the_effect_now_contributes()
+	{
+		var target = new TestEntity(_tagsManager, _cuesManager);
+		_cue.Reset();
+		_sideCue.Reset();
+
+		// Applied while the entity lacks the set, so only the kept attribute is modified at first.
+		target.EffectsManager.ApplyEffect(CreateCrossSetEffect(target));
+
+		target.Attributes.AddAttributeSet(new VitalAttributeSet());
+
+		// The arriving attribute takes the effect's full modifier; the kept one is unapplied and re-applied, which nets
+		// to nothing. Both are read after every effect was rebuilt and the attributes flushed.
+		target.Attributes[ArrivingAttribute].CurrentValue.Should().Be(90);
+		_cue.UpdateData.Count.Should().Be(1);
+		_cue.UpdateData.Value.Should().Be(-10);
+		_sideCue.UpdateData.Count.Should().Be(1);
+		_sideCue.UpdateData.Value.Should().Be(0);
+	}
+
+	private static Effect CreateSideEffect(
+		TestEntity target,
+		string attribute = SideAttribute,
+		float magnitude = 1,
+		CueData? cue = null,
+		CueData? otherCue = null,
+		bool requireModifierSuccess = false)
 	{
+		CueTriggerRequirement requirement = requireModifierSuccess
+			? CueTriggerRequirement.OnExecute
+			: CueTriggerRequirement.None;
+
 		var effectData = new EffectData(
 			"Side Effect",
 			new DurationData(DurationType.Instant),
-			[CreateModifier(SideAttribute, 1)]);
+			[CreateModifier(attribute, magnitude)],
+			requireModifierSuccessToTriggerCue: requirement,
+			cues: [.. new[] { cue, otherCue }.OfType<CueData>()]);
 
 		return new Effect(effectData, new EffectOwnership(target, target));
 	}
@@ -152,7 +301,39 @@ private static Modifier CreateModifier(string attribute, float magnitude)
 			new ModifierMagnitude(MagnitudeCalculationType.ScalableFloat, new ScalableFloat(magnitude)));
 	}
 
-	private Effect CreateInstantEffect(TestEntity target)
+	private static CueData CreateCue(
+		Tag cueTag,
+		string attribute,
+		CueMagnitudeType magnitudeType = CueMagnitudeType.AttributeValueChange)
+	{
+		return new CueData(cueTag.GetSingleTagContainer(), -100, 100, magnitudeType, attribute);
+	}
+
+	private CueData CreateCue(Tag? cueTag = null)
+	{
+		return CreateCue(cueTag ?? Tag.RequestTag(_tagsManager, "test.cue1"), CueAttribute);
+	}
+
+	private CueData CreateCue(
+		string cueTagName,
+		string attribute = CueAttribute,
+		CueMagnitudeType magnitudeType = CueMagnitudeType.AttributeValueChange)
+	{
+		return CreateCue(Tag.RequestTag(_tagsManager, cueTagName), attribute, magnitudeType);
+	}
+
+	private Effect CreateCrossSetEffect(TestEntity target)
+	{
+		var effectData = new EffectData(
+			"Cross Set Buff",
+			new DurationData(DurationType.Infinite),
+			[CreateModifier(KeptAttribute, 10), CreateModifier(ArrivingAttribute, -10)],
+			cues: [CreateCue("test.cue1", ArrivingAttribute), CreateCue("test.cue2", KeptAttribute)]);
+
+		return new Effect(effectData, new EffectOwnership(target, target));
+	}
+
+	private Effect CreateInstantEffect(TestEntity target, CueData? extraCue = null)
 	{
 		var effectData = new EffectData(
 			"Instant Effect",
@@ -162,7 +343,7 @@ private Effect CreateInstantEffect(TestEntity target)
 			[
 				new RaiseEventEffectComponent(EventTag().GetSingleTagContainer()!, EffectEventTrigger.Executed)
 			],
-			cues: [CreateCue()]);
+			cues: [.. new[] { CreateCue(), extraCue }.OfType<CueData>()]);
 
 		return new Effect(effectData, new EffectOwnership(target, target));
 	}
@@ -201,16 +382,6 @@ private Effect CreateStackableEffect(TestEntity target, Tag? cueTag = null)
 		return new Effect(effectData, new EffectOwnership(target, target));
 	}
 
-	private CueData CreateCue(Tag? cueTag = null)
-	{
-		return new CueData(
-			(cueTag ?? Tag.RequestTag(_tagsManager, "test.cue1")).GetSingleTagContainer(),
-			-100,
-			100,
-			CueMagnitudeType.AttributeValueChange,
-			CueAttribute);
-	}
-
 	private Tag EventTag()
 	{
 		return Tag.RequestTag(_tagsManager, "tag");

Forge/Attributes/AttributeChangeSet.cs

@@ -0,0 +1,103 @@
+// Copyright © Gamesmiths Guild.
+
+using Gamesmiths.Forge.Core;
+
+namespace Gamesmiths.Forge.Attributes;
+
+/// <summary>
+/// The net change one operation — an effect executing, applying, restacking or being rebuilt — made to each attribute
+/// of an entity, kept apart from whatever else changed those attributes meanwhile.
+/// </summary>
+/// <remarks>
+/// <para>
+/// An <see cref="EntityAttribute"/> also keeps an entity-wide pending change, but that one accumulates across every
+/// operation until the next flush publishes it, so a nested operation — a raised event activating an ability that
+/// commits its cost while a hit is still landing — would read the hit's deltas as its own. Cues and the
+/// modifier-success check read this instead, through the scope <see cref="EntityAttributes.BeginChanges"/> opens
+/// around the operation.
+/// </para>
+/// <para>
+/// Pooled by its <see cref="EntityAttributes"/>, so an execution costs no allocation once warm. A few entries searched
+/// linearly are cheaper than a dictionary for the handful of attributes one effect touches.
+/// </para>
+/// </remarks>
+internal sealed class AttributeChangeSet
+{
+	private readonly List<AttributeChange> _changes = [];
+
+	/// <summary>
+	/// Gets a value indicating whether the operation left any attribute's current value different from before.
+	/// </summary>
+	internal bool HasChanges
+	{
+		get
+		{
+			foreach (AttributeChange change in _changes)
+			{
+				if (change.Delta != 0)
+				{
+					return true;
+				}
+			}
+
+			return false;
+		}
+	}
+
+	/// <summary>
+	/// Gets the net change the operation made to the current value of the attribute with a key; zero for one it never
+	/// touched.
+	/// </summary>
+	/// <remarks>
+	/// Looked up by key rather than through the entity's attributes so the change survives the attribute leaving: an
+	/// attribute set being removed unapplies its modifiers, which is recorded here, and detaches the attributes before
+	/// the update cues read it, and a hook can swap a set for a fresh instance between an operation and its cues.
+	/// </remarks>
+	/// <param name="attributeKey">The key of the attribute to look up.</param>
+	/// <returns>The net change to the attribute's current value.</returns>
+	internal int DeltaOf(StringKey attributeKey)
+	{
+		for (int i = 0; i < _changes.Count; i++)
+		{
+			if (_changes[i].Attribute.Key == attributeKey)
+			{
+				return _changes[i].Delta;
+			}
+		}
+
+		return 0;
+	}
+
+	internal void Record(EntityAttribute attribute, int delta)
+	{
+		int index = IndexOf(attribute);
+
+		if (index < 0)
+		{
+			_changes.Add(new AttributeChange(attribute, delta));
+			return;
+		}
+
+		_changes[index] = new AttributeChange(attribute, _changes[index].Delta + delta);
+	}
+
+	internal void Clear()
+	{
+		_changes.Clear();
+	}
+
+	private int IndexOf(EntityAttribute attribute)
+	{
+		for (int i = 0; i < _changes.Count; i++)
+		{
+			if (ReferenceEquals(_changes[i].Attribute, attribute))
+			{
+				return i;
+			}
+		}
+
+		return -1;
+	}
+
+	private readonly record struct AttributeChange(EntityAttribute Attribute, int Delta);
+}

Forge/Attributes/EntityAttribute.cs

@@ -110,6 +110,12 @@ public sealed class EntityAttribute
 
 	internal int PendingValueChange { get; private set; }
 
+	/// <summary>
+	/// Gets or sets the entity's attribute container this attribute is attached to, which tallies its changes per
+	/// operation; <see langword="null"/> while it belongs to no entity.
+	/// </summary>
+	internal EntityAttributes? Container { get; set; }
+
 	internal EntityAttribute(
 		StringKey key,
 		int defaultValue,
@@ -440,7 +446,9 @@ private void RefreshValues(int oldValue)
 
 		if (CurrentValue != oldValue)
 		{
-			PendingValueChange += CurrentValue - oldValue;
+			int delta = CurrentValue - oldValue;
+			PendingValueChange += delta;
+			Container?.RecordChange(this, delta);
 		}
 	}