Parent
Part of the MagicAST restructure milestone. Phase 4 of 5. Marked ready-for-human because trait-boundary decisions are domain-modeling judgment calls that need a human review checkpoint mid-implementation.
This is the riskiest phase in the milestone because it touches every consumer of the current Effect.IsOptional / Duration / IfYouDo / UnlessClause fields.
What to build
Today, the base Effect record carries optional fields that apply to most effects but not all:
IsOptional (declarative "you may" effects)
Duration (durative effects: "until end of turn", etc.)
IfYouDo / UnlessClause (conditional sub-clauses)
Adding a new optional dimension touches the base record and every derived type. Worse: some concrete effects carry mutually-exclusive optional fields with no type-level enforcement — e.g., EvasionEffect.CanBeBlockedBy is meaningful exactly when UnblockableCondition is null, but the type doesn't say so.
Replace with trait composition via marker interfaces:
// New trait interfaces in libs/magic-ast/AST/Effects/Traits/:
public interface IOptionalEffect { bool IsOptional { get; init; } }
public interface IDurativeEffect { Duration? Duration { get; init; } }
public interface IConditionalEffect { Effect? IfYouDo { get; init; } Effect? UnlessClause { get; init; } }
// Base Effect is stripped to truly universal fields only:
public abstract record Effect { /* likely just SourceSpan and any other universals */ }
// Concrete effects declare their trait set:
[OracleEffect("deal-damage")]
public sealed record DealDamageEffect(...) : Effect, IDurativeEffect, IOptionalEffect { ... }
[OracleEffect("evasion")]
public sealed record EvasionEffect(...) : Effect { ... } // no traits
// Consumers check via pattern matching:
if (effect is IDurativeEffect dur) { ... }
HITL checkpoints required during implementation:
- Trait boundary review — once the proposed list of trait interfaces is drafted (likely 3–6 traits), pause and get human sign-off. Splitting wrong (e.g., conflating durative + conditional into one interface) is hard to reverse later.
- Consumer audit — every place that reads
effect.IsOptional / .Duration / etc. needs updating. Likely consumers: serialization layer (Phase 2 converter), tests, any analysis code in tools/ or tests/. Pause for review after the consumer list is enumerated but before bulk edits.
- JSON round-trip verification — confirm the Phase 2 polymorphic converter handles trait-bearing effects correctly under
Strict mode. The discriminator-driven deserialization must populate trait properties on the correct concrete type. This is where mistakes in (1) surface as test failures.
Acceptance criteria
Blocked by
Parent
Part of the MagicAST restructure milestone. Phase 4 of 5. Marked
ready-for-humanbecause trait-boundary decisions are domain-modeling judgment calls that need a human review checkpoint mid-implementation.This is the riskiest phase in the milestone because it touches every consumer of the current
Effect.IsOptional/Duration/IfYouDo/UnlessClausefields.What to build
Today, the base
Effectrecord carries optional fields that apply to most effects but not all:IsOptional(declarative "you may" effects)Duration(durative effects: "until end of turn", etc.)IfYouDo/UnlessClause(conditional sub-clauses)Adding a new optional dimension touches the base record and every derived type. Worse: some concrete effects carry mutually-exclusive optional fields with no type-level enforcement — e.g.,
EvasionEffect.CanBeBlockedByis meaningful exactly whenUnblockableConditionis null, but the type doesn't say so.Replace with trait composition via marker interfaces:
HITL checkpoints required during implementation:
effect.IsOptional/.Duration/ etc. needs updating. Likely consumers: serialization layer (Phase 2 converter), tests, any analysis code intools/ortests/. Pause for review after the consumer list is enumerated but before bulk edits.Strictmode. The discriminator-driven deserialization must populate trait properties on the correct concrete type. This is where mistakes in (1) surface as test failures.Acceptance criteria
Effectrecord contains only fields universal to all effect types (audit the corpus to confirm).libs/magic-ast/AST/Effects/Traits/(or similar location).docs/adr/entry if you prefer).SealedTrait<T1, T2>pattern, or separate concrete effect subtypes — implementer's call after design review).test-baseline.json.Blocked by