Migrate to Null Safety Flutter V2 by mitchross · Pull Request #205 · solid-software/flutter_vlc_player

mitchross · 2021-03-16T17:05:04Z

Testing Locally

Change Pubspec
flutter_vlc_player_platform_interface:
path: ../flutter_vlc_player_platform_interface

Migration Strategy

Ran 'Dart Migrate' in each folder
Manually migrated Pigeon upgrade path leveraging [pigeon] migrate to null-safety flutter/packages#300
Manually Fixed Java classes that Pigeon leverages for created auto generated files.

mitchross · 2021-03-16T17:05:37Z

@solid-software

solid-software · 2021-03-16T17:37:36Z

Thanks @mitchross - we'll take a look!

solid-yuriiprykhodko

Great work!
Some minor suggestions added.

I think it makes sense to establish a practice for dealing with nullable variables.
I believe a force-unwrap (variable!) should only occur after the following:

A non-null assertion.

assert(variable !=null, "This most certainly shouldn't be null!");
variable!.doSmth();

A null-check

if (variable !=null) {
variable!.doSmth();
} else {
// fallback
}

Explicit assignment to the variable:

variable =SurelyNotNull();
variable!.doSmth();

Let's make sure we don't have any stray force-unwraps in the code.

solid-yuriiprykhodko · 2021-03-17T11:04:27Z

+ int? get viewId => _viewId;

 /// The viewId for this controller
- int _viewId;
+ int? _viewId;


Not really sure this has to be nullable.
In any case the code below force-unwraps this variable, and will cause a runtime exception if it happens to be null.
We could:
Add a non-null assertion to the getter above, and return int instead of int?.
Make the variable late final.
I'm personally more in favour of option 1, since it feels more runtime-safe.

Went with late init since the getter is used for testing only. The _viewId comes from the platform itself via
Future onPlatformViewCreated(int viewId) async

solid-yuriiprykhodko · 2021-03-17T11:08:51Z

 _creatingCompleter = Completer<void>();

 await vlcPlayerPlatform.create(
- viewId: _viewId,
+ viewId: _viewId!,
 uri: dataSource,
 type: dataSourceType,
 package: package,
- hwAcc: hwAcc ?? HwAcc.AUTO,
- autoPlay: autoPlay ?? true,
+ hwAcc: hwAcc,
+ autoPlay: autoPlay,
 options: options,
 );

- _creatingCompleter.complete(null);
+ _creatingCompleter!.complete(null);


We could probably avoid using a Completer here.
Given Future<void>? _viewCreationFuture; is a field in the enclosing class.
_viewCreationFuture = vlcPlayerPlatform.create( viewId: _viewId, uri: dataSource, type: dataSourceType, package: package, hwAcc: hwAcc, autoPlay: autoPlay, options: options, ); await _viewCreationFuture;
The future can be awaited elsewhere, if necessary.

solid-yuriiprykhodko · 2021-03-17T11:12:56Z

 isPlaying: event.isPlaying,
 playingState:
- event.isPlaying ? PlayingState.playing : value.playingState,
+ event.isPlaying! ? PlayingState.playing : value.playingState,


Can we be absolutely sure we're not getting a null here?

solid-yuriiprykhodko · 2021-03-17T11:15:32Z

+late VoidCallback _listener;

- bool _isInitialized;
+ bool? _isInitialized;


Can be non-nullable:
Suggested change
bool? _isInitialized;
bool _isInitialized=false;

mitchross · 2021-03-17T13:47:52Z

Great work!
Some minor suggestions added.
I think it makes sense to establish a practice for dealing with nullable variables.
I believe a force-unwrap (variable!) should only occur after the following:
A non-null assertion.
assert(variable !=null, "This most certainly shouldn't be null!");
variable!.doSmth();
A null-check
if (variable !=null) {
variable!.doSmth();
} else {
// fallback
}
Explicit assignment to the variable:
variable =SurelyNotNull();
variable!.doSmth();
Let's make sure we don't have any stray force-unwraps in the code.

@solid-yuriiprykhodko
No worries, most of the code was auto-generated by the migration tool based on the talk at Flutter Engage. https://youtu.be/yll3SNXvQCw?t=3739
I'll get these fixes in this week, thanks for the review!

mitchross · 2021-03-22T03:09:00Z

@solid-software any more changes needed?

solid-software · 2021-03-22T09:50:28Z

@solid-yuriiprykhodko - do you mind giving it another look?

solid-yuriiprykhodko

Great work on suggestions!
One change requested, after that we should be good to merge.

Co-authored-by: Yuri Prykhodko <40931732+solid-yuriiprykhodko@users.noreply.github.com>

mitchross · 2021-03-22T15:42:14Z

Great work on suggestions!
One change requested, after that we should be good to merge.

@solid-pavloprykhodko resolved!

For the merge I think because the platform interface package version changed you need to publish that first then the pubsec will resolve.

solid-software · 2021-03-22T18:32:01Z

thanks @mitchross - amazing contribution!

NaarGes · 2021-08-17T12:30:35Z

Why it doesn't have null-safe tag on pub.dev?

NaarGes · 2021-08-17T12:31:11Z

I got this error

Error: Null safety features are disabled for this library.
Try removing the package language version or setting the language version to 2.12 or higher.
late VlcPlayerController _videoPlayerController;

solid-software · 2021-08-17T12:41:34Z

@NaarGes please try again, it should be fixed now

mitchross added 5 commits March 14, 2021 01:54

platform interface upgrade
c190f9d

pre-pigeon
f14e893

convert pigeon
5785cc5

Fix snapshot issue
8586ac1

Pr prep
7d1b830

mitchross mentioned this pull request Mar 16, 2021
Support flutter 2.0 #198
Open

solid-yuriiprykhodko suggested changes Mar 17, 2021
View reviewed changes

mitchross added 2 commits March 18, 2021 22:44

PR comments
3bafb78

Pr comments
5be239c

solid-yuriiprykhodko suggested changes Mar 22, 2021
View reviewed changes

Comment threadflutter_vlc_player/lib/src/vlc_player_controller.dart Outdated

Update flutter_vlc_player/lib/src/vlc_player_controller.dart
978de72
Co-authored-by: Yuri Prykhodko <40931732+solid-yuriiprykhodko@users.noreply.github.com>

solid-yuriiprykhodko approved these changes Mar 22, 2021
View reviewed changes

solid-software merged commit 6011761 into solid-software:masterMar 22, 2021

Uh oh!

Conversation

mitchross commented Mar 16, 2021

Uh oh!

mitchross commented Mar 16, 2021

Uh oh!

solid-software commented Mar 16, 2021

Uh oh!

solid-yuriiprykhodko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchross commented Mar 17, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mitchross commented Mar 22, 2021

Uh oh!

solid-software commented Mar 22, 2021

Uh oh!

solid-yuriiprykhodko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mitchross commented Mar 22, 2021

Uh oh!

solid-software commented Mar 22, 2021

Uh oh!

NaarGes commented Aug 17, 2021

Uh oh!

NaarGes commented Aug 17, 2021• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solid-software commented Aug 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mitchross commented Mar 17, 2021•
edited
Loading

NaarGes commented Aug 17, 2021•
edited
Loading