-
Notifications
You must be signed in to change notification settings - Fork 243
feat: added interactive games (fixes #1312) #1316
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
base: development
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR refactors the HomeScreen to support a new “Game” tab alongside the badge editor and integrates two interactive games (Snake and Tetris) with dedicated providers and UI screens, while also enhancing badge saving/editing flows by persisting original text and improving cache and file handling. Sequence Diagram: User Navigates to Play Snake GamesequenceDiagram
actor User
participant HS as HomeScreen
participant GSS as GameSelectionScreen
participant GS as GameScreen (Snake)
participant SGP as SnakeGameProvider
User->>HS: Taps "Game" tab
HS->>HS: _selectedIndex = 1 (show game UI)
HS->>GSS: Displays GameSelectionScreen
User->>GSS: Selects "Snake"
GSS->>GS: Navigates to GameScreen
activate GS
GS->>SGP: initGame()
activate SGP
SGP-->>GS: Game initialized
deactivate SGP
GS->>SGP: startGame()
activate SGP
SGP-->>GS: Game loop starts, grid updates
deactivate SGP
GS-->>User: Display Snake game
deactivate GS
Sequence Diagram: User Edits and Saves an Existing BadgesequenceDiagram
actor User
participant SBC as SaveBadgeCard
participant HS as HomeScreen
participant BTS as BadgeTextStorage
participant SBP as SavedBadgeProvider
participant FH as FileHelper
User->>SBC: Taps "Edit" for "my_badge.json"
SBC->>HS: Navigate(savedBadgeData, savedBadgeFilename="my_badge.json")
activate HS
HS->>HS: initState() calls _applySavedBadgeData()
HS->>BTS: getOriginalText("my_badge.json")
activate BTS
BTS-->>HS: Returns "Original Text for my_badge"
deactivate BTS
HS->>HS: Update UI with badge data & original text (e.g., inlineimagecontroller.text)
HS-->>User: Shows badge editor with pre-filled data
deactivate HS
User->>HS: Modifies badge & Taps "Save" button
activate HS
HS->>SBP: updateBadgeData("my_badge", newText, newEffects, newSpeed, newMode)
activate SBP
SBP->>FH: saveBadgeData(updatedData, "my_badge", isInverted) // FileHelper updates file and cache
activate FH
FH-->>SBP: Badge data saved/overwritten
deactivate FH
SBP->>BTS: saveOriginalText("my_badge.json", newText)
activate BTS
BTS-->>SBP: Original text updated in storage
deactivate BTS
SBP-->>HS: Update successful
deactivate SBP
HS->>User: Shows "Badge Updated Successfully" toast
deactivate HS
Entity Relationship Diagram: Original Badge Text StorageerDiagram
OriginalTextEntry {
string badgeFilename PK "Unique identifier for the badge file, e.g., 'my_badge.json'"
string originalText "The raw, user-entered text for the badge, e.g., 'Hello <<icon_id>>'"
}
note OriginalTextEntry "Represents an entry within 'badge_original_texts.json'. This JSON file is managed by the BadgeTextStorage class and acts as a key-value store mapping badge filenames to their original textual content before any processing or compilation for the LED display."
Class Diagram: Badge Saving and Data Handling ComponentsclassDiagram
class HomeScreen {
+Map~String, dynamic~ savedBadgeData
+String savedBadgeFilename
+void _applySavedBadgeData()
+void onTapSaveBadge() void
}
class SavedBadgeProvider {
+FileHelper fileHelper
+Future~void~ saveNewBadgeData(String filename, String message, bool isFlash, bool isMarquee, bool isInvert, int speed, int animation)
+Future~void~ updateBadgeData(String filename, String message, bool isFlash, bool isMarquee, bool isInvert, int speed, int animation)
}
class FileHelper {
+Future~void~ saveBadgeData(Data data, String filename, bool isInvert) // Handles file I/O and cache
+Data jsonToData(Map~String, dynamic~ jsonData) // Includes error handling for missing keys
}
class BadgeTextStorage {
+static String TEXT_STORAGE_FILENAME
+static Future~void~ saveOriginalText(String badgeFilename, String originalText)
+static Future~String~ getOriginalText(String badgeFilename)
+static Future~void~ deleteOriginalText(String badgeFilename)
}
class SaveBadgeCard {
+Map~String, dynamic~ badgeData
+String badgeFilename
+void onTapEditButton() // Navigates to HomeScreen for editing
}
HomeScreen o-- SavedBadgeProvider : uses
HomeScreen o-- BadgeTextStorage : uses (via _applySavedBadgeData)
SavedBadgeProvider o-- FileHelper : uses
SavedBadgeProvider o-- BadgeTextStorage : uses (indirectly or directly)
SaveBadgeCard ..> HomeScreen : (navigates to for editing)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nope3472 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
children: [ | ||
GestureDetector( | ||
onTap: () async { | ||
if (inlineImageProvider.getController().text.isEmpty) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Inconsistent empty check for text field.
Use '.trim().isEmpty' instead of '.isEmpty' to ensure whitespace-only input is treated as empty and to maintain consistency with the original code.
if (inlineImageProvider.getController().text.isEmpty) { | |
if (inlineImageProvider.getController().text.trim().isEmpty) { |
try { | ||
// Convert JSON data to Data object | ||
Data data = Data.fromJson(jsonData); | ||
return data; | ||
} catch (e) { | ||
// If there's an error with the 'messages' key missing, add it with default values | ||
if (e.toString().contains("Missing \"messages\" key")) { | ||
logger.w('Fixing missing "messages" key in badge data'); | ||
|
||
// Create a default message structure if missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): jsonToData may mask data issues by auto-fixing missing keys.
Log a warning or error when default values are used, and notify the UI or user about potentially incomplete badge data.
void tick() { | ||
if (isGameOver) return; | ||
if (!_moveShape(1, 0)) { | ||
_mergeShapeToGrid(); | ||
_clearLines(); | ||
if (!_spawnNewShape()) { | ||
isGameOver = true; | ||
_timer?.cancel(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Game timer is started in constructor, which may cause issues if provider is disposed and recreated.
This approach can result in multiple active timers if the provider is recreated. Move timer initialization to a dedicated startGame method and handle timer cancellation in dispose to prevent resource leaks.
Suggested implementation:
TetrisGameProvider() {
_spawnNewShape();
}
void startGame() {
_timer?.cancel();
_timer = Timer.periodic(Duration(milliseconds: tickMillis), (_) => tick());
}
@override
void dispose() {
_timer?.cancel();
super.dispose();
}
- You will need to call
startGame()
explicitly when you want the game to begin (e.g., from your UI or controller). - If your provider does not already extend
ChangeNotifier
or a class with adispose
method, ensure it does so, or adjust thedispose
method accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nope3472 - I've reviewed your changes - here's some feedback:
- The HomeScreen widget has grown to over 600 lines handling both badge editing and game selection; consider splitting it into smaller widgets (e.g., BadgeEditorScreen and GameTabNavigator) to improve readability and maintainability.
- The bottom navigation UI is duplicated across HomeScreen, GameScreen, and TetrisGameScreen; extract a common BottomNavBar widget to reduce redundancy and ensure consistency.
- Bundling badge editing enhancements and two new games in a single PR makes review difficult; try breaking this into focused PRs per feature (e.g., Snake, Tetris, badge edit) for clearer feedback and safer merges.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
badgeText = widget.savedBadgeFilename! | ||
.substring(0, widget.savedBadgeFilename!.length - 5); | ||
// If the filename is a timestamp, use a generic text | ||
if (badgeText.contains(":") && badgeText.contains("-")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: Filename fallback logic is fragile
Parsing the filename using a defined timestamp format or storing the original text as metadata would provide a more reliable solution than checking for ':' and '-'.
return data; | ||
} catch (e) { | ||
// If there's an error with the 'messages' key missing, add it with default values | ||
if (e.toString().contains("Missing \"messages\" key")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Fragile error message string matching in jsonToData
Instead of matching error strings, check if 'messages' exists in jsonData before calling Data.fromJson, or catch a more specific exception if possible.
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/15445790529/artifacts/3260100579. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
Hey @mariobehling @Jhalakupadhyay can you have a look on this pr for adding interactive games do let me know if my approach is right or do i need to add anything else |
Hi, please add images to the step by step guide. The issue says that games should be added into the firmware of the badge. I do not see this. Where do you upload the game to the badge, where is the firmware upload happening? |
Hi @mariobehling, since I don’t yet have the physical badge, I’m relying on the preview badge in the app. I haven’t had a chance to transfer it to the firmware—once I receive the badge, I’ll test whether the transfer works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @nope3472,
I saw your recent changes—great work! I was curious about how you're connecting to the badge for live sync because as far as i know the current badge disconnects on every transfer, how you are approaching this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, as discussed some time ago elsewhere this PR cannot be merged yet. I am adding relevant points to start below:
-
No confirmed data transfer to firmware
- There is no indication that the game data is actually being transferred to the badge hardware.
- This PR seems to simulate UI logic or animations inside the app without pushing content to the badge. Hence it is incomplete in scope.
-
Missing end-to-end flow
- There needs to be a clear flow from user interaction to rendered output on the physical badge.
- The PR so far lacks integration with the existing rendering and Bluetooth transmission logic (usually handled via
badgePreview
,hexConverter
, andBluetoothService
).
-
No validation of hardware compatibility
- The badge display has "very limited resolution and input capacity". It is not clear whether the game logic respects those constraints or even attempts to function within them.
-
Missing documentation
- No user-facing explanation of what the feature does, how to activate it, or how it appears on the badge.
- No architectural notes on how this is supposed to fit into the data pipeline from app to badge.
-
Unclear or no testable outcome
- It is not possible to verify what to expect or test without a step by step description, screenshots, or expected output on the badge.
- If there is no test badge rendering or actual Bluetooth transmission triggered, the feature is incomplete.
-
Standalone logic without Iitegration
- The game may be implemented as a separate widget or screen, but it is not integrated into the badge content generation or save/export flow.
Next steps to make the PR mergeable
- Follow up on the issue you opened after our discussion a month ago in the firmware repository fossasia/badgemagic-firmware#91 and implement missing functionality to make the feature work
- Confirm that game output is rendered into a badge-compatible bitmap and translated to a 22-bit hex string.
- Ensure the generated game frames or state are ransferred to the badge over Bluetooth.
- Provide a demo or screenshot showing what users see and what is transmitted.
- Document the constraints and limitations clearly.
- Integrate game output into the existing badge preview/send workflow, or explain why it needs a separate flow.
🎯 What does this PR do?
This PR introduces two playable games – Snake and Tetris – into the app. It adds new UI screens, game logic providers, and navigation to allow users to access and interact with the games directly from the home screen.
🐛 Problem Solved
Fixes #1312 – Previously, the app had no game content. This PR adds built-in interactive gameplay to improve user engagement and app experience.
✨ Changes Made
SnakeGameProvider
– Handles game state, movement, and interactions for SnakeTetrisGameProvider
– Manages piece logic, collisions, and line-clearing for Tetrisgame_selection_screen.dart
– Allows user to choose between Snake and Tetrisgamescreen.dart
– Common layout for rendering game contenttetris_game_screen.dart
– Custom rendering logic and UI for Tetrishomescreen.dart
modified to include a route to the game selection screen📁 Files Modified
lib/view/homescreen.dart
– Added navigation to game selectionlib/view/game_selection_screen.dart
– UI for selecting which game to playlib/view/gamescreen.dart
– Shared layout for game viewslib/view/tetris_game_screen.dart
– Tetris-specific view and logiclib/providers/snake_game_provider.dart
– Snake game state logiclib/providers/tetris_game_provider.dart
– Tetris game state logic🧪 How to Test
Summary by Sourcery
Add two playable games (Snake and Tetris) into the app, integrate game navigation in HomeScreen, and improve badge saving and editing workflows with enhanced persistence and error handling.
New Features:
Enhancements: