Add balloon sway animation

[onefovrth]

Summary

The balloon icon in the conversations toolbar now responds to device tilt via CoreMotion. When the user tilts the phone, the balloon smoothly sways following gravity (±30° max). On simulator, an idle sway keeps the animation running for preview purposes.

Implementation

• DeviceMotionManager: Singleton observable class wrapping CMMotionManager, reads gravity.x via device motion updates at 30Hz
• BalloonSwayModifier: ViewModifier applying rotationEffect anchored at the circle center, manages motion manager lifecycle
• Exponential smoothing: Applies 0.12 factor for floaty feel (~280ms lag to 63% of target angle)
• Reference counting: Stops motion updates when no listeners are active to preserve battery

Test Plan

• Build on device and tilt phone left/right — balloon sways smoothly
• Run on simulator — idle sway animation plays
• Lint passes (0 violations)

[claude]

Claude finished @onefovrth's task —— View job

---

Code Review Complete ✓

Todo List

• Read all changed files
• Review code quality and best practices
• Check for potential bugs
• Evaluate performance considerations
• Review security concerns
• Assess test coverage
• Provide final feedback

---

Review Feedback

✅ Strengths

Architecture & Design:

• Clean separation of concerns with DeviceMotionManager handling CoreMotion and BalloonSwayModifier handling SwiftUI integration
• Proper use of @Observable with @MainActor for modern SwiftUI state management
• Reference counting pattern for motion updates is smart for battery optimization
• Fallback to idle sway animation on simulator is thoughtful for development workflow

Code Quality:

• Follows project conventions from CLAUDE.md (Observable pattern, enum Constant naming)
• Good use of weak self to prevent retain cycles
• Exponential smoothing provides nice floaty feel

---

🔍 Issues & Suggestions

1. Thread Safety Issue in DeviceMotionManager:48-55

The device motion updates deliver to .main queue, but then wrap in Task { @MainActor }. This is redundant and could cause subtle timing issues.

// Current (line 48-55):
motionManager.startDeviceMotionUpdates(using: .xArbitraryZVertical, to: .main) { [weak self] motion, _ in
    Task { @MainActor [weak self] in
        guard let self, let gravity = motion?.gravity else { return }
        // ...
    }
}

// Suggested:
motionManager.startDeviceMotionUpdates(using: .xArbitraryZVertical, to: .main) { [weak self] motion, _ in
    guard let self, let gravity = motion?.gravity else { return }
    // Already on main queue, no Task wrapper needed
    let rawAngle: Double = asin(max(-1, min(1, gravity.x))) * (180.0 / .pi)
    let clamped: Double = max(-Constant.maxAngle, min(Constant.maxAngle, rawAngle))
    self.rollAngle = self.rollAngle * (1 - Constant.smoothingFactor) + clamped * Constant.smoothingFactor
}

Why: The .main queue parameter already ensures execution on the main thread. Adding Task { @MainActor } creates unnecessary async overhead and could introduce timing inconsistencies.

---

2. Same Issue in Timer Callback (DeviceMotionManager:60-67)

// Current (line 60-67):
idleSwayTimer = Timer.scheduledTimer(withTimeInterval: Constant.updateInterval, repeats: true) { [weak self] _ in
    Task { @MainActor [weak self] in
        guard let self else { return }
        // ...
    }
}

// Suggested:
idleSwayTimer = Timer.scheduledTimer(withTimeInterval: Constant.updateInterval, repeats: true) { [weak self] _ in
    guard let self else { return }
    self.idleSwayPhase += Constant.idleSwaySpeed
    self.rollAngle = sin(self.idleSwayPhase) * Constant.idleSwayPrimaryAmplitude
        + sin(self.idleSwayPhase * 0.7) * Constant.idleSwaySecondaryAmplitude
}

Why: Timer.scheduledTimer already fires on the RunLoop where it was scheduled (main thread in this case since the class is @MainActor).

---

3. Reference Count Can Go Negative (DeviceMotionManager:37-40)

func stopUpdates() {
    activeListenerCount -= 1
    guard activeListenerCount <= 0 else { return }
    activeListenerCount = 0  // Reset to prevent negative values

Issue: If stopUpdates() is called more times than startUpdates(), activeListenerCount could go negative before being reset. While line 40 prevents this from persisting, it could cause bugs if logic relies on the count mid-function.

Suggested:

func stopUpdates() {
    guard activeListenerCount > 0 else { return }
    activeListenerCount -= 1
    guard activeListenerCount == 0 else { return }
    
    motionManager.stopDeviceMotionUpdates()
    startIdleSway()
}

---

4. Idle Sway Doesn't Stop on Device (DeviceMotionManager:42-43)

func stopUpdates() {
    // ...
    motionManager.stopDeviceMotionUpdates()
    startIdleSway()  // ⚠️ Always starts idle sway, even on device
}

Issue: When all listeners disconnect on a real device, the code stops device motion but immediately starts idle sway. This means the animation never truly stops, which defeats the battery optimization goal.

Expected behavior: On device with motion available, stopping updates should stop all animation. Idle sway should only be a fallback for simulator/unavailable motion.

Suggested:

func stopUpdates() {
    guard activeListenerCount > 0 else { return }
    activeListenerCount -= 1
    guard activeListenerCount == 0 else { return }
    
    motionManager.stopDeviceMotionUpdates()
    stopIdleSway()  // Stop all animations when no listeners
}

---

5. Missing Test Coverage

No unit tests were added for:

• DeviceMotionManager reference counting logic
• Motion update smoothing calculations
• Idle sway animation math
• Lifecycle behavior (start/stop scenarios)

Suggested: Add tests for the reference counting logic at minimum, since bugs there could cause battery drain or crashes.

---

⚡ Performance Considerations

Positive:

• 30Hz update rate is reasonable for smooth animation without excessive CPU usage
• Reference counting pattern prevents unnecessary motion updates
• Exponential smoothing reduces jitter without expensive filtering

Concern:

• Issue Add PostHog for Analytics #4 above means the Timer continues running even when motion should be stopped on device, using unnecessary CPU cycles

---

🔒 Security

No security concerns identified. CoreMotion access is standard for motion-based features and requires proper privacy string in Info.plist (which should already exist).

---

📋 Best Practices Check

CLAUDE.md Compliance:

• ✅ Uses @Observable instead of ObservableObject
• ✅ Enum named Constant (singular, not plural)
• ✅ No force unwrapping
• ✅ No implicitly unwrapped optionals
• ✅ Proper use of private(set) for exposed state
• ✅ Constants enum at bottom of scope
• ⚠️ Minor: Line 51-53 could be split for readability (currently 138 chars, max is 200 but shorter is better)

---

Summary

This is a well-structured feature with good separation of concerns and thoughtful battery optimization. The main issues are:

1. Critical: Redundant Task { @MainActor } wrappers cause unnecessary async overhead (lines 48-55, 60-67)
2. Important: Idle sway continues running on device after stopUpdates(), defeating battery optimization (line 43)
3. Minor: Reference count could briefly go negative (line 37-40)
4. Nice-to-have: Add unit tests for reference counting and animation logic

The code quality is strong overall and follows project conventions well. With the fixes above, this will be production-ready.

---

[macroscopeapp]

Add motion-driven rotation to the settings toolbar icon using DeviceMotionManager.startUpdates at ~30 Hz with roll clamped to ±30° in AppSettingsView.swift

Introduce BalloonSwayModifier and a .balloonSway() view extension to rotate views from DeviceMotionManager.rollAngle, including lifecycle hooks; apply the modifier to the settings toolbar image.

📍Where to Start

Start with DeviceMotionManager in DeviceMotionManager.swift, then review BalloonSwayModifier in BalloonSwayModifier.swift, and finally its use in AppSettingsView.swift.

---

📊 Macroscope summarized c0504df. 3 files reviewed, 2 issues evaluated, 0 issues filtered, 1 comment posted. View details

[macroscopeapp]

🟢 Low

Animation/DeviceMotionManager.swift:43 stopUpdates() should call stopIdleSway() (not startIdleSway()) when the listener count drops to zero, so no timer keeps running.

Suggested change

	startIdleSway()
	stopIdleSway()

🚀 Want me to fix this? Reply ex: "fix it for me".

🤖 Prompt for AI

In file Convos/Design System/Animation/DeviceMotionManager.swift around line 43:

`stopUpdates()` should call `stopIdleSway()` (not `startIdleSway()`) when the listener count drops to zero, so no timer keeps running.