migrate to null safety #57
Conversation
There was a problem hiding this comment.
First of all: thank you for all the time and effort you put into this PR. I really welcome your job. It would me take much more time to try to push the null-safety into the codebase without your help.
💪
I've left some suggestions and concerns about changes you've prepared. Could you take a look and check if you agree with them?
| defaultConfig { | ||
| // TODO: Specify your own unique Application ID (https://developer.android.com/studio/build/application-id.html). | ||
| applicationId "pl.ukaszapps.soundpoolexample" | ||
| applicationId "com.example.example" |
There was a problem hiding this comment.
I'd rather have old version here
| applicationId "com.example.example" | |
| applicationId "pl.ukaszapps.soundpoolexample" |
| package="pl.ukaszapps.soundpoolexample"> | ||
|
|
||
| <!-- The INTERNET permission is required for development. Specifically, | ||
| flutter needs it to communicate with the running application | ||
| to allow setting breakpoints, to provide hot reload, etc. | ||
| --> | ||
| <uses-permission android:name="android.permission.INTERNET"/> | ||
|
|
||
| <!-- io.flutter.app.FlutterApplication is an android.app.Application that | ||
| calls FlutterMain.startInitialization(this); in its onCreate method. | ||
| In most cases you can leave this as-is, but you if you want to provide | ||
| additional functionality it is fine to subclass or reimplement | ||
| FlutterApplication and put your custom class here. --> | ||
| <application | ||
| android:name="io.flutter.app.FlutterApplication" | ||
| android:label="soundpool_example" | ||
| package="com.example.example"> |
There was a problem hiding this comment.
I'd prefer to keep old package name and permissions here and remove debug version of AndroidManifest.xml.
| @@ -0,0 +1,6 @@ | |||
| package com.example.example | |||
There was a problem hiding this comment.
Again: please keep packages from previous version
| @@ -0,0 +1,7 @@ | |||
| <manifest xmlns:android="http://schemas.android.com/apk/res/android" | |||
| @@ -1,41 +0,0 @@ | |||
| # Uncomment this line to define a global platform for your project | |||
There was a problem hiding this comment.
I think this is removed by mistake but probably it would be generated again if example would be run on iOS. I'd fix this if any problem occurs.
| int _cheeringStreamId = -1; | ||
|
|
||
| String get _cheeringUrl => kIsWeb ? '/c-c-1.mp3' : 'https://raw.githubusercontent.com/ukasz123/soundpool/feature/web_support/example/web/c-c-1.mp3'; | ||
|
|
||
| void initState(){ | ||
| _soundId = _loadSound(); | ||
| _cheeringId = _loadCheering(); | ||
| super.initState(); |
| late Map<Soundpool, SoundsMap> soundsMap; | ||
| Soundpool? _selectedPool; |
There was a problem hiding this comment.
This leads to multiple bang (!) operators across the code. I believe better solution would be to split this widget into two: _MyAppState - which handles preparing soundpools, soundsMap and _selectedPool and ReadyWidget which would receive all of these non-null and do playing and stopping sounds.
What do you think?
| @@ -0,0 +1,106 @@ | |||
| cmake_minimum_required(VERSION 3.10) | |||
There was a problem hiding this comment.
No support for Linux (and Windows) so files for these platforms should be removed.
soundpool/pubspec.yaml
Outdated
|
|
||
| dependencies: | ||
| flutter: | ||
| sdk: flutter | ||
|
|
||
| soundpool_platform_interface: ^1.0.1 | ||
| soundpool_platform_interface: | ||
| path : ../soundpool_platform_interface |
There was a problem hiding this comment.
This is fine during development but it cannot reach pub. New versions 2.0.0-nullsafety.0 should be set everywhere.
I already committed version changes
soundpool_web/lib/soundpool_web.dart
Outdated
| @@ -211,18 +211,15 @@ class _CachedAudioSettings { | |||
| double volumeLeft; | |||
| double volumeRight; | |||
| _CachedAudioSettings( | |||
| {this.buffer, this.volumeLeft = 1.0, this.volumeRight = 1.0}) | |||
| {required this.buffer, this.volumeLeft = 1.0, this.volumeRight = 1.0}) | |||
| : assert(buffer != null); | |||
There was a problem hiding this comment.
| : assert(buffer != null); | |
| ; |
migrate to null safety
migrate all 4 package
migrate example
recreate example ( create new project and copy old code to it )
tested on android