Fixed DownloadProgress reporting bug when using buffering

Updated CHANGELOG

CHANGELOG.md

@@ -44,11 +44,14 @@ Additionally, vector tiles are now supported in theory, as the internal caching/
   * Improved speed (by massive amounts) and accuracy & reduced memory consumption of `CircleRegion`'s tile generation & counting algorithm
   * Deprecated `BaseRegion.(maybe)When` - this is easy to perform using a standard pattern-matched switch
 
-* Changes to bulk downloading
-  * `DownloadProgress.latestTileEvent` is now nullable
+* Major changes to bulk downloading
+  * Added support for retrying failed tiles (that failed because the request could not be made) once at the end of the download
+  * `StoreDownload.startForeground` output stream split into two streams returned as a record, one for `TileEvent`s, one for `DownloadProgress`s
+  * `TileEvents` has been split up into multiple classes and mixins to reduce nullability and uncertainty
+  * `DownloadProgress` has had its contained metrics changed to reflect the failed tiles retry, and `latestTileEvent` removed
 
 * Exporting stores is now more stable, and has improved documentation  
-  The method now works in a dedicated temporary environment and attempts to perform two different strategies to move/copy-and-delete the result to the specified directory at the end before failing. Improved documentation covers the potential pitfalls of permissions and now recommends exporting to an app directory, then using the system share functionality on some devices. It now also returns the number of exported tiles.
+  > The method now works in a dedicated temporary environment and attempts to perform two different strategies to move/copy-and-delete the result to the specified directory at the end before failing. Improved documentation covers the potential pitfalls of permissions and now recommends exporting to an app directory, then using the system share functionality on some devices. It now also returns the number of exported tiles.
 
 * Removed deprecated remnants from v9.*
 

example/lib/src/screens/main/map_view/map_view.dart

@@ -1,4 +1,4 @@
-import 'dart:async';
+//import 'dart:async';
 import 'dart:io';
 import 'dart:math';
 
@@ -13,11 +13,11 @@ import 'package:provider/provider.dart';
 
 import '../../../shared/misc/shared_preferences.dart';
 import '../../../shared/misc/store_metadata_keys.dart';
-import '../../../shared/state/download_provider.dart';
+//import '../../../shared/state/download_provider.dart';
 import '../../../shared/state/general_provider.dart';
 import '../../../shared/state/region_selection_provider.dart';
 import 'components/debugging_tile_builder/debugging_tile_builder.dart';
-import 'components/download_progress/download_progress_masker.dart';
+//import 'components/download_progress/download_progress_masker.dart';
 import 'components/fmtc_not_in_use_indicator.dart';
 import 'components/region_selection/crosshairs.dart';
 import 'components/region_selection/custom_polygon_snapping_indicator.dart';
@@ -329,9 +329,9 @@ class _MapViewState extends State<MapView> with TickerProviderStateMixin {
                       ),
             );
 
-            final isDownloadProgressMaskerVisible = widget.mode ==
-                    MapViewMode.downloadRegion &&
-                context.select<DownloadingProvider, bool>((p) => p.isFocused);
+            //final isDownloadProgressMaskerVisible = widget.mode ==
+            //        MapViewMode.downloadRegion &&
+            //    context.select<DownloadingProvider, bool>((p) => p.isFocused);
 
             final map = FlutterMap(
               mapController: _mapController.mapController,

example/lib/src/screens/main/secondary_view/contents/download_configuration/components/config_options/components/slider_option.dart

@@ -34,7 +34,7 @@ class _SliderOption extends StatelessWidget {
             ),
           ),
           SizedBox(
-            width: 72,
+            width: 80,
             child: Text(
               '$value $descriptor',
               textAlign: TextAlign.end,

lib/src/bulk_download/external/download_progress.dart

@@ -16,8 +16,8 @@ class DownloadProgress {
   /// update
   @protected
   const DownloadProgress._({
-    required this.flushedTilesCount,
-    required this.flushedTilesSize,
+    required this.successfulTilesSize,
+    required this.successfulTilesCount,
     required this.bufferedTilesCount,
     required this.bufferedTilesSize,
     required this.seaTilesCount,
@@ -44,8 +44,8 @@ class DownloadProgress {
     required this.maxTilesCount,
     required int? tilesPerSecondLimit,
     required bool retryFailedRequestTiles,
-  })  : flushedTilesCount = 0,
-        flushedTilesSize = 0,
+  })  : successfulTilesCount = 0,
+        successfulTilesSize = 0,
         bufferedTilesCount = 0,
         bufferedTilesSize = 0,
         seaTilesCount = 0,
@@ -68,21 +68,24 @@ class DownloadProgress {
   ///
   /// For [SuccessfulTileEvent]s, the flushed and buffered metrics cannot be
   /// automatically updated from information in the tile event alone.
-  /// [flushedTiles] & [bufferedTiles] should be updated manually.
+  /// [bufferedTiles] should be updated manually.
   ///
   /// [maxTilesCount], [_tilesPerSecondLimit] & [_retryFailedRequestTiles] may
   /// not be modified. [elapsedDuration] & [tilesPerSecond] must always be
   /// modified.
   DownloadProgress _updateWithTile({
     required TileEvent newTileEvent,
-    ({int count, double size})? flushedTiles,
     ({int count, double size})? bufferedTiles,
     required Duration elapsedDuration,
     required double tilesPerSecond,
   }) =>
       DownloadProgress._(
-        flushedTilesCount: flushedTiles?.count ?? flushedTilesCount,
-        flushedTilesSize: flushedTiles?.size ?? flushedTilesSize,
+        successfulTilesCount: successfulTilesCount +
+            (newTileEvent is SuccessfulTileEvent ? 1 : 0),
+        successfulTilesSize: successfulTilesSize +
+            (newTileEvent is SuccessfulTileEvent
+                ? newTileEvent.tileImage.lengthInBytes / 1024
+                : 0),
         bufferedTilesCount: bufferedTiles?.count ?? bufferedTilesCount,
         bufferedTilesSize: bufferedTiles?.size ?? bufferedTilesSize,
         seaTilesCount: seaTilesCount + (newTileEvent is SeaTileEvent ? 1 : 0),
@@ -124,8 +127,8 @@ class DownloadProgress {
     required double tilesPerSecond,
   }) =>
       DownloadProgress._(
-        flushedTilesCount: flushedTilesCount,
-        flushedTilesSize: flushedTilesSize,
+        successfulTilesCount: successfulTilesCount,
+        successfulTilesSize: successfulTilesSize,
         bufferedTilesCount: bufferedTilesCount,
         bufferedTilesSize: bufferedTilesSize,
         seaTilesCount: seaTilesCount,
@@ -142,6 +145,32 @@ class DownloadProgress {
         retryFailedRequestTiles: _retryFailedRequestTiles,
       );
 
+  /// Create a new progress object that represents a finished download
+  ///
+  /// This means [tilesPerSecond] is set to 0, and the buffered statistics are
+  /// set to 0.
+  DownloadProgress _updateToComplete({
+    required Duration elapsedDuration,
+  }) =>
+      DownloadProgress._(
+        successfulTilesCount: successfulTilesCount,
+        successfulTilesSize: successfulTilesSize,
+        bufferedTilesCount: 0,
+        bufferedTilesSize: 0,
+        seaTilesCount: seaTilesCount,
+        seaTilesSize: seaTilesSize,
+        existingTilesCount: existingTilesCount,
+        existingTilesSize: existingTilesSize,
+        negativeResponseTilesCount: negativeResponseTilesCount,
+        failedRequestTilesCount: failedRequestTilesCount,
+        retryTilesQueuedCount: retryTilesQueuedCount,
+        maxTilesCount: maxTilesCount,
+        elapsedDuration: elapsedDuration,
+        tilesPerSecond: 0,
+        tilesPerSecondLimit: _tilesPerSecondLimit,
+        retryFailedRequestTiles: _retryFailedRequestTiles,
+      );
+
   /// The number of tiles remaining to be attempted to download
   ///
   /// This includes [retryTilesQueuedCount] as tiles remaining.
@@ -163,19 +192,19 @@ class DownloadProgress {
   /// and actually flushed/written to cache)
   ///
   /// This is the number of [SuccessfulTileEvent]s emitted.
-  int get successfulTilesCount => flushedTilesCount + bufferedTilesCount;
+  final int successfulTilesCount;
 
   /// The size in KiB of the tile images successfully downloaded (including both
   /// tiles buffered and actually flushed/written to cache)
-  double get successfulTilesSize => flushedTilesSize + bufferedTilesSize;
+  final double successfulTilesSize;
 
   /// The number of tiles successfully downloaded and written to the cache
   /// (flushed from the buffer)
-  final int flushedTilesCount;
+  int get flushedTilesCount => successfulTilesCount - bufferedTilesCount;
 
   /// The size in KiB of the tile images successfully downloaded and written to
   /// the cache (flushed from the buffer)
-  final double flushedTilesSize;
+  double get flushedTilesSize => successfulTilesSize - bufferedTilesSize;
 
   /// The number of tiles successfully downloaded but still to be written to the
   /// cache
@@ -325,8 +354,8 @@ class DownloadProgress {
   bool operator ==(Object other) =>
       identical(this, other) ||
       (other is DownloadProgress &&
-          flushedTilesCount == other.flushedTilesCount &&
-          flushedTilesSize == other.flushedTilesSize &&
+          successfulTilesCount == other.successfulTilesCount &&
+          successfulTilesSize == other.successfulTilesSize &&
           bufferedTilesCount == other.bufferedTilesCount &&
           bufferedTilesSize == other.bufferedTilesSize &&
           seaTilesCount == other.seaTilesCount &&
@@ -343,8 +372,8 @@ class DownloadProgress {
 
   @override
   int get hashCode => Object.hashAllUnordered([
-        flushedTilesCount,
-        flushedTilesSize,
+        successfulTilesCount,
+        successfulTilesSize,
         bufferedTilesCount,
         bufferedTilesSize,
         seaTilesCount,

lib/src/bulk_download/internal/manager.dart

@@ -318,15 +318,9 @@ Future<void> _downloadManager(
 
               // If buffering is in use, send a progress update with buffer info
               if (input.maxBufferLength != 0) {
-                // TODO: Fix incorrect stats reporting
-
                 // Update correct thread buffer with new tile on success
-                late final int flushedTilesCount;
-                late final int flushedTilesSize;
                 if (evt is SuccessfulTileEvent) {
                   if (evt._wasBufferFlushed) {
-                    flushedTilesCount = threadBuffersTiles[threadNo];
-                    flushedTilesSize = threadBuffersSize[threadNo];
                     threadBuffersTiles[threadNo] = 0;
                     threadBuffersSize[threadNo] = 0;
                   } else {
@@ -340,22 +334,14 @@ Future<void> _downloadManager(
 
                 sendToMain(
                   lastDownloadProgress = lastDownloadProgress._updateWithTile(
-                    newTileEvent: evt,
                     bufferedTiles: evt is SuccessfulTileEvent
                         ? (
                             count: threadBuffersTiles.reduce((a, b) => a + b),
                             size: threadBuffersSize.reduce((a, b) => a + b) /
                                 1024,
                           )
                         : null,
-                    flushedTiles: wasBufferFlushed
-                        ? (
-                            count: lastDownloadProgress.flushedTilesCount +
-                                flushedTilesCount,
-                            size: lastDownloadProgress.flushedTilesSize +
-                                flushedTilesSize / 1024
-                          )
-                        : null,
+                    newTileEvent: evt,
                     elapsedDuration: downloadDuration.elapsed,
                     tilesPerSecond: getCurrentTPS(registerNewTPS: true),
                   ),
@@ -373,13 +359,6 @@ Future<void> _downloadManager(
                 sendToMain(
                   lastDownloadProgress = lastDownloadProgress._updateWithTile(
                     newTileEvent: evt,
-                    flushedTiles: evt is SuccessfulTileEvent
-                        ? (
-                            count: lastDownloadProgress.flushedTilesCount + 1,
-                            size: lastDownloadProgress.flushedTilesSize +
-                                (evt.tileImage.lengthInBytes / 1024)
-                          )
-                        : null,
                     elapsedDuration: downloadDuration.elapsed,
                     tilesPerSecond: getCurrentTPS(registerNewTPS: true),
                   ),
@@ -446,7 +425,14 @@ Future<void> _downloadManager(
       growable: false,
     ),
   );
+
+  // Send final progress update
   downloadDuration.stop();
+  sendToMain(
+    lastDownloadProgress = lastDownloadProgress._updateToComplete(
+      elapsedDuration: downloadDuration.elapsed,
+    ),
+  );
 
   // Cleanup resources and shutdown
   fallbackReportTimer?.cancel();

lib/src/store/download.dart

@@ -48,11 +48,16 @@ class StoreDownload {
   /// One emits [TileEvent]s which contain info about the most recent tile
   /// attempted only.
   ///
-  /// The first stream will emit events once per tile emitted on the second
-  /// stream, at intervals of no longer than [maxReportInterval], and once at
-  /// the start of the download indicating setup is complete and the first tile
-  /// is being downloaded. It finishes once the download is complete, with the
-  /// last event representing the last emitted tile event on the second stream.
+  /// The first stream (of [DownloadProgress]s) will emit events:
+  ///  * once per [TileEvent] emitted on the second stream
+  ///  * at intervals of no longer than [maxReportInterval]
+  ///  * once at the start of the download indicating setup is complete and the
+  ///    first tile is being downloaded
+  ///  * once additionally at the end of the download after the last tile
+  ///    setting some final statistics (such as tiles per second to 0)
+  ///
+  /// Once the stream of [DownloadProgress]s completes/finishes, the download
+  /// has stopped.
   ///
   /// Neither output stream respects listen, pause, resume, or cancel events
   /// when submitted through the stream subscription.