Rename .muted to .audioOn

The fact that this flag was negative (true when audio is off) lead to some confusion. Changing to `audioOn` standardizes the three audio-related settings options.

This also addresses some lingering issues around music: namely bluefireteam/audioplayers#1687 and the inability of web apps to play sounds before user interaction.

lib/audio/audio_controller.dart

@@ -7,6 +7,7 @@ import 'dart:collection';
 import 'dart:math';
 
 import 'package:audioplayers/audioplayers.dart';
+import 'package:flutter/foundation.dart';
 import 'package:flutter/widgets.dart';
 import 'package:logging/logging.dart';
 
@@ -76,25 +77,25 @@ class AudioController {
   /// Plays a single sound effect, defined by [type].
   ///
   /// The controller will ignore this call when the attached settings'
-  /// [SettingsController.muted] is `true` or if its
+  /// [SettingsController.audioOn] is `true` or if its
   /// [SettingsController.soundsOn] is `false`.
   void playSfx(SfxType type) {
-    final muted = _settings?.muted.value ?? true;
-    if (muted) {
-      _log.info(() => 'Ignoring playing sound ($type) because audio is muted.');
+    final audioOn = _settings?.audioOn.value ?? false;
+    if (!audioOn) {
+      _log.fine(() => 'Ignoring playing sound ($type) because audio is muted.');
       return;
     }
     final soundsOn = _settings?.soundsOn.value ?? false;
     if (!soundsOn) {
-      _log.info(() =>
+      _log.fine(() =>
           'Ignoring playing sound ($type) because sounds are turned off.');
       return;
     }
 
-    _log.info(() => 'Playing sound: $type');
+    _log.fine(() => 'Playing sound: $type');
     final options = soundTypeToFilename(type);
     final filename = options[_random.nextInt(options.length)];
-    _log.info(() => '- Chosen filename: $filename');
+    _log.fine(() => '- Chosen filename: $filename');
 
     final currentPlayer = _sfxPlayers[_currentSfxPlayer];
     currentPlayer.play(AssetSource('sfx/$filename'),
@@ -113,7 +114,7 @@ class AudioController {
   }
 
   /// Enables the [AudioController] to track changes to settings.
-  /// Namely, when any of [SettingsController.muted],
+  /// Namely, when any of [SettingsController.audioOn],
   /// [SettingsController.musicOn] or [SettingsController.soundsOn] changes,
   /// the audio controller will act accordingly.
   void _attachSettings(SettingsController settingsController) {
@@ -125,20 +126,37 @@ class AudioController {
     // Remove handlers from the old settings controller if present
     final oldSettings = _settings;
     if (oldSettings != null) {
-      oldSettings.muted.removeListener(_mutedHandler);
+      oldSettings.audioOn.removeListener(_audioOnHandler);
       oldSettings.musicOn.removeListener(_musicOnHandler);
       oldSettings.soundsOn.removeListener(_soundsOnHandler);
     }
 
     _settings = settingsController;
 
     // Add handlers to the new settings controller
-    settingsController.muted.addListener(_mutedHandler);
+    settingsController.audioOn.addListener(_audioOnHandler);
     settingsController.musicOn.addListener(_musicOnHandler);
     settingsController.soundsOn.addListener(_soundsOnHandler);
 
-    if (!settingsController.muted.value && settingsController.musicOn.value) {
-      _playCurrentSongInPlaylist();
+    if (settingsController.audioOn.value && settingsController.musicOn.value) {
+      if (kIsWeb) {
+        _log.info('On the web, music can only start after user interaction.');
+      } else {
+        _playCurrentSongInPlaylist();
+      }
+    }
+  }
+
+  void _audioOnHandler() {
+    _log.fine('audioOn changed to ${_settings!.audioOn.value}');
+    if (_settings!.audioOn.value) {
+      // All sound just got un-muted. Audio is on.
+      if (_settings!.musicOn.value) {
+        _startOrResumeMusic();
+      }
+    } else {
+      // All sound just got muted. Audio is off.
+      _stopAllSound();
     }
   }
 
@@ -149,7 +167,7 @@ class AudioController {
       case AppLifecycleState.hidden:
         _stopAllSound();
       case AppLifecycleState.resumed:
-        if (!_settings!.muted.value && _settings!.musicOn.value) {
+        if (_settings!.audioOn.value && _settings!.musicOn.value) {
           _startOrResumeMusic();
         }
       case AppLifecycleState.inactive:
@@ -169,7 +187,7 @@ class AudioController {
   void _musicOnHandler() {
     if (_settings!.musicOn.value) {
       // Music got turned on.
-      if (!_settings!.muted.value) {
+      if (_settings!.audioOn.value) {
         _startOrResumeMusic();
       }
     } else {
@@ -178,26 +196,29 @@ class AudioController {
     }
   }
 
-  void _mutedHandler() {
-    _log.fine('Muted changed to ${_settings!.muted.value}');
-    if (_settings!.muted.value) {
-      // All sound just got muted.
-      _stopAllSound();
-    } else {
-      // All sound just got un-muted.
-      if (_settings!.musicOn.value) {
-        _startOrResumeMusic();
-      }
-    }
-  }
-
   Future<void> _playCurrentSongInPlaylist() async {
     _log.info(() => 'Playing ${_playlist.first} now.');
     try {
-      _musicPlayer.play(AssetSource('music/${_playlist.first.filename}'));
+      await _musicPlayer.play(AssetSource('music/${_playlist.first.filename}'));
     } catch (e) {
       _log.severe('Could not play song ${_playlist.first}', e);
     }
+
+    // Settings can change while the music player is preparing
+    // to play a song (i.e. during the `await` above).
+    // Unfortunately, `audioplayers` has a bug which will ignore calls
+    // to `pause()` before that await is finished, so we need
+    // to double check here.
+    // See issue: https://github.com/bluefireteam/audioplayers/issues/1687
+    if (!_settings!.audioOn.value || !_settings!.musicOn.value) {
+      try {
+        _log.fine('Settings changed while preparing to play song. '
+            'Pausing music.');
+        await _musicPlayer.pause();
+      } catch (e) {
+        _log.severe('Could not pause music player', e);
+      }
+    }
   }
 
   /// Preloads all sound effects.

lib/main_menu/main_menu_screen.dart

@@ -59,11 +59,11 @@ class MainMenuScreen extends StatelessWidget {
             Padding(
               padding: const EdgeInsets.only(top: 32),
               child: ValueListenableBuilder<bool>(
-                valueListenable: settingsController.muted,
-                builder: (context, muted, child) {
+                valueListenable: settingsController.audioOn,
+                builder: (context, audioOn, child) {
                   return IconButton(
-                    onPressed: () => settingsController.toggleMuted(),
-                    icon: Icon(muted ? Icons.volume_off : Icons.volume_up),
+                    onPressed: () => settingsController.toggleAudioOn(),
+                    icon: Icon(audioOn ? Icons.volume_up : Icons.volume_off),
                   );
                 },
               ),

lib/settings/persistence/local_storage_settings_persistence.dart

@@ -13,15 +13,15 @@ class LocalStorageSettingsPersistence extends SettingsPersistence {
       SharedPreferences.getInstance();
 
   @override
-  Future<bool> getMusicOn({required bool defaultValue}) async {
+  Future<bool> getAudioOn({required bool defaultValue}) async {
     final prefs = await instanceFuture;
-    return prefs.getBool('musicOn') ?? defaultValue;
+    return prefs.getBool('mute') ?? defaultValue;
   }
 
   @override
-  Future<bool> getMuted({required bool defaultValue}) async {
+  Future<bool> getMusicOn({required bool defaultValue}) async {
     final prefs = await instanceFuture;
-    return prefs.getBool('mute') ?? defaultValue;
+    return prefs.getBool('musicOn') ?? defaultValue;
   }
 
   @override
@@ -37,15 +37,15 @@ class LocalStorageSettingsPersistence extends SettingsPersistence {
   }
 
   @override
-  Future<void> saveMusicOn(bool value) async {
+  Future<void> saveAudioOn(bool value) async {
     final prefs = await instanceFuture;
-    await prefs.setBool('musicOn', value);
+    await prefs.setBool('mute', value);
   }
 
   @override
-  Future<void> saveMuted(bool value) async {
+  Future<void> saveMusicOn(bool value) async {
     final prefs = await instanceFuture;
-    await prefs.setBool('mute', value);
+    await prefs.setBool('musicOn', value);
   }
 
   @override

lib/settings/persistence/memory_settings_persistence.dart

@@ -11,15 +11,15 @@ class MemoryOnlySettingsPersistence implements SettingsPersistence {
 
   bool soundsOn = true;
 
-  bool muted = false;
+  bool audioOn = true;
 
   String playerName = 'Player';
 
   @override
-  Future<bool> getMusicOn({required bool defaultValue}) async => musicOn;
+  Future<bool> getAudioOn({required bool defaultValue}) async => audioOn;
 
   @override
-  Future<bool> getMuted({required bool defaultValue}) async => muted;
+  Future<bool> getMusicOn({required bool defaultValue}) async => musicOn;
 
   @override
   Future<String> getPlayerName() async => playerName;
@@ -28,10 +28,10 @@ class MemoryOnlySettingsPersistence implements SettingsPersistence {
   Future<bool> getSoundsOn({required bool defaultValue}) async => soundsOn;
 
   @override
-  Future<void> saveMusicOn(bool value) async => musicOn = value;
+  Future<void> saveAudioOn(bool value) async => audioOn = value;
 
   @override
-  Future<void> saveMuted(bool value) async => muted = value;
+  Future<void> saveMusicOn(bool value) async => musicOn = value;
 
   @override
   Future<void> savePlayerName(String value) async => playerName = value;

lib/settings/persistence/settings_persistence.dart

@@ -7,17 +7,17 @@
 /// Implementations can range from simple in-memory storage through
 /// local preferences to cloud-based solutions.
 abstract class SettingsPersistence {
-  Future<bool> getMusicOn({required bool defaultValue});
+  Future<bool> getAudioOn({required bool defaultValue});
 
-  Future<bool> getMuted({required bool defaultValue});
+  Future<bool> getMusicOn({required bool defaultValue});
 
   Future<String> getPlayerName();
 
   Future<bool> getSoundsOn({required bool defaultValue});
 
-  Future<void> saveMusicOn(bool value);
+  Future<void> saveAudioOn(bool value);
 
-  Future<void> saveMuted(bool value);
+  Future<void> saveMusicOn(bool value);
 
   Future<void> savePlayerName(String value);
 

lib/settings/settings.dart

@@ -3,72 +3,89 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'package:flutter/foundation.dart';
+import 'package:logging/logging.dart';
 
 import 'persistence/local_storage_settings_persistence.dart';
 import 'persistence/settings_persistence.dart';
 
 /// An class that holds settings like [playerName] or [musicOn],
 /// and saves them to an injected persistence store.
 class SettingsController {
-  /// TODO: If needed, replace this with some other mechanism for saving
-  ///       settings. Currently, this uses the local storage
-  ///       (i.e. NSUserDefaults on iOS, SharedPreferences on Android
-  ///       or local storage on the web).
-  final SettingsPersistence _persistence = LocalStorageSettingsPersistence();
+  static final _log = Logger('SettingsController');
 
-  /// Whether or not the sound is on at all. This overrides both music
-  /// and sound.
-  ValueNotifier<bool> muted = ValueNotifier(true);
+  /// The persistence store that is used to save settings.
+  final SettingsPersistence _persistence;
 
+  /// Whether or not the audio is on at all. This overrides both music
+  /// and sounds (sfx).
+  ///
+  /// This is an important feature especially on mobile, where players
+  /// expect to be able to quickly mute all the audio. Having this as
+  /// a separate flag (as opposed to some kind of {off, sound, everything}
+  /// enum) means that the player will not lose their [soundsOn] and
+  /// [musicOn] preferences when they temporarily mute the game.
+  ValueNotifier<bool> audioOn = ValueNotifier(true);
+
+  /// The player's name. Used for things like high score lists.
   ValueNotifier<String> playerName = ValueNotifier('Player');
 
+  /// Whether or not the sound effects (sfx) are on.
   ValueNotifier<bool> soundsOn = ValueNotifier(true);
 
+  /// Whether or not the music is on.
   ValueNotifier<bool> musicOn = ValueNotifier(true);
 
   /// Creates a new instance of [SettingsController] backed by [persistence].
-  SettingsController() {
-    loadStateFromPersistence();
-  }
-
-  /// Asynchronously loads values from the injected persistence store.
-  Future<void> loadStateFromPersistence() async {
-    await Future.wait([
-      _persistence
-          .getMuted(defaultValue: false)
-          // On the web, sound can only start after user interaction, so
-          // we start muted there on every game start.
-          // On other platforms, we can use the persisted value.
-          .then((value) {
-        return muted.value = kIsWeb || value;
-      }),
-      _persistence
-          .getSoundsOn(defaultValue: true)
-          .then((value) => soundsOn.value = value),
-      _persistence
-          .getMusicOn(defaultValue: true)
-          .then((value) => musicOn.value = value),
-      _persistence.getPlayerName().then((value) => playerName.value = value),
-    ]);
+  ///
+  /// By default, settings are persisted using [LocalStorageSettingsPersistence]
+  /// (i.e. NSUserDefaults on iOS, SharedPreferences on Android or
+  /// local storage on the web).
+  SettingsController({SettingsPersistence? persistence})
+      : _persistence = persistence ?? LocalStorageSettingsPersistence() {
+    _loadStateFromPersistence();
   }
 
   void setPlayerName(String name) {
     playerName.value = name;
     _persistence.savePlayerName(playerName.value);
   }
 
+  void toggleAudioOn() {
+    audioOn.value = !audioOn.value;
+    _persistence.saveAudioOn(audioOn.value);
+  }
+
   void toggleMusicOn() {
     musicOn.value = !musicOn.value;
     _persistence.saveMusicOn(musicOn.value);
   }
 
-  void toggleMuted() {
-    muted.value = !muted.value;
-    _persistence.saveMuted(muted.value);
-  }
-
   void toggleSoundsOn() {
     soundsOn.value = !soundsOn.value;
     _persistence.saveSoundsOn(soundsOn.value);
   }
+
+  /// Asynchronously loads values from the injected persistence store.
+  Future<void> _loadStateFromPersistence() async {
+    final loadedValues = await Future.wait([
+      _persistence.getAudioOn(defaultValue: true).then((value) {
+        if (kIsWeb) {
+          // On the web, sound can only start after user interaction, so
+          // we start muted there on every game start.
+          return audioOn.value = false;
+        }
+        // On other platforms, we can use the persisted value.
+        return audioOn.value = value;
+      }),
+      _persistence
+          .getSoundsOn(defaultValue: true)
+          .then((value) => soundsOn.value = value),
+      _persistence
+          .getMusicOn(defaultValue: true)
+          .then((value) => musicOn.value = value),
+      _persistence.getPlayerName().then((value) => playerName.value = value),
+    ]);
+
+    _log.fine(() => 'Loaded settings: $loadedValues');
+  }
 }