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

bugfix - 228 - add setReleaseMode method for manage player state #269

Merged
merged 2 commits into from
Dec 2, 2024

Conversation

himanshuGandhiSimform
Copy link
Collaborator

BugFix - 228 - Add setRelease Mode as per the requirement

@himanshuGandhiSimform himanshuGandhiSimform linked an issue Feb 20, 2024 that may be closed by this pull request
Copy link
Collaborator

@ujas-m-simformsolutions ujas-m-simformsolutions left a comment

Choose a reason for hiding this comment

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

Some files look unformatted in native android and ios. Please format those

/// This enum is meant to be used as a parameter of setReleaseMode method.
///
/// It represents the behavior of AudioPlayer when an audio is finished or stopped.
enum ReleaseMode{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not change the name of the enum because then it will be a bigger breaking change

fun setReleaseMode(result: MethodChannel.Result, releaseModeType : Int?){
try{
releaseModeType?.let {
when (releaseModeType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks much better than previous

Copy link
Collaborator Author

@himanshuGandhiSimform himanshuGandhiSimform Feb 22, 2024

Choose a reason for hiding this comment

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

Thank you

///Release all resources, just like calling release method.
///
/// In Android, the media player is quite resource-intensive, and this will
/// let it go. Data will be buffered again when needed (if it's a remote file,
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't support remote files so remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or even better can replace this whole docs with ours

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also don't change the enum name

///Set the release Mode.
///
/// Check[FinishMode]'s doc to understand the difference between the modes.
Future<void> setReleaseMode(FinishMode finishMode)async{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make named parameter with default value and format

hemantbeast added a commit to hemantbeast/audio_waveforms that referenced this pull request May 28, 2024
lib/src/controllers/player_controller.dart Outdated Show resolved Hide resolved
lib/src/controllers/player_controller.dart Outdated Show resolved Hide resolved
lib/src/controllers/player_controller.dart Outdated Show resolved Hide resolved
pause,

///Stops player and disposes it(a PlayerController won't be disposed).
stop
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we removing this?

///Release all resources, just like calling release method.
///
/// In Android, the media player is quite resource-intensive, and this will
/// let it go. Data will be buffered again when needed (if it's a remote file,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also don't change the enum name

lib/src/base/audio_waveforms_interface.dart Outdated Show resolved Hide resolved
@@ -78,3 +73,9 @@ public extension RangeReplaceableCollection where Iterator.Element: ExpressibleB
self.init(repeating: 0, count: count)
}
}

enum FinishMode : Int{
case release = 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use same name as old flutter side name

ios/Classes/Utils.swift Outdated Show resolved Hide resolved
@himanshuGandhiSimform himanshuGandhiSimform force-pushed the fix/228/add_setReleaseMode branch 3 times, most recently from 9b33ed5 to 2befda3 Compare December 2, 2024 06:46
@himanshuGandhiSimform himanshuGandhiSimform force-pushed the fix/228/add_setReleaseMode branch 2 times, most recently from ea827a9 to 53d30c1 Compare December 2, 2024 07:47
Update the code as per reviewer suggestion
@ujas-m-simformsolutions ujas-m-simformsolutions merged commit 35f47d3 into main Dec 2, 2024
@ujas-m-simformsolutions ujas-m-simformsolutions deleted the fix/228/add_setReleaseMode branch December 2, 2024 08:16
@lhengl
Copy link

lhengl commented Dec 11, 2024

This merge is a breaking change due to the removal of finishMode as a parameter. Some packages that rely on this parameter now breaks (e.g. chatview). I suggest to add this back and call setFinishMode as a hotfix. This should be flagged with @deprecated instead so that it can be removed in the next major release. Most packages will not expect a breaking change without bumping the major version.

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.

Add setReleaseMode
3 participants