feat: Added the updated base structure for handling the animations #1012

Jhalakupadhyay · 2024-09-22T09:22:26Z

Summary by Sourcery

Add new utility functions for byte and binary data conversion, introduce a new provider for managing badge animations and drawing, and refactor existing components to integrate with the new provider structure. This update enhances the animation handling capabilities and improves the modularity of the badge rendering system.

New Features:

Introduce a new utility function to convert byte arrays to binary arrays and binary strings to 2D lists, enhancing data manipulation capabilities.
Add a new badgeAnimation function in the Converters class to handle badge animations based on message input.
Implement a new DrawBadgeProvider class to manage badge drawing and animation states, replacing the previous DrawBadgeProvider implementation.

Enhancements:

Refactor the BadgeWidget to accept a grid parameter, allowing for more flexible badge rendering.
Update the HomeScreen to initialize and manage animations using the new DrawBadgeProvider.

Chores:

Replace the drawbadge_provider.dart with badgeview_provider.dart across multiple files to align with the new provider structure.

…badge. feat: Added the modes support for left right top down fixed and animation support for flash and marquee. animation right

…badge. feat: Added the modes support for left right top down fixed and animation support for flash and marquee. animation right feat:Added the base structure for the animations with speed controll.

sourcery-ai · 2024-09-22T09:22:31Z

Reviewer's Guide by Sourcery

This pull request implements a new base structure for handling animations in the BadgeMagic app. It introduces new utility functions for converting data formats, updates the UI to support animations, and adds new providers and abstract classes for managing badge animations and effects.

File-Level Changes

Change	Details	Files
Added new utility functions for data conversion	Implemented byteArrayToBinaryArray function Added hexToBin function Created binaryStringTo2DList function	`lib/bademagic_module/utils/byte_array_utils.dart`
Updated HomeScreen to support animations	Added textNotifier and textfieldLength variables Implemented _controllerListner function Updated initState to initialize animation	`lib/view/homescreen.dart`
Added badgeAnimation function to Converters class	Implemented badgeAnimation function to convert message to binary grid Updated messageTohex function	`lib/bademagic_module/utils/converters.dart`
Added animation speed constants and calculation function	Defined constants for animation speeds Implemented aniSpeedStrategy function	`lib/constants.dart`
Refactored BadgeWidget to use grid prop	Updated BadgeWidget to accept grid as a prop Modified _BMBadgeState to use Provider for grid data	`lib/virtualbadge/widgets/badge_widget.dart` `lib/virtualbadge/view/draw_badge.dart`
Created new BadgeViewProvider	Implemented DrawBadgeProvider class Added functions for managing grid state and animations	`lib/providers/badgeview_provider.dart`
Added abstract classes for animations and effects	Created BadgeAnimation abstract class Implemented BadgeEffect abstract class	`lib/badge_animation/animation_abstract.dart` `lib/badge_effect/badgeeffectabstract.dart`

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @Jhalakupadhyay - I've reviewed your changes - here's some feedback:

Overall Comments:

Consider adding more inline comments to explain complex logic, particularly in new utility functions like binaryStringTo2DList.
Review the debug logging statements (e.g., logger.d) and consider removing them or implementing a way to disable them in production builds.

Here's what I looked at during the review

🟡 General issues: 5 issues found
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-09-22T09:23:45Z

lib/bademagic_module/utils/byte_array_utils.dart

+    binaryArray[rowIndex].addAll(binaryRepresentation);
+
+    rowIndex = (rowIndex + 1) % 11;
+  }


suggestion: Simplify nested loop structure in binaryStringTo2DList

The current nested loop structure is complex and hard to follow. Consider simplifying it by using integer division and modulo operations to determine the row and column for each bit.

List<List<int>> binary2DList = List.generate(maxHeight, (_) => []); for (int i = 0; i < binaryString.length; i++) { int row = i ~/ 8; int col = i % 8; if (row < maxHeight) { binary2DList[row].add(int.parse(binaryString[i])); } }

sourcery-ai · 2024-09-22T09:23:45Z

lib/view/homescreen.dart

@@ -26,6 +29,7 @@ class HomeScreen extends StatefulWidget {
 }

 class _HomeScreenState extends State<HomeScreen> with TickerProviderStateMixin {
+  final ValueNotifier<String> textNotifier = ValueNotifier<String>('');


suggestion: Remove unused textNotifier

This ValueNotifier is declared but never used. Consider removing it if it's not needed for future functionality.

Suggested change

final ValueNotifier<String> textNotifier = ValueNotifier<String>('');

// Remove the following line:

// final ValueNotifier<String> textNotifier = ValueNotifier<String>('');

sourcery-ai · 2024-09-22T09:23:45Z

lib/view/homescreen.dart

+  void _controllerListner() {
+    logger
+        .d('Controller Listener : ${inlineImageProvider.getController().text}');
+    converters.badgeAnimation(inlineImageProvider.getController().text.isEmpty
+        ? ""
+        : inlineImageProvider.getController().text);
+    inlineImageProvider.controllerListener();
+  }


suggestion (performance): Optimize badge animation updates

The badgeAnimation call on every text change could be inefficient for longer inputs. Consider debouncing this operation or updating only when the text input is complete.

void _controllerListener() { final text = inlineImageProvider.getController().text; logger.d('Controller Listener: $text'); if (_debounce?.isActive ?? false) _debounce!.cancel(); _debounce = Timer(const Duration(milliseconds: 300), () { converters.badgeAnimation(text.isEmpty ? "" : text); }); inlineImageProvider.controllerListener(); }

sourcery-ai · 2024-09-22T09:23:45Z

lib/constants.dart

+int aniSpeedStrategy(int speedLevel) {
+  int speedInMicroseconds = aniBaseSpeed.inMicroseconds -
+      (speedLevel * aniBaseSpeed.inMicroseconds ~/ 8);
+  return speedInMicroseconds;
+}


suggestion: Clarify animation speed calculation

The speed calculation seems arbitrary. Consider adding a comment explaining the logic behind this calculation or explore a more nuanced approach that might provide better control over animation speeds.

Suggested change

int aniSpeedStrategy(int speedLevel) {

int speedInMicroseconds = aniBaseSpeed.inMicroseconds -

(speedLevel * aniBaseSpeed.inMicroseconds ~/ 8);

return speedInMicroseconds;

}

int aniSpeedStrategy(int speedLevel) {

final maxSpeedReduction = aniBaseSpeed.inMicroseconds * 7 ~/ 8;

final speedReduction = (speedLevel * maxSpeedReduction) ~/ 8;

return aniBaseSpeed.inMicroseconds - speedReduction;

}

sourcery-ai · 2024-09-22T09:23:45Z

lib/providers/badgeview_provider.dart

+  Map<int, BadgeAnimation?> animationMap = {
+    // 0: LeftAnimation(),
+    // 1: RightAnimation(),
+    // 2: UpAnimation(),
+    // 3: DownAnimation(),
+    // 4: FixedAnimation(),
+    // 5: SnowFlakeAnimation(),
+    // 6: PictureAnimation(),
+    // 7: AniAnimation(),
+    // 8: LaserAnimation(),


issue (bug_risk): Implement missing animations

The animation map is currently empty and commented out. This suggests that the animation system is not fully implemented, which could lead to runtime errors or unexpected behavior. Prioritize implementing these animations or provide a default fallback.

adityastic

Nice comments from sourcery to look at

adityastic · 2024-09-22T09:29:35Z

lib/bademagic_module/utils/byte_array_utils.dart

@@ -28,3 +28,56 @@ List<int> hexStringToByteArray(String hexString) {
  logger.d(data.length);
  return data;
 }
+
+List<List<int>> byteArrayToBinaryArray(List<int> byteArray) {
+  List<List<int>> binaryArray = List.generate(11, (_) => []);


Use badge height

adityastic · 2024-09-23T16:20:21Z

lib/bademagic_module/utils/converters.dart

+          List.generate(11, (i) => List.generate(44, (j) => 0));
+      badgeList.setNewGrid(image);
+    } else {
+      List<String> hexStrings = await messageTohex(message);


We should avoid these 3 operations and decide a scheme to store messages to one format which can be used across the app. List<List> would be a good candidate. It does not make sense to have these translations for every step when we could built our logic just on one kind of data

But in order to get the bitmap (List<List>) we would first need to have the hexstring and then convert it to bitmap.
Can you clarify what you mean by avoiding the operations.

Hex String -> Byte Array, Byte Array -> Binary Array, Binary Array -> Bool Array. Why not Hex String -> Bool Array

I have not seen this conversion so far is it possible?

Give me some time I will look into it and if I can make it then it will be very efficient.

So we're trying to minimize the number of conversions. HexString to something you can work off of everywhere. Maybe 2 conversions is fine but not more

So HexString -> ByteArray is fine. Then maybe ByteArray -> Bool Array when needed.

I don't see a point in holding up 4 bytes of memory whereas you can get away with just a bit

Correction, flutter uses 8 bytes int so more memory usage for no reason

adityastic · 2024-09-23T16:21:25Z

lib/providers/badgeview_provider.dart

+  List<List<bool>> homeViewGrid =
+      List.generate(11, (i) => List.generate(44, (j) => false));
+
+  //List that contains the state of each cell of the badge for draw view
+  List<List<bool>> drawViewGrid =
+      List.generate(11, (i) => List.generate(44, (j) => false));
+


This needs to be cleaned. 2 seperate widgets for draw and display seperately

this I will clear in seperate PR where I will add separate custom paint class for the drawbadge

…rom the hexString to boolean 2D list.

…ic-android into flutter_app

Jhalakupadhyay and others added 6 commits September 8, 2024 01:42

feat: Added the logic to display the clipart and text on the virtual …

bcdb92c

…badge. feat: Added the modes support for left right top down fixed and animation support for flash and marquee. animation right

feat:Added the base structure for the animations with speed controll.

85da751

chore: update flutter version to 3.24.2 from 3.22.2.

4be7e17

feat: Added the updated base structure for executing the animations.

e2de4fa

feat: Added the logic to display the clipart and text on the virtual …

8cea27f

…badge. feat: Added the modes support for left right top down fixed and animation support for flash and marquee. animation right feat:Added the base structure for the animations with speed controll.

Merge branch 'fossasia:flutter_app' into flutter_app

9f00cc5

Jhalakupadhyay requested a review from adityastic September 22, 2024 09:22

sourcery-ai bot reviewed Sep 22, 2024

View reviewed changes

Jhalakupadhyay removed the request for review from adityastic September 22, 2024 09:40

Jhalakupadhyay added 2 commits September 22, 2024 15:11

chore: update flutter version to 3.24.2 from 3.22.2.

899738c

feat: Added the updated base structure for executing the animations.

e234707

adityastic requested changes Sep 23, 2024

View reviewed changes

Jhalakupadhyay and others added 3 commits September 27, 2024 23:30

feat: updated the badgeAnimation method to have a direct conversion f…

a3607ca

…rom the hexString to boolean 2D list.

Merge branch 'fossasia:flutter_app' into flutter_app

777be21

Merge branch 'flutter_app' of github.personal:Jhalakupadhyay/badgemag…

2eacdcd

…ic-android into flutter_app

Jhalakupadhyay requested a review from adityastic September 27, 2024 18:06

Merge branch 'flutter_app' of github.personal:Jhalakupadhyay/badgemag…

0d65bc7

…ic-android into flutter_app

Jhalakupadhyay force-pushed the flutter_app branch 2 times, most recently from ae0b572 to 6f16c2a Compare September 28, 2024 19:20

chore: removed merge conflicts.

7ef4c74

Jhalakupadhyay force-pushed the flutter_app branch from 6f16c2a to 7ef4c74 Compare September 28, 2024 19:22

adityastic approved these changes Sep 29, 2024

View reviewed changes

adityastic merged commit 461c414 into fossasia:flutter_app Sep 29, 2024
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added the updated base structure for handling the animations #1012

feat: Added the updated base structure for handling the animations #1012

Jhalakupadhyay commented Sep 22, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Sep 22, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Sep 22, 2024

sourcery-ai bot Sep 22, 2024

sourcery-ai bot Sep 22, 2024

sourcery-ai bot Sep 22, 2024

sourcery-ai bot Sep 22, 2024

adityastic left a comment

adityastic Sep 22, 2024

JhalakUpadhyay7 Sep 23, 2024

adityastic Sep 23, 2024

JhalakUpadhyay7 Sep 23, 2024

adityastic Sep 23, 2024

JhalakUpadhyay7 Sep 23, 2024 •

edited

Loading

JhalakUpadhyay7 Sep 23, 2024

adityastic Sep 23, 2024

adityastic Sep 23, 2024

adityastic Sep 23, 2024

adityastic Sep 23, 2024

adityastic Sep 23, 2024

JhalakUpadhyay7 Sep 23, 2024

	final ValueNotifier<String> textNotifier = ValueNotifier<String>('');
	// Remove the following line:
	// final ValueNotifier<String> textNotifier = ValueNotifier<String>('');

feat: Added the updated base structure for handling the animations #1012

feat: Added the updated base structure for handling the animations #1012

Conversation

Jhalakupadhyay commented Sep 22, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

sourcery-ai bot commented Sep 22, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Sep 22, 2024

Choose a reason for hiding this comment

sourcery-ai bot Sep 22, 2024

Choose a reason for hiding this comment

sourcery-ai bot Sep 22, 2024

Choose a reason for hiding this comment

sourcery-ai bot Sep 22, 2024

Choose a reason for hiding this comment

sourcery-ai bot Sep 22, 2024

Choose a reason for hiding this comment

adityastic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JhalakUpadhyay7 Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jhalakupadhyay commented Sep 22, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Sep 22, 2024 •

edited

Loading

JhalakUpadhyay7 Sep 23, 2024 •

edited

Loading