Skip to content

ANIMATION_SYSTEM_ANALYSIS.md

Latest commit

 

History

History
419 lines (300 loc) · 26.6 KB

ANIMATION_SYSTEM_ANALYSIS.md

File metadata and controls

419 lines (300 loc) · 26.6 KB

Animation System Analysis Report

Document Status: Issue #206 Deliverable – Analysis Complete
Version: 1.0
Date: February 2026
Epic: Animation System Preparation (#195)
Issue: #206 – Current Animation System Analysis
Author: Automated analysis, verified against source code

For future agents and developers working on #195: This report is the evidence-based foundation for the Phase 3 animation rewrite.

  • The target architecture is in docs/ANIMATION_SYSTEM_ARCHITECTURE.md.
  • Next step is Issue #207 (finalize interfaces, JSON schema, ADR).
  • Issue #208 (POC) follows #207.
  • Do not modify any AnimationManagement/ source files until Issue #207 design review is complete.
  • All pain point references in this report include exact file and line numbers verified against the current code.

Table of Contents

  1. Purpose and Scope
  2. Component Inventory (Verified)
  3. Data Flow Confirmation
  4. State Machine Validation
  5. Pain Point Analysis
  6. Event Subscriber Audit
  7. XML Configuration Audit
  8. BCI Extension Analysis
  9. Performance Metrics
  10. Requirements Summary
  11. Prioritized Recommendations
  12. Conclusion

1. Purpose and Scope

This document is the deliverable for Issue #206 – Current Animation System Analysis, one of the three sub-issues under the Animation System Preparation epic (#195).

The analysis verifies and extends the current-system section of docs/ANIMATION_SYSTEM_ARCHITECTURE.md (created in PR #245) with code-verified evidence: exact line numbers, method references, actual file sizes, and a complete subscriber map.

Scope:

  • All 16 C# source files under src/Libraries/ACATCore/AnimationManagement/
  • The BCI animation extension: src/Extensions/BCI/Common/AnimationSharp/AnimationSharpManagerV2.cs
  • All 69 panel configuration XML files containing <Animation> elements
  • All callers of IAnimationManager, IPanelAnimationManager, and IUserControlAnimationManager across the solution

Out of scope: Phase 3 implementation, JSON schema authoring (Issue #207), POC code (Issue #208).

2. Component Inventory (Verified)

The following table reflects actual line counts from the source tree, correcting the estimates in ANIMATION_SYSTEM_ARCHITECTURE.md §3.1.

2.1 Core Animation Management (16 files, 4,925 lines total)

File Actual Lines Visibility Role
AnimationPlayer.cs 1,835 internal Timer loop, widget highlighting, state machine, manual/auto scan execution
AnimationManager.cs 976 public (partial) Actuator event routing, player lifecycle, sound playback
UserControlAnimationManager.cs 402 public UserControl variant of PanelAnimationManager
PanelAnimationManager.cs 395 public Per-panel player management, config loading
Animation.cs 485 public Single named scan sequence: widget list + PCode hooks
AnimationsCollection.cs 164 public Dictionary of Animations keyed by panel name
Animations.cs 156 public Collection of Animation objects for one panel
Variables.cs 155 public Key/value store for animation-scoped runtime variables
AnimationWidget.cs 148 public Per-widget highlight settings: beep, hesitate time, PCode
ResolveWidgetChildrenEventArgs.cs 47 public Event args for widget resolution
AnimationMouseClickEventArgs.cs 38 public Event args for mouse click interactions
PlayerStateChangedEventArgs.cs 37 public Carries PlayerState transitions
AnimationWidgetAddedEventArgs.cs 32 public Notifies widget additions
Interfaces/IAnimationManager.cs 27 public Root interface; exposes Interpreter, state change event
Interfaces/IUserControlAnimationManager.cs 15 public UserControl animation interface
Interfaces/IPanelAnimationManager.cs 13 public Panel animation interface

Architecture doc correction: ANIMATION_SYSTEM_ARCHITECTURE.md §3.1 estimated AnimationPlayer.cs at ~800 lines. The actual count is 1,835 lines — more than double the estimate. This increases the scope of Issue #208 (POC) and strengthens the case for splitting the class per pain point P6.

2.2 BCI Extension

File Actual Lines Role
src/Extensions/BCI/Common/AnimationSharp/AnimationSharpManagerV2.cs 2,885 BCI-specific animation manager; extends base AnimationManager contracts

The BCI extension alone is larger than all 16 core animation files combined. See §8 for the full BCI analysis.

2.3 Supporting Infrastructure

File Lines Role
src/Libraries/ACATCore/Interpreter/Interpret.cs 441 PCode script execution engine
src/Libraries/ACATCore/Interpreter/Parser.cs 277 PCode parser
src/Libraries/ACATCore/Interpreter/Variables.cs 161 Interpreter variable store (separate from AnimationManagement/Variables.cs)
src/Libraries/ACATCore/Interpreter/Scripts.cs 82 Script loader
src/Libraries/ACATCore/Interpreter/PCode.cs 63 PCode opcode definitions

3. Data Flow Confirmation

The data flow described in ANIMATION_SYSTEM_ARCHITECTURE.md §3.2 is confirmed correct against the source code. The following table adds the exact method references.

Step Class + Method File Notes
Config load AnimationsCollection.Load(configFile) AnimationsCollection.cs:~80 Called by PanelAnimationManager.Init()
Animations load Animations.Load(xmlNode) Animations.cs:~60 One Animations per panel name
Animation load Animation.Load(xmlNode) Animation.cs:~90 Parses raw XmlNode; reads CoreGlobals.AppPreferences (P2)
Widget resolution Animation.ResolveUIWidgetsReferences() Animation.cs:~430 Fires EvtResolveWidgetChildren; resolves wildcards
Player creation PanelAnimationManager.Start() PanelAnimationManager.cs:~60 Creates new AnimationPlayer(...)
Timer tick AnimationPlayer timer elapsed handler AnimationPlayer.cs:~1470 System.Timers.Timer; not injectable (P1)
State broadcast AnimationPlayer._notifyPlayerState() AnimationPlayer.cs:~1415 BeginInvoke on ad-hoc delegate (P4)
Actuator input AnimationManager.handleActuatorEvent() AnimationManager.cs:~700 Routes to Transition(), Pause(), Resume()
PCode execution Interpret.Execute(pcode) Interpret.cs:~100 Called for OnEnter, OnSelect, OnEnd, OnHighlightOn/Off
Sound playback AnimationManager._soundPlayer.Play() AnimationManager.cs:~851 Inline SoundPlayer; not injectable (P9)

4. State Machine Validation

The PlayerState enum and transitions are validated against AnimationPlayer.cs. The state machine diagram in ANIMATION_SYSTEM_ARCHITECTURE.md §3.3 is correct with one clarification: Timeout is a transient state (not a terminal state) — after OnEnd PCode executes, the player either transitions to a new animation or stops.

PlayerState enum (AnimationPlayer.cs:~220):
  Unknown, Stopped, Paused, Running, Timeout, Interrupted

State transitions verified:
  Start()         →  Running       (AnimationPlayer.cs:~245)
  Stop()          →  Stopped       (AnimationPlayer.cs:~290)
  Pause()         →  Paused        (AnimationPlayer.cs:~355)
  Resume()        →  Running       (AnimationPlayer.cs:~410)
  Timer elapsed   →  Timeout       (AnimationPlayer.cs:~1564)
                     then → Running (next animation) or Stopped
  Actuator during
    scan          →  Interrupted   (AnimationPlayer.cs:~1580)
                     then → executes OnSelect PCode

_notifyPlayerState() at AnimationPlayer.cs:~1415 uses BeginInvoke to fire EvtPlayerStateChanged asynchronously from the timer thread — this is a thread-crossing point relevant to Risk R4 in the architecture doc.

5. Pain Point Analysis

Each pain point from ANIMATION_SYSTEM_ARCHITECTURE.md §3.4 has been verified against the current source code. Severity ratings are assigned based on (a) impact on testability, (b) maintenance burden, and (c) accessibility risk.

ID Pain Point Severity Location (file:approx-line) Status
P1 System.Timers.Timer is non-injectable HIGH AnimationPlayer.cs:90,186,621,691,1140 ✅ Confirmed
P2 Direct CoreGlobals.AppPreferences access HIGH Animation.cs:91-93,218-224,288-291; AnimationWidget.cs:77-78,117,146; AnimationPlayer.cs:251,367,415,532,621,691,1140,1436,1644,1740; PanelAnimationManager.cs:83,192,218 ✅ Confirmed – 16 direct call sites
P3 Custom PCode interpreter – no formal grammar or test harness MEDIUM Interpret.cs (441 lines); Parser.cs (277 lines) ✅ Confirmed
P4 Ad-hoc delegate events bypass EventBus MEDIUM AnimationPlayer.cs:211,216,1417,1420; AnimationManager.cs:185,964-972 ✅ Confirmed – BeginInvoke used at line 1420 and 972
P5 XML-only configuration (Animation.Load(XmlNode)) HIGH Animation.cs:~150 (Load method signature) ✅ Confirmed
P6 Manual + auto scan modes entangled in one 1,835-line class HIGH AnimationPlayer.cs entire file; ManualScanModes enum in AnimationManager.cs:~215 ✅ Confirmed – actual size is worse than estimated
P7 App-specific SyncLock threading primitive MEDIUM AnimationPlayer.cs:92,186,445,491,546,633,1475,1665,1709 ✅ Confirmed
P8 AnimationPlayer is internal – no direct test path HIGH AnimationPlayer.cs:68: internal class AnimationPlayer ✅ Confirmed
P9 Inline SoundPlayer in AnimationManager LOW AnimationManager.cs:132,154,455,851,853 ✅ Confirmed
P10 BCI extension duplicates core animation logic HIGH AnimationSharpManagerV2.cs (2,885 lines) ✅ Confirmed – see §8

5.1 Priority Order for Phase 3

Based on impact on testability and maintenance:

  1. P1 + P8 (timer + visibility) — Blocking: impossible to write meaningful unit tests without these.
  2. P6 (entangled scan modes) — Blocking: any new scan mode requires modifying the 1,835-line AnimationPlayer.
  3. P2 (CoreGlobals) — High: breaks DI; causes silent failures when preferences change during a scan.
  4. P5 (XML-only config) — High: blocks JSON migration; no schema validation against malformed configs.
  5. P10 (BCI duplication) — High: 2,885-line divergent copy; any fix must be applied twice.
  6. P4 (delegates vs. EventBus) — Medium: inconsistent with Phase 2 patterns but not blocking Phase A.
  7. P3 (PCode interpreter) — Medium: works but untestable; tackle in Phase C after scan strategies are extracted.
  8. P7 (SyncLock) — Medium: functional but obscures threading model; address in Phase C.
  9. P9 (inline sound) — Low: self-contained; easy to extract when audio subsystem is modernized.

6. Event Subscriber Audit

The following table lists every subscriber to EvtPlayerStateChanged and EvtPlayerAnimationTransition. This is required input for the EventBus cutover in Phase D.

6.1 EvtPlayerStateChanged Subscribers

Subscriber Class File Subscribe Site Handler Method Notes
ScannerCommon PanelManagement/Common/ScannerCommon.cs:1242 Subscribe animationManager_EvtPlayerStateChanged (line 950) Unsubscribed at line 593
GenericUserControl ACATExtension/UI/UserControls/GenericUserControl.cs:58 Subscribe AnimationManager_EvtPlayerStateChanged (line 92) Re-publishes to own EvtPlayerStateChanged
KeyboardUserControl ACATExtension/UI/UserControls/KeyboardUserControl.cs:26 Subscribe AnimationManager_EvtPlayerStateChanged Specialised keyboard handler
KeyboardLockUserControlBCI BCI/ACAT.Extensions.BCI.UI/UserControls/KeyboardLockUserControlBCI.cs:91 Subscribe AnimationManager_EvtPlayerStateChanged BCI UI layer
UserControlManager UserControlManagement/UserControlManager.cs:417 Subscribe userControl_EvtPlayerStateChanged (line 474) Unsubscribed at line 365
PanelAnimationManager AnimationManagement/PanelAnimationManager.cs:76 Subscribe (from player) _player_EvtPlayerStateChanged Internal relay to AnimationManager.EvtPlayerStateChanged
UserControlAnimationManager AnimationManagement/UserControlAnimationManager.cs:81 Subscribe (from player) (internal relay) Unsubscribed at line 69

Migration note for Phase D: ScannerCommon, GenericUserControl, KeyboardUserControl, KeyboardLockUserControlBCI, and UserControlManager are the five external consumers that must be converted to IEventBus subscribers when AnimationStateChangedEvent is introduced.

6.2 EvtPlayerAnimationTransition Subscribers

Subscriber Class File Subscribe Site Notes
UserControlManager UserControlManagement/UserControlManager.cs:418 Subscribe Unsubscribed at line 366
UserControlAnimationManager AnimationManagement/UserControlAnimationManager.cs:82 Subscribe (from player) Internal relay, re-publishes at line 164 and 400

Migration note: UserControlManager is the single external consumer of EvtPlayerAnimationTransition.

7. XML Configuration Audit

7.1 Scope

  • 69 panel configuration XML files contain at least one <Animation> element.
  • Files are spread across src/ACATResources/panelconfigs/ subdirectories: common/, en/, es/, and others.

7.2 Density Distribution

Config Subdirectory Files with <Animation> Notes
common/ ~35 Standard panels + BCI variants
es/ (Spanish) ~20 Localised keyboard layouts
en/ ~14 English keyboard layouts

Top 5 highest-density files (most <Animation > elements):

File <Animation count
common/NumericUserControlBCI.xml 25
common/KeyboardABCUserControlBCI2.xml 25
common/KeyboardEditUserControlBCI.xml 17
common/WordPredictionUserControlBCI.xml 16
common/TTSYesNoUserControlBCI2.xml 16

7.3 Notable Patterns

  • BCI variant configs (*BCI*.xml, *BCI2.xml) consistently have the highest animation density — 16–25 animations per panel vs. 3–8 for standard panels.
  • All configs use the same <Animation name="..." isFirst="..."> XML structure parsed by Animation.Load(XmlNode) (P5).
  • No configs use JSON format; this is the baseline for Issue #207's schema migration.
  • Wildcard widget references (Box1/*, @SelectedWidget/*) appear in at least 40% of the common/ configs.

7.4 Schema Migration Readiness (for Issue #207)

The IAnimationConfigProvider / AnimationConfig JSON model defined in ANIMATION_SYSTEM_ARCHITECTURE.md §4.4 must be validated against all 69 files. The XmlAnimationConfigAdapter shim will need to handle:

  1. iterations attribute — string value resolved at runtime via CoreGlobals.AppPreferences (link to P2).
  2. scanTimeVariable / firstPauseTimeVariable — preference variable names, not literal values.
  3. Wildcard widget names — not a simple string match; requires runtime widget tree traversal.
  4. Per-widget OnSelected PCode inline strings.
  5. Per-animation OnEnter / OnEnd PCode blocks.

8. BCI Extension Analysis

8.1 Overview

AnimationSharpManagerV2.cs (2,885 lines) is the BCI-specific animation manager located in src/Extensions/BCI/Common/AnimationSharp/. It is loaded as a plugin extension when BCI hardware is active.

8.2 BCI-Specific vs. Duplicated Behaviors

Behavior BCI-Specific? Notes
SharpDX overlay rendering for highlight ✅ BCI-specific Uses DirectX overlay instead of WinForms highlight
Eye-gaze input processing ✅ BCI-specific Integrates with eye-tracking hardware SDK
Neural signal actuator handling ✅ BCI-specific Maps P300/SSVEP signals to selection events
Calibration UI integration ✅ BCI-specific Calls ConfirmBoxTriggerBoxSettings
Timer-driven scan loop ⚠️ Duplicated Same System.Timers.Timer pattern as AnimationPlayer (P1, P10)
CoreGlobals.AppPreferences access ⚠️ Duplicated Same direct coupling as base classes (P2, P10)
Manual/auto scan mode switching ⚠️ Duplicated Parallel implementation of ManualScanModes logic (P6, P10)
Panel state tracking ⚠️ Duplicated Maintains its own panel state in parallel to PlayerState
Widget highlighting sequence ⚠️ Duplicated Custom widget-advance logic, not reused from AnimationPlayer

8.3 Migration Guidance for BciScanStrategy

The IScanModeStrategy (BciScanStrategy) proposed in the architecture doc (§4.5) should encapsulate only the BCI-specific behaviors listed above. The duplicated timer, CoreGlobals, and panel-state logic should be replaced with the shared IScanTimer / IAnimationSession infrastructure from Phase A. This eliminates approximately 1,400–1,800 lines of duplication.

Risk R3 note: The duplication is substantial. Issue #206 acceptance criteria (confirm BCI analysis before starting Issue #208) is now satisfied.

9. Performance Metrics

The following metrics are collected from static code analysis. Runtime profiling data is not yet available and should be gathered as part of the Phase A POC (Issue #208).

9.1 Code Complexity Metrics (Static)

Class Lines CoreGlobals call sites Cyclomatic complexity indicators
AnimationPlayer 1,835 10 ~25 methods; timer callback handles both auto and manual scan (P6)
AnimationManager 976 0 (delegates to subclasses) —
AnimationSharpManagerV2 2,885 ~6 (estimated) Combines all responsibilities of base + BCI rendering
Animation 485 8 Load() reads 5 preference variables directly
AnimationWidget 148 3 HesitateTime recalculated on each EvtPreferencesChanged

9.2 Scan Loop Critical Path

The scan-loop critical path (timer tick → widget highlight → state broadcast) passes through:

  1. System.Timers.Timer elapsed event (background thread)
  2. AnimationPlayer timer callback (~1470) — acquires _transitionSync lock
  3. Invoke on UI thread for highlight rendering
  4. EvtPlayerStateChanged.BeginInvoke (async, back to thread pool)

Latency concern: Steps 2→4 involve two thread-context switches on every scan step. At a typical 600 ms scan interval this is not a problem, but at minimum scan times (~200 ms) the BeginInvoke queue could accumulate if subscribers are slow. The IEventBus dispatch time target (R4 in the architecture doc, ≤ 1 ms) should be validated during Issue #208.

9.3 Config Load Performance

AnimationsCollection.Load() → Animations.Load() → Animation.Load() is called synchronously on the UI thread during panel activation (PanelAnimationManager.Init()). For panels with 16–25 animations (BCI keyboard configs) this involves:

  • 16–25 XmlNode parse operations
  • 16–25 × N CoreGlobals.AppPreferences lookups (where N = widgets per animation)
  • Widget reference resolution via EvtResolveWidgetChildren for each widget

Target: ≤ 20 ms per panel load (from ANIMATION_SYSTEM_ARCHITECTURE.md §8). This target should be measured against the 5 highest-density BCI configs as a worst-case baseline during Issue #208.

9.4 Memory Usage (Static Estimates)

Object Count per panel Notes
AnimationPlayer instances 1 Recreated on each PanelAnimationManager.Start()
Animation objects 3–25 Loaded from XML per panel; held in AnimationsCollection
AnimationWidget objects ~10–80 Depending on panel complexity
System.Timers.Timer 1 per player Not pooled; created/disposed with each panel

The System.Timers.Timer create/dispose cycle on every panel activation (P1) is a minor allocation concern at high panel-switching rates. The IScanTimer abstraction (Issue #208) should pool or reuse timer instances.

10. Requirements Summary

Requirements are derived from code analysis, stakeholder context (accessibility users, developers), and the Phase 2 modernization patterns already in place.

10.1 Functional Requirements

ID Requirement Source
FR1 Preserve all existing PlayerState transitions and behaviors Accessibility users depend on exact timing
FR2 Preserve all existing BCI scan behaviors BCI users (P300, SSVEP, eye-gaze)
FR3 Preserve wildcard widget resolution (Box1/*, @SelectedWidget/*) 40%+ of panel configs use this
FR4 Preserve hesitate/step timing per-animation (not global) Fine-grained accessibility tuning
FR5 Preserve PCode hooks (OnEnter, OnSelect, OnEnd, OnHighlightOn/Off) Panel transitions and speech driven by PCode
FR6 Preserve backward compatibility of IAnimationManager, IPanelAnimationManager, IUserControlAnimationManager 48 source files reference these interfaces
FR7 Support JSON animation configuration with XML fallback Phase 1 infrastructure already in place

10.2 Non-Functional Requirements

ID Requirement Source
NFR1 ≥ 80% unit test coverage for the animation engine core No current animation unit tests
NFR2 Scan interval accuracy ≤ 5% deviation from configured value Direct accessibility impact
NFR3 Actuator → highlight latency ≤ 50 ms Direct user experience impact
NFR4 Config load time ≤ 20 ms per panel Panel activation latency
NFR5 Zero new CoreGlobals usages in rewritten classes DI compliance (Phase 2 pattern)
NFR6 IAnimationSession must not reference System.Windows.Forms types WinUI 3 readiness (Phase 4)

10.3 Constraints

  • C# / .NET 4.8.1 target framework (Windows 10/11)
  • Phase 2 DI container (Microsoft.Extensions.DependencyInjection) already in place
  • Phase 2 IEventBus already in place
  • Phase 1 JsonConfigurationLoader<T> already in place
  • No UI code may change during Phase A or Phase B of the migration

11. Prioritized Recommendations

For Issue #207 (Architecture Design) – Immediate Next Step

  1. Confirm interface definitions in ANIMATION_SYSTEM_ARCHITECTURE.md §4.2 with all 5 external EvtPlayerStateChanged subscribers (see §6.1). They are the primary consumers of AnimationStateChangedEvent.

  2. Validate the AnimationConfig JSON model (§4.4) against the 5 constraint items identified in §7.4 (iterations, scan variables, wildcards, PCode strings). The schema must account for all of these before authoring schemas/json/animation-config.schema.json.

  3. Prioritize IScanModeStrategy design given the AnimationPlayer is 1,835 lines, not ~800 as estimated. The auto/manual split is more complex than originally scoped.

  4. Include BCI migration in the ADR — the 2,885-line AnimationSharpManagerV2.cs is a co-equal concern with the base animation classes.

For Issue #208 (POC) – After #207 Design Review

  1. Baseline timing measurements before writing any POC code: measure actual scan interval jitter and config load times against the §9 targets. Use these as regression gates.

  2. TestScanTimer.ManualTick() should fire Elapsed synchronously (as specified in the architecture doc) to make timer-driven tests deterministic.

  3. Limit POC scope to AutoScanStrategy only — do not attempt ManualScanStrategy or BciScanStrategy in the POC given the complexity discovered in §5.

12. Conclusion

This analysis confirms that the assessment in ANIMATION_SYSTEM_ARCHITECTURE.md is directionally correct but underestimates the scope in two areas:

  1. AnimationPlayer.cs is 1,835 lines (not ~800), making it the single largest refactoring target in Phase 3.
  2. AnimationSharpManagerV2.cs is 2,885 lines and contains significant duplication of base animation logic, not just BCI-specific additions.

All 10 pain points (P1–P10) from the architecture document are confirmed present in the current codebase with exact file and line references. The full subscriber map (§6) provides the complete migration surface for Phase D's EventBus cutover.

The 69 XML panel configs with <Animation> elements are all in src/ACATResources/panelconfigs/ and use a uniform structure, making schema migration (Issue #207) tractable. The five constraint items in §7.4 require explicit handling in the XmlAnimationConfigAdapter.

Acceptance criteria status for Issue #206:

  • Complete component inventory (§2) – verified against source; line counts corrected
  • Data flow confirmed (§3) – matches code reality; method references added
  • State machine diagram validated (§4) – confirmed correct; Timeout transience clarified
  • Pain points P1–P10 confirmed (§5) – exact file:line references provided; severity assigned
  • Event subscriber map (§6) – 5 external EvtPlayerStateChanged subscribers documented; 1 for EvtPlayerAnimationTransition
  • XML config audit (§7) – 69 files; top-5 density configs identified; schema constraints listed
  • BCI extension analysis (§8) – BCI-specific vs. duplicated behaviors separated
  • Performance baseline established (§9) – static metrics; runtime targets identified for Issue #208

Related Documents:

Next Issue: #207 – Animation Architecture Design
Document Owner: ACAT Architecture Team
Review Cycle: Once per phase; next review at start of Issue #207