Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental feature : Lu #488

Merged
merged 2 commits into from
Feb 22, 2025

Conversation

luf1k
Copy link
Contributor

@luf1k luf1k commented Feb 18, 2025

Mark the type contribution you are making:

  • Experimental feature (new functionality that can be selectively enabled/disabled)
  • Bug fix (non-breaking change which fixes an issue)

Description

Adding a new experimental feature "Play with Lu" that integrates an AI gaming assistant into Delta. This feature allows players to ask questions about their games during gameplay and receive helpful responses.

Why this change is necessary:

  • Enhances player experience by providing immediate game-related assistance
  • Integrates modern AI capabilities within the emulator experience
  • Provides contextual help based on current gameplay state

Solution details:

  • Extension-based approach keeps changes isolated and toggleable
  • Uses existing Delta experimental features framework
  • Implements proper error handling and loading states
  • Follows Delta's UI/UX patterns for consistency

Key implementations:

  • Adds a new "Ask Lu" menu item to the pause menu
  • Integrates with Lu Labs API for game knowledge and responses (Lu-Info.plist for configs)
  • Includes user feedback mechanism for response quality
  • Handles save state tracking for gameplay context

Testing

Tested Configurations:

  • iPhone 11 Pro, iOS 16.5
  • iPhone 12, iOS 18.1.1
  • iPhone 15 Pro iOS 18.2.1
  • iPhone 15 Pro Max 18.2.1
  • iPhone 14 Pro iOS 18.3
  • iPhone 15 iOS 18.1.1
  • iPhone 12 Pro iOS 17.6.1

Tested scenarios:

  • Feature enable/disable toggle
  • Supported and unsupported games
  • Network error conditions
  • Different save state configurations
  • Question answering flow
  • User feedback flow
  • Welcome message flow
  • Remembering conversations

Checklist

General (All PRs)

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I've tested my changes with different device + OS version configurations

Experimental Feature-specific

  • Added property to ExperimentalFeatures struct annotated with @Feature
  • Uses @Option's to persist all feature-related data
  • Locked all behavior changes behind ExperimentalFeatures.shared.[feature].isEnabled runtime check
  • Isolates changes to separate files as much as possible (e.g. via Swift extensions)

- Add Lu API integration for real-time game assistance
- Implement authorization flow and token management
- Enhance logging for AI interaction events
- Add caching layer for improved performance
- Update pause menu UI for Lu integration
- Include configuration files and assets for Lu feature
- Implement extension points in PauseViewController for AI support
Copy link
Owner

@rileytestut rileytestut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting everything into separate files as much as possible! Just have a minor comment about redeclaring Notification.Name.settingsDidChange, but besides that LGTM for an Experimental Feature release.

Comment on lines 35 to 38

extension Notification.Name {
static let settingsDidChange = Notification.Name("Settings.didChangeNotification")
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
extension Notification.Name {
static let settingsDidChange = Notification.Name("Settings.didChangeNotification")
}

Is there a reason Notification.Name.settingsDidChange is redefined here, instead of using existing definition in Notification+Settings.swift (or Settings.didChangeNotification)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, this was an alternative way for presenting the welcome message, not needed indeed. just removed it 👍

@luf1k luf1k force-pushed the feature/lu-integration branch from 6d1e1b0 to 4337113 Compare February 21, 2025 22:01
@luf1k luf1k requested a review from rileytestut February 21, 2025 22:06
@rileytestut rileytestut merged commit ffbf666 into rileytestut:main Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants