# Card System Architecture Audit

**Date:** November 11, 2025  
**Author:** Senior Software Engineer  
**Status:** Critical Review

---

## Executive Summary

The current card UI system suffers from **excessive wrapper nesting**, **duplicated animation logic**, and **unclear separation of concerns**. While functional, it violates DRY principles and creates maintenance overhead. A refactor using composition and state machines is recommended.

---

## Current Architecture

### Component Hierarchy
```
CardDisplay (core visual renderer)
  └─ AlbumCard (album-specific wrapper)
      └─ FlippableCard (flip animation wrapper)
          └─ AlbumCardPlacementDraggable (drag/placement wrapper)
              └─ CardDraggable (generic drag wrapper)
                  └─ CardDraggableVisual (visual for dragging)
```

### Critical Issues

#### 1. **Wrapper Hell**
- **5 layers of wrappers** around a single card display
- Each wrapper duplicates transform/animation state management
- Example: `FlippableCard`, `AlbumCard`, and `CardDraggable` all manage scales, positions, and parent tracking
- **Code smell**: `AlbumCard.OnPointerClick()` forwards clicks to parent `FlippableCard` during reveal flow

#### 2. **Duplicated Animation Logic**
Animation behaviors repeated across multiple components:

| Animation | FlippableCard | AlbumCard | CardDraggable | AlbumCardPlacementDraggable |
|-----------|---------------|-----------|---------------|------------------------------|
| Scale tweens | ✓ (hover, flip punch) | ✓ (enlarge/shrink) | - | - |
| Position tweens | ✓ (idle hover) | - | ✓ (drag) | ✓ (snap to slot) |
| Rotation tweens | ✓ (flip) | - | - | - |
| Transform state tracking | ✓ (_originalPosition, _originalScale) | ✓ (_originalParent, _originalLocalPosition, _originalLocalRotation) | - | - |

**Impact**: ~150 lines of redundant tween/transform code across 4 files.

#### 3. **State Management Chaos**
Multiple boolean flags tracking overlapping states:
- `FlippableCard`: `_isFlipped`, `_isFlipping`, `_isWaitingForTap`, `_isClickable`, `_isNew`
- `AlbumCard`: `_isEnlarged`, `_parentSlot != null` (implicit state)
- `AlbumCardPlacementDraggable`: `_isRevealed`, `_isDragRevealing`, `_waitingForPlacementTap`, `_isHolding`

**Problems**:
- No single source of truth for card state
- Complex conditional logic: `if (_parentSlot == null) { forward to FlippableCard }`
- State transitions scattered across 3+ classes

#### 4. **Unclear Responsibilities**
- `CardDisplay`: Pure renderer ✓ (well-designed)
- `AlbumCard`: Handles enlargement + slot parenting + click forwarding
- `FlippableCard`: Handles flipping + hover animations + new/repeat UI + waiting for taps
- `AlbumCardPlacementDraggable`: Handles drag + flip triggering + slot snapping

Each wrapper blurs the line between "what" (state) and "how" (presentation).

#### 5. **Event Callback Spaghetti**
- 12+ events across components (`OnEnlargeRequested`, `OnShrinkRequested`, `OnCardRevealed`, `OnCardTappedAfterReveal`, `OnFlipStarted`, `OnClickedWhileInactive`, etc.)
- Events chained: `AlbumCard.OnEnlargeRequested` → `AlbumViewPage` → reparent → `AlbumCard.EnlargeCard()`
- Brittle: Changing card flow requires updating 3-4 components + page controllers

---

## Recommended Architecture

### Principles
1. **Composition over inheritance/wrapping**
2. **Single Responsibility**: Card visuals ≠ card behavior ≠ card state
3. **State machines** for clear state transitions
4. **Reusable animation system** instead of per-component tweens

### Proposed Design

Using **Pixelplacement StateMachine** (already in project) with **isolated state-owned visuals**:

```
Card (root GameObject with RectTransform)
├─ CardDisplay (always visible core visual)
├─ CardContext (component - shared data/references)
├─ CardAnimator (component - reusable animations)
└─ CardStateMachine (AppleMachine component)
    ├─ IdleState (GameObject + CardIdleState component)
    ├─ FlippingState (GameObject + CardFlippingState component)
    │   └─ CardBackVisual (child GameObject - owned by this state)
    ├─ RevealedState (GameObject + CardRevealedState component)
    ├─ EnlargedNewState (GameObject + CardEnlargedNewState component)
    │   └─ NewCardBadge (child GameObject - owned by this state)
    ├─ EnlargedRepeatState (GameObject + CardEnlargedRepeatState component)
    │   └─ ProgressBarUI (child GameObject - owned by this state)
    ├─ DraggingState (GameObject + CardDraggingState component)
    └─ PlacedInSlotState (GameObject + CardPlacedInSlotState component)
```

**Key Architecture Decisions:**

1. **State Isolation**: Each state is a **GameObject child** of the StateMachine. State-specific visual elements (CardBackVisual, NewCardBadge, ProgressBarUI) are **children of their state GameObject**. When a state activates, its children activate automatically.

2. **Transform Animation Target**: The root **Card.transform** is the primary animation target. All position/scale animations affect the root, and children inherit transforms naturally. States can also animate their own child visuals independently (e.g., rotating CardBackVisual during flip).

3. **Shared Resources via CardContext**: States access common components (CardDisplay, CardAnimator, StateMachine, CardData) through `CardContext`, avoiding tight coupling.

4. **Reusable Animations**: `CardAnimator` provides animation methods (PlayFlip, PlayEnlarge, etc.) that states invoke. No duplicate tween code across states.

5. **State Transitions**: States call `context.StateMachine.ChangeState("NextState")` to transition. Example flow:
   ```
   IdleState [click] → FlippingState [flip complete] → EnlargedNewState [tap] → RevealedState
   ```

#### Benefits
- **60% less code**: Shared animation system, no wrapper components
- **True state isolation**: Each state owns its visuals, no global visibility management
- **Clear state transitions**: Explicit state machine flow instead of boolean flag soup
- **Extensible**: Add new states without touching existing ones (e.g., `TradingState`, `BattleState`)
- **Designer-friendly**: States are visible GameObjects in hierarchy, easy to understand
- **No prefab nesting**: Single Card prefab with state children, not 5 nested prefabs

---

## Concrete Refactor Plan

### Phase 1: Implement State Machine Architecture ✅ COMPLETE

**Created Files:**
- `CardContext.cs` - Shared context component
- `CardAnimator.cs` - Reusable animation controller  
- `CardAnimationConfig.cs` - ScriptableObject for animation settings
- `States/CardIdleState.cs` - Idle state with hover
- `States/CardFlippingState.cs` - Flip animation state (owns CardBackVisual)
- `States/CardRevealedState.cs` - Revealed/interactable state
- `States/CardEnlargedNewState.cs` - Enlarged new card state (owns NewCardBadge)
- `States/CardEnlargedRepeatState.cs` - Enlarged repeat state (owns ProgressBarUI)

**Example State Implementation:**
```csharp
public class CardFlippingState : AppleState
{
    [SerializeField] private GameObject cardBackVisual; // State owns this visual
    private CardContext _context;
    
    void Awake() => _context = GetComponentInParent<CardContext>();
    
    public override void OnEnterState()
    {
        // Show card back (owned by this state)
        cardBackVisual.SetActive(true);
        _context.CardDisplay.gameObject.SetActive(false);
        
        // Use shared animator
        _context.Animator.PlayFlip(
            cardBackVisual.transform,
            _context.CardDisplay.transform,
            onComplete: () => {
                // Transition to next state
                string nextState = _context.IsNewCard ? "EnlargedNewState" : "RevealedState";
                _context.StateMachine.ChangeState(nextState);
            }
        );
    }
    
    void OnDisable()
    {
        // Hide card back when leaving state
        cardBackVisual.SetActive(false);
        _context.CardDisplay.gameObject.SetActive(true);
    }
}
```

**Prefab Structure:**
```
Card.prefab
├─ CardDisplay
├─ CardContext (component)
├─ CardAnimator (component)
└─ CardStateMachine (AppleMachine)
    ├─ IdleState/
    ├─ FlippingState/
    │   └─ CardBackVisual (Image)
    ├─ RevealedState/
    ├─ EnlargedNewState/
    │   └─ NewCardBadge (GameObject)
    └─ EnlargedRepeatState/
        └─ ProgressBarUI (GameObject with Image/Text)
```

**Impact**: Foundation complete. States are isolated, visuals are state-owned, animations are shared.

### Phase 2: Create Remaining States (Low Risk)

**Additional states needed:**
- `CardDraggingState.cs` - Handles drag interaction for album placement
- `CardPlacedInSlotState.cs` - Card placed in album slot, handles enlarge on click
- `CardAlbumEnlargedState.cs` - Enlarged view when clicking card in album

**Example - Album Placed State:**
```csharp
public class CardPlacedInSlotState : AppleState, IPointerClickHandler
{
    private CardContext _context;
    private AlbumCardSlot _parentSlot;
    
    public void SetParentSlot(AlbumCardSlot slot) => _parentSlot = slot;
    
    public void OnPointerClick(PointerEventData eventData)
    {
        _context.StateMachine.ChangeState("AlbumEnlargedState");
    }
}
```

**Time**: 2-3 days

### Phase 3: Migrate Existing Prefabs (Medium Risk)

**Steps:**
1. Create new `Card.prefab` with state machine structure
2. Build migration tool to convert old prefabs → new structure:
   - Copy CardDisplay references
   - Setup CardContext with data
   - Create state GameObjects
3. Update scenes one at a time:
   - Replace `FlippableCard` spawns with `Card` spawns
   - Update `BoosterOpeningPage` to use new Card system
   - Update `AlbumViewPage` to use new Card system
4. Remove old wrapper scripts once migration complete

**Migration Helper Script:**
```csharp
// Editor tool to convert old card prefabs
[MenuItem("AppleHills/Convert Old Card to New Card")]
static void ConvertCard()
{
    // Find old FlippableCard
    var oldCard = Selection.activeGameObject.GetComponent<FlippableCard>();
    // Extract data, create new Card with states
    // ...
}
```

**Time**: 1-2 weeks (includes testing)

---

## Migration Strategy

### Option A: Incremental (Recommended)
1. Create `CardAnimator` alongside existing code (2-3 days)
2. Refactor one wrapper at a time to use `CardAnimator` (1 week)
3. Test each step with existing scenes
4. Introduce state machine once animations are consolidated (3-5 days)
5. Collapse wrappers last, update prefabs (2-3 days)

**Total**: ~3 weeks, low risk

### Option B: Parallel Track
1. Build new `Card` system in separate namespace (1 week)
2. Create migration tools to convert old prefabs → new prefabs (2-3 days)
3. Switch one scene at a time (1 week)
4. Delete old system once migration complete

**Total**: ~3 weeks, higher risk but cleaner result

---

## Immediate Wins (Low-Hanging Fruit)

Even without full refactor, these changes reduce pain:

### 1. Extract Common Transform Tracking
```csharp
// Assets/Scripts/UI/CardSystem/TransformMemento.cs
public class TransformMemento {
    public Vector3 LocalPosition;
    public Quaternion LocalRotation;
    public Vector3 LocalScale;
    public Transform Parent;
    
    public static TransformMemento Capture(Transform t) { ... }
    public void Restore(Transform t) { ... }
}
```

**Usage**: Replace 8+ `_originalX` fields across components with single `TransformMemento`.

### 2. Shared Animation Config ScriptableObject
```csharp
// Assets/Scripts/UI/CardSystem/CardAnimationConfig.asset
[CreateAssetMenu]
public class CardAnimationConfig : ScriptableObject {
    public float flipDuration = 0.6f;
    public float enlargedScale = 2.5f;
    public float hoverHeight = 10f;
    // etc.
}
```

**Impact**: Tweak all card animations from one asset instead of searching 5 prefabs.

### 3. Document State Transitions
Add state diagram to `FlippableCard.cs`:
```csharp
/// State Flow:
/// Unflipped → [Click] → Flipping → Revealed → [IsNew] → EnlargedNew → [Tap] → Revealed
///                                           → [IsRepeat] → ShowingProgress → Revealed
///                                           → [Tap during drag] → PlacementMode → PlacedInSlot
```

**Impact**: Future devs understand flow without debugging.

---

## Metrics

| Metric | Current | After Refactor |
|--------|---------|----------------|
| Lines of code (card UI) | ~1,200 | ~500 |
| Animation logic locations | 4 files | 1 file |
| State tracking booleans | 12+ | 0 (enum-based) |
| Prefab nesting depth | 5 layers | 1 layer |
| Event callback chains | 12 events | ~3-4 events |
| Time to add new card state | 4-6 hours | ~30 min |

---

## Conclusion

The current system works but is **expensive to maintain and extend**. The root cause is **wrapping components instead of composing behavior**. 

**Recommendation**: Approve **Phase 1 (Animation System)** immediately as it has zero breaking changes and reduces code by 20%. Schedule **Phase 2-3 (State Machine + Wrapper Collapse)** for next sprint based on team bandwidth.

**Risk Assessment**: Medium. Prefab changes require thorough testing, but state machine pattern is battle-tested.

**ROI**: High. Estimated 70% reduction in time to add new card interactions (e.g., trading, upgrading, battling).