Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -509,3 +509,4 @@ Thumbs.db
!/gradle/wrapper/gradle-wrapper.jar

# End of https://www.gitignore.io/api/java,swift,kotlin,android,flutter,intellij,objective-c,androidstudio
*.sh
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid a global *.sh ignore pattern.

Line 512 will ignore all shell scripts, including valid repo automation scripts (CI, release, tooling), which can silently block commits. Prefer ignoring only generated/script-specific paths.

Suggested fix
-*.sh
+# Keep shell scripts versioned by default; ignore only generated ones explicitly.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
*.sh
# Keep shell scripts versioned by default; ignore only generated ones explicitly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.gitignore at line 512, The .gitignore currently uses a global "*.sh"
pattern which unintentionally hides all shell scripts; replace this broad
pattern (the "*.sh" entry) with more specific ignores that target generated or
tooling script locations only (for example ignore a build or generated scripts
directory or specific filenames), or remove the entry entirely and list
individual generated script paths instead so repo automation and CI scripts
aren’t accidentally excluded; update the "*.sh" line accordingly to a scoped
path or explicit filenames.

18 changes: 18 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
.PHONY: pub-get

PUBSPEC_DIRS := $(sort $(dir $(wildcard packages/*/pubspec.yaml)))

## Run flutter pub get in all packages containing pubspec.yaml
pub-get:
@failed=""; \
for dir in $(PUBSPEC_DIRS); do \
echo "=> pub get: $$dir"; \
(cd $$dir && flutter pub get) || failed="$$failed $$dir"; \
done; \
echo ""; \
if [ -n "$$failed" ]; then \
echo "FAILED in:$$failed"; \
exit 1; \
else \
echo "All packages updated!"; \
fi
5 changes: 2 additions & 3 deletions packages/mediascanner/example/test/widget_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

import 'package:mediascanner_example/main.dart';
import '../lib/main.dart';

void main() {
testWidgets('Verify Platform version', (WidgetTester tester) async {
Expand All @@ -18,8 +18,7 @@ void main() {
// Verify that platform version is retrieved.
expect(
find.byWidgetPredicate(
(Widget widget) => widget is Text &&
widget.data.startsWith('Running on:'),
(Widget widget) => widget is Text && widget.data!.startsWith('Running on:'),
),
findsOneWidget,
);
Expand Down
5 changes: 2 additions & 3 deletions packages/migration/example/test/widget_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

import 'package:migration_example/main.dart';
import '../lib/main.dart';

void main() {
testWidgets('Verify Platform version', (WidgetTester tester) async {
Expand All @@ -18,8 +18,7 @@ void main() {
// Verify that platform version is retrieved.
expect(
find.byWidgetPredicate(
(Widget widget) => widget is Text &&
widget.data.startsWith('Running on:'),
(Widget widget) => widget is Text && widget.data!.startsWith('Running on:'),
),
findsOneWidget,
);
Expand Down
141 changes: 88 additions & 53 deletions packages/player/lib/src/player.dart
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import 'dart:async';
import 'dart:io';

import 'package:flutter/foundation.dart';
import 'package:smaws/aws.dart';
import 'package:flutter/services.dart';
import 'package:mutex/mutex.dart';
import 'package:smaws/aws.dart';
import 'package:smplayer/src/before_play_event.dart';
import 'package:smplayer/src/duration_change_event.dart';
import 'package:smplayer/src/event.dart';
import 'package:smplayer/src/event_type.dart';
import 'package:smplayer/src/isar_service.dart';
import 'package:smplayer/src/media.dart';
import 'package:smplayer/src/duration_change_event.dart';
import 'package:smplayer/src/position_change_event.dart';
import 'package:smplayer/src/previous_playlist_model.dart';
import 'package:smplayer/src/queue.dart';
import 'package:smplayer/src/repeat_mode.dart';
import 'package:mutex/mutex.dart';

import 'player_state.dart';

Expand Down Expand Up @@ -66,7 +67,7 @@ class Player {
EventType.PAUSED,
EventType.PLAYING,
EventType.EXTERNAL_RESUME_REQUESTED,
EventType.EXTERNAL_PAUSE_REQUESTED
EventType.EXTERNAL_PAUSE_REQUESTED,
];

Stream<Event>? _stream;
Expand Down Expand Up @@ -102,29 +103,36 @@ class Player {
}

Future<int> enqueue(
Media media,
) async {
_queue.add(media);
Media media, [
bool enqueueAfterCurrent = false,
]) async {
_queue.add(media, enqueueAfterCurrent);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check the Queue.add and Queue.addAll signatures
echo "=== Checking Queue.add/addAll signatures ==="
rg -n --type=dart '^\s*(Future|async).*\s+(add|addAll)\s*\(' packages/player/lib/src/queue.dart -B1 -A5

echo -e "\n=== Checking Player._queue.add/addAll calls ==="
rg -n --type=dart '_queue\.(add|addAll)\s*\(' packages/player/lib/src/player.dart -B2 -A2

Repository: SuaMusica/flutter_plugins

Length of output: 446


🏁 Script executed:

#!/bin/bash
# Get file sizes to determine if we should read them entirely
echo "=== File sizes ==="
wc -l packages/player/lib/src/queue.dart packages/player/lib/src/player.dart

echo -e "\n=== Check specific lines 109 and 119 in player.dart ==="
sed -n '105,125p' packages/player/lib/src/player.dart

Repository: SuaMusica/flutter_plugins

Length of output: 736


🏁 Script executed:

#!/bin/bash
# Search for Queue class and its add/addAll method definitions
echo "=== Full search for add/addAll methods in Queue ==="
ast-grep --pattern 'class Queue {
  $$$
  $_* add$_* $$$
  $$$
}'

ast-grep --pattern 'class Queue {
  $$$
  $_* addAll$_* $$$
  $$$
}'

Repository: SuaMusica/flutter_plugins

Length of output: 120


🏁 Script executed:

#!/bin/bash
# Read queue.dart to see the full implementation
cat -n packages/player/lib/src/queue.dart

Repository: SuaMusica/flutter_plugins

Length of output: 14415


🏁 Script executed:

#!/bin/bash
# Search for add and addAll method definitions specifically
echo "=== Searching for add method definition ==="
rg -n 'add\s*\(' packages/player/lib/src/queue.dart -B1 -A3

echo -e "\n=== Searching for addAll method definition ==="
rg -n 'addAll\s*\(' packages/player/lib/src/queue.dart -B1 -A3

Repository: SuaMusica/flutter_plugins

Length of output: 1023


🏁 Script executed:

#!/bin/bash
# Verify the _save method signature to confirm persistence is truly async
rg -n '_save\(' packages/player/lib/src/queue.dart -B2 -A8

Repository: SuaMusica/flutter_plugins

Length of output: 1076


Add await to queue persistence calls to prevent silent failures.

Queue.add() and Queue.addAll() are async methods that internally await _save(), which performs I/O with IsarService. Calling these without await at lines 109 and 119 creates a fire-and-forget pattern where persistence errors are silently ignored and callers receive success before writes complete.

Suggested fix
-    _queue.add(media, enqueueAfterCurrent);
+    await _queue.add(media, enqueueAfterCurrent);
-    _queue.addAll(
+    await _queue.addAll(
       items,
       shouldRemoveFirst: shouldRemoveFirst,
       saveOnTop: saveOnTop,
       enqueueAfterCurrent: enqueueAfterCurrent,
     );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/player/lib/src/player.dart` at line 109, The calls to the async
persistence methods _queue.add and _queue.addAll are currently invoked without
awaiting, causing fire-and-forget writes and silent failures; update the Player
methods that call _queue.add(media, enqueueAfterCurrent) and
_queue.addAll(mediaList, enqueueAfterCurrent) to await those calls (and mark the
caller async if not already) so that the internal _save (which uses IsarService)
completes or surfaces errors to callers instead of returning immediately.

return Ok;
}

Future<int> enqueueAll(
List<Media> items, {
bool shouldRemoveFirst = false,
bool saveOnTop = false,
bool enqueueAfterCurrent = false,
}) async {
_queue.addAll(
items,
shouldRemoveFirst: shouldRemoveFirst,
saveOnTop: saveOnTop,
enqueueAfterCurrent: enqueueAfterCurrent,
);
return Ok;
}

int removeByPosition(
{required List<int> positionsToDelete, required bool isShuffle}) {
int removeByPosition({
required List<int> positionsToDelete,
required bool isShuffle,
}) {
return _queue.removeByPosition(
positionsToDelete: positionsToDelete, isShuffle: isShuffle);
positionsToDelete: positionsToDelete,
isShuffle: isShuffle,
);
}

Future<int> removeAll() async {
Expand Down Expand Up @@ -216,8 +224,11 @@ class Player {
return media;
}

Future<int> reorder(int oldIndex, int newIndex,
[bool isShuffle = false]) async {
Future<int> reorder(
int oldIndex,
int newIndex, [
bool isShuffle = false,
]) async {
_queue.reorder(oldIndex, newIndex, isShuffle);
return Ok;
}
Expand All @@ -241,9 +252,9 @@ class Player {
Media? get top => _queue.top;

Future<int> load(Media media) async => _doPlay(
_queue.current!,
shouldLoadOnly: true,
);
_queue.current!,
shouldLoadOnly: true,
);
Comment on lines 254 to +257
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

load(Media media) ignores its media parameter.

Line 255 loads _queue.current! instead of the method argument, which can load the wrong track (or throw when current is null).

🔧 Suggested fix
-  Future<int> load(Media media) async => _doPlay(
-    _queue.current!,
+  Future<int> load(Media media) async => _doPlay(
+    media,
     shouldLoadOnly: true,
   );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Future<int> load(Media media) async => _doPlay(
_queue.current!,
shouldLoadOnly: true,
);
_queue.current!,
shouldLoadOnly: true,
);
Future<int> load(Media media) async => _doPlay(
media,
shouldLoadOnly: true,
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/player/lib/src/player.dart` around lines 254 - 257, The load(Media
media) method currently ignores its media parameter and calls
_doPlay(_queue.current!, shouldLoadOnly: true), which can load the wrong track
or throw if _queue.current is null; change the call in load to pass the media
argument to _doPlay (i.e. _doPlay(media, shouldLoadOnly: true)) and ensure any
callers or _doPlay signature support a Media argument (update _doPlay parameter
types or overload if needed) and add a null-safety check if _doPlay can accept
nulls.


Future<int> play(
Media media, {
Expand Down Expand Up @@ -333,7 +344,7 @@ class Player {
'position': position?.inMilliseconds,
'respectSilence': respectSilence,
'stayAwake': stayAwake,
'isFavorite': media.isFavorite
'isFavorite': media.isFavorite,
});
} else if (autoPlay) {
_notifyBeforePlayEvent((loadOnly) => {});
Expand All @@ -352,7 +363,7 @@ class Player {
'position': position?.inMilliseconds,
'respectSilence': respectSilence,
'stayAwake': stayAwake,
'isFavorite': media.isFavorite
'isFavorite': media.isFavorite,
});
} else {
_notifyBeforePlayEvent((loadOnly) {
Expand All @@ -370,7 +381,7 @@ class Player {
'position': position?.inMilliseconds,
'respectSilence': respectSilence,
'stayAwake': stayAwake,
'isFavorite': media.isFavorite
'isFavorite': media.isFavorite,
});
});

Expand Down Expand Up @@ -694,14 +705,16 @@ class Player {

case PlayerState.ERROR:
final error = callArgs['error'] ?? "Unknown from Source";
final isPermissionError =
(error as String).contains('Permission denied');
final isPermissionError = (error as String).contains(
'Permission denied',
);
_notifyPlayerErrorEvent(
player: player,
error: error,
errorType: isPermissionError
? PlayerErrorType.PERMISSION_DENIED
: null);
player: player,
error: error,
errorType: isPermissionError
? PlayerErrorType.PERMISSION_DENIED
: null,
);
break;
}

Expand Down Expand Up @@ -773,7 +786,10 @@ class Player {
case 'externalPlayback.play':
print("Player: externalPlayback : Play");
_notifyPlayerStateChangeEvent(
player, EventType.EXTERNAL_RESUME_REQUESTED, "");
player,
EventType.EXTERNAL_RESUME_REQUESTED,
"",
);
break;
case 'externalPlayback.pause':
print("Player: externalPlayback : Pause");
Expand All @@ -800,61 +816,77 @@ class Player {

_notifyChangeToNext(Media media) {
_add(
Event(type: EventType.NEXT, media: media, queuePosition: _queue.index));
Event(type: EventType.NEXT, media: media, queuePosition: _queue.index),
);
}

_notifyChangeToPrevious(Media media) {
_add(Event(
type: EventType.PREVIOUS, media: media, queuePosition: _queue.index));
_add(
Event(
type: EventType.PREVIOUS,
media: media,
queuePosition: _queue.index,
),
);
}

_notifyRewind(Media media) async {
final positionInMilli = await getCurrentPosition();
final durationInMilli = await getDuration();
_add(Event(
type: EventType.REWIND,
media: media,
queuePosition: _queue.index,
position: Duration(milliseconds: positionInMilli),
duration: Duration(milliseconds: durationInMilli),
));
_add(
Event(
type: EventType.REWIND,
media: media,
queuePosition: _queue.index,
position: Duration(milliseconds: positionInMilli),
duration: Duration(milliseconds: durationInMilli),
),
);
}

_notifyForward(Media media) async {
final positionInMilli = await getCurrentPosition();
final durationInMilli = await getDuration();

_add(Event(
type: EventType.FORWARD,
media: media,
queuePosition: _queue.index,
position: Duration(milliseconds: positionInMilli),
duration: Duration(milliseconds: durationInMilli),
));
_add(
Event(
type: EventType.FORWARD,
media: media,
queuePosition: _queue.index,
position: Duration(milliseconds: positionInMilli),
duration: Duration(milliseconds: durationInMilli),
),
);
}

_notifyPlayerStatusChangeEvent(EventType type) {
if (_queue.current != null) {
_add(Event(
type: type, media: _queue.current!, queuePosition: _queue.index));
_add(
Event(type: type, media: _queue.current!, queuePosition: _queue.index),
);
}
}

_notifyBeforePlayEvent(Function(bool) operation) {
_add(BeforePlayEvent(
_add(
BeforePlayEvent(
media: _queue.current!,
queuePosition: _queue.index,
operation: operation));
operation: operation,
),
);
}

static _notifyDurationChangeEvent(Player player, Duration newDuration) {
if (player._queue.current != null) {
_addUsingPlayer(
player,
DurationChangeEvent(
media: player._queue.current!,
queuePosition: player._queue.index,
duration: newDuration));
player,
DurationChangeEvent(
media: player._queue.current!,
queuePosition: player._queue.index,
duration: newDuration,
),
);
}
}

Expand Down Expand Up @@ -902,7 +934,10 @@ class Player {
}

static _notifyPositionChangeEvent(
Player player, Duration newPosition, Duration newDuration) {
Player player,
Duration newPosition,
Duration newDuration,
) {
final media = player.current;
if (media != null) {
final position = newPosition.inSeconds;
Expand Down
Loading