From 09253726e37eafccc5a0fca8d11ca333bbd39317 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 21:21:38 -0500 Subject: [PATCH 1/5] Send the map and theme only when the user changed them Every save compared the canvas's map and theme with the server's and queued the difference. A teammate's change made them differ too, so the next save wrote the old map back over it. The canvas never drew the teammate's change either: the check that redraws a page left the map and theme out. A change sent with no answer, then set back, was dropped from the outbox as if it had never been sent. If it had landed, the server kept it while the canvas showed the old value and the chip said synced. --- .../collab/strategy_op_queue_provider.dart | 14 +- .../strategy_page_session_provider.dart | 8 ++ lib/providers/strategy_provider.dart | 131 +++++++++--------- lib/strategy/strategy_page_source.dart | 64 +-------- test/strategy_page_session_provider_test.dart | 78 ++++++++++- 5 files changed, 165 insertions(+), 130 deletions(-) diff --git a/lib/providers/collab/strategy_op_queue_provider.dart b/lib/providers/collab/strategy_op_queue_provider.dart index 1fe501bf9..a0fd56ef7 100644 --- a/lib/providers/collab/strategy_op_queue_provider.dart +++ b/lib/providers/collab/strategy_op_queue_provider.dart @@ -229,17 +229,23 @@ StrategyPatchOp? waitingStrategyPatch( } /// Whether a strategy-level change to [strategyPublicId] that [queue] can -/// no longer take back may still land: one being sent, or one waiting -/// behind another. A canvas set back to the server's map and theme then -/// needs that written too, or the change landing undoes it. +/// no longer take back may still land: one being sent, one waiting behind +/// another, or one sent before with no answer (waiting to go again, or +/// paused). A canvas set back to the server's map and theme then needs that +/// written too, or the change landing undoes it. bool strategyPatchMayLand( StrategyOpQueueState queue, String strategyPublicId, ) { if (queue.strategyPublicId != strategyPublicId) return false; const key = EntitySyncKey.strategy(); + bool sentBefore(QueuedEntityIntent? intent) => + intent != null && + (intent.pending.attempts > 0 || intent.pending.lastAttemptAt != null); return queue.inFlightByEntityKey.containsKey(key) || - queue.successorByEntityKey.containsKey(key); + queue.successorByEntityKey.containsKey(key) || + sentBefore(queue.queuedByEntityKey[key]) || + sentBefore(queue.pausedByEntityKey[key]); } /// [header], the server's as read, as this device knows the server holds diff --git a/lib/providers/strategy_page_session_provider.dart b/lib/providers/strategy_page_session_provider.dart index 2d719dde2..429670e16 100644 --- a/lib/providers/strategy_page_session_provider.dart +++ b/lib/providers/strategy_page_session_provider.dart @@ -1875,7 +1875,15 @@ class StrategyPageSessionNotifier extends Notifier { final assets = snapshot.assetsById.values.toList() ..sort((a, b) => a.publicId.compareTo(b.publicId)); + // The map and theme are drawn with the page, as the canvas draws them + // (see drawnStrategyHeader): a teammate's change to either redraws it. + final header = drawnStrategyHeader(ref, snapshot.header); final fingerprint = jsonEncode({ + 'strategy': { + 'mapData': header.mapData, + 'themeProfileId': header.themeProfileId, + 'themeOverridePalette': header.themeOverridePalette, + }, 'page': { 'publicId': page.publicId, 'name': page.name, diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index abaecfd3e..ee1f6ce35 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -39,7 +39,6 @@ import 'package:icarus/providers/utility_provider.dart'; import 'package:hive_ce/hive.dart'; import 'package:icarus/services/strategy_folders.dart'; import 'package:uuid/uuid.dart'; -import 'package:icarus/collab/canonical_json.dart'; import 'package:icarus/collab/cloud_media_models.dart'; import 'package:icarus/collab/collab_models.dart'; import 'package:icarus/collab/page_made_here.dart'; @@ -158,6 +157,12 @@ class StrategyProvider extends Notifier { bool _cloudStrategyMutationSyncScheduled = false; int _persistenceTrackingSuspensionCount = 0; + /// The strategy whose map or theme the user changed on the canvas, until + /// the server or the outbox holds what the canvas shows. A canvas that + /// differs from the server without it was not changed here: a teammate + /// changed the server's (see [syncStrategyMetadata]). + String? _mapOrThemeChangedIn; + void _registerPersistenceTrackingListeners() { _listenForPageBackedState(agentProvider); _listenForPageBackedState(abilityProvider); @@ -195,6 +200,7 @@ class StrategyProvider extends Notifier { if (!_shouldTrackPersistedEditorMutation()) { return; } + _mapOrThemeChangedIn = state.strategyId; _markStrategyBackedStateUnsaved(); }); } @@ -557,13 +563,7 @@ class StrategyProvider extends Notifier { ..setPendingCloudSync(true) ..setCloudSyncError(null); - if (_showsWaitingStrategyPatch()) return; - final desiredOp = _buildDesiredStrategySyncOp(); - ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( - entityKey: const EntitySyncKey.strategy(), - desiredOp: desiredOp, - flushImmediately: flushImmediately, - ); + unawaited(syncStrategyMetadata(flushImmediately: flushImmediately)); if (flushImmediately) { await ref.read(strategyOpQueueProvider.notifier).flushNow(); } @@ -627,74 +627,81 @@ class StrategyProvider extends Notifier { refreshAutosaveScheduling(); } - /// Whether the canvas's map and theme are the strategy-level change still - /// waiting in the outbox, as it was drawn (see drawnStrategyHeader): that - /// change waits as it is. - bool _showsWaitingStrategyPatch() { + /// Queues the canvas's map and theme for the server, when the user changed + /// them. Every save asks, so this is where a difference between the canvas + /// and the server is told apart: + /// + /// - The outbox already holds what the canvas shows: that change waits as + /// it is. + /// - The server holds what the canvas shows: nothing is sent, and a change + /// still waiting is dropped, unless one may still land and undo that + /// (see strategyPatchMayLand); then the canvas's values are written + /// behind it. + /// - They differ and the user changed neither: a teammate changed the + /// server's. Nothing is sent, and the canvas takes the server's when the + /// page is next brought up to date. + /// - Otherwise the user's change is queued, against the revision this + /// device knows the server holds. + Future syncStrategyMetadata({bool flushImmediately = false}) async { final strategyId = state.strategyId; final snapshot = ref.read(remoteEditorSnapshotProvider).valueOrNull; + final mapData = Maps.mapNames[ref.read(mapProvider).currentMap]; if (strategyId == null || snapshot == null || - snapshot.header.publicId != strategyId) { - return false; + snapshot.header.publicId != strategyId || + mapData == null) { + return; } - final waiting = - waitingStrategyPatch(ref.read(strategyOpQueueProvider), strategyId); - final drawn = drawnStrategyHeader(ref, snapshot.header); final strategyTheme = ref.read(strategyThemeProvider); - final mapData = Maps.mapNames[ref.read(mapProvider).currentMap]; - return waiting != null && - mapData != null && - drawn.shows( + final themeOverridePalette = strategyTheme.overridePalette?.toJson(); + bool shows(RemoteStrategyHeader header) => header.shows( mapData: mapData, themeProfileId: strategyTheme.profileId, - themeOverridePalette: strategyTheme.overridePalette?.toJson(), + themeOverridePalette: themeOverridePalette, ); - } - - StrategyOp? _buildDesiredStrategySyncOp() { - final strategyId = state.strategyId; - final snapshot = ref.read(remoteEditorSnapshotProvider).valueOrNull; - if (strategyId == null || - snapshot == null || - snapshot.header.publicId != strategyId) { - return null; - } // The server's, as this device knows it holds them. final header = knownStrategyHeader(ref, snapshot.header); - final strategyTheme = ref.read(strategyThemeProvider); - final localMapData = - Maps.mapNames[ref.read(mapProvider).currentMap] ?? header.mapData; - final localThemeProfileId = strategyTheme.profileId; - final localThemeOverridePalette = strategyTheme.overridePalette?.toJson(); - - final matchesRemote = header.mapData == localMapData && - header.themeProfileId == localThemeProfileId && - cloudJsonEquivalent( - header.themeOverridePalette, - localThemeOverridePalette, - ); - // A change that may still land would undo the canvas matching the - // server, so the canvas's values are written behind it. - if (matchesRemote && - !strategyPatchMayLand(ref.read(strategyOpQueueProvider), strategyId)) { - return null; + final queue = ref.read(strategyOpQueueProvider); + final waiting = waitingStrategyPatch(queue, strategyId); + if (waiting != null && shows(header.withPatch(waiting.payload))) { + _mapOrThemeChangedIn = null; + return; } - return StrategyPatchOp( - opId: const Uuid().v4(), - expectedStrategyRevision: header.revision, - payload: { - 'mapData': localMapData, - if (localThemeProfileId != null) 'themeProfileId': localThemeProfileId, - if (localThemeProfileId == null) 'clearThemeProfileId': true, - if (localThemeOverridePalette != null) - 'themeOverridePalette': localThemeOverridePalette, - if (localThemeOverridePalette == null) - 'clearThemeOverridePalette': true, - }, - ); + if (shows(header)) { + _mapOrThemeChangedIn = null; + if (!strategyPatchMayLand(queue, strategyId)) { + await ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( + entityKey: const EntitySyncKey.strategy(), + desiredOp: null, + flushImmediately: flushImmediately, + ); + return; + } + } else if (waiting == null && _mapOrThemeChangedIn != strategyId) { + return; + } + + await ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( + entityKey: const EntitySyncKey.strategy(), + desiredOp: StrategyPatchOp( + opId: const Uuid().v4(), + expectedStrategyRevision: header.revision, + payload: { + 'mapData': mapData, + if (strategyTheme.profileId != null) + 'themeProfileId': strategyTheme.profileId + else + 'clearThemeProfileId': true, + if (themeOverridePalette != null) + 'themeOverridePalette': themeOverridePalette + else + 'clearThemeOverridePalette': true, + }, + ), + flushImmediately: flushImmediately, + ); } void setUnsaved() { diff --git a/lib/strategy/strategy_page_source.dart b/lib/strategy/strategy_page_source.dart index 3a555c9ec..8c0c46ce0 100644 --- a/lib/strategy/strategy_page_source.dart +++ b/lib/strategy/strategy_page_source.dart @@ -28,7 +28,6 @@ import 'package:icarus/providers/utility_provider.dart'; import 'package:icarus/strategy/cloud_element_decoding.dart'; import 'package:icarus/strategy/strategy_migrator.dart'; import 'package:icarus/strategy/strategy_page_models.dart'; -import 'package:uuid/uuid.dart'; abstract class StrategyPageSource { RemoteEditorSnapshot? get loadedRemoteSnapshot; @@ -358,7 +357,7 @@ class CloudStrategyPageSource implements StrategyPageSource { return false; } - await _syncStrategyMetadata(); + await ref.read(strategyProvider.notifier).syncStrategyMetadata(); final desiredOpsByEntityKey = ref.read(activePageLiveSyncProvider.notifier).syncLocalPage( @@ -377,67 +376,6 @@ class CloudStrategyPageSource implements StrategyPageSource { return true; } - Future _syncStrategyMetadata() async { - final snapshot = ref.read(remoteEditorSnapshotProvider).valueOrNull; - if (snapshot == null) { - return; - } - - final currentMapData = Maps.mapNames[ref.read(mapProvider).currentMap]; - if (currentMapData == null) { - return; - } - - final strategyTheme = ref.read(strategyThemeProvider); - final desiredThemeOverride = strategyTheme.overridePalette?.toJson(); - // The server's, as this device knows it holds them. - final header = knownStrategyHeader(ref, snapshot.header); - bool shows(RemoteStrategyHeader header) => header.shows( - mapData: currentMapData, - themeProfileId: strategyTheme.profileId, - themeOverridePalette: desiredThemeOverride, - ); - - // The canvas shows the strategy-level change still waiting in the - // outbox, as it was drawn: that change waits as it is. - final queue = ref.read(strategyOpQueueProvider); - final waiting = waitingStrategyPatch(queue, header.publicId); - if (waiting != null && shows(header.withPatch(waiting.payload))) return; - - // Shown as the server has it: whatever waits can be dropped, unless it - // may still land and undo that. - if (shows(header) && !strategyPatchMayLand(queue, header.publicId)) { - await ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( - entityKey: const EntitySyncKey.strategy(), - desiredOp: null, - flushImmediately: false, - ); - return; - } - - final payload = { - 'mapData': currentMapData, - if (strategyTheme.profileId != null) - 'themeProfileId': strategyTheme.profileId - else - 'clearThemeProfileId': true, - if (desiredThemeOverride != null) - 'themeOverridePalette': desiredThemeOverride - else - 'clearThemeOverridePalette': true, - }; - - await ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( - entityKey: const EntitySyncKey.strategy(), - desiredOp: StrategyPatchOp( - opId: const Uuid().v4(), - payload: payload, - expectedStrategyRevision: header.revision, - ), - flushImmediately: false, - ); - } - StrategyEditorPageData _hydrateProjectedPage( RemoteEditorSnapshot snapshot, RemotePage page, diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 2dfb1f4fd..2e6039b4b 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -1687,6 +1687,8 @@ void main() { queue: _FakeStrategyOpQueueNotifier(), ); + // The user changes the map: the canvas no longer shows the server's. + container.read(mapProvider.notifier).updateMap(MapValue.bind); await container .read(strategyProvider.notifier) .notifyCloudStrategyMutation(); @@ -1697,7 +1699,7 @@ void main() { expect(op.toConvexJson()['expectedStrategyRevision'], 17); expect( op.payload, - containsPair('mapData', Maps.mapNames[MapValue.ascent]), + containsPair('mapData', Maps.mapNames[MapValue.bind]), ); expect(op.payload, containsPair('clearThemeProfileId', true)); }); @@ -9997,6 +9999,80 @@ void main() { }); }); + group('a map or theme changed by a teammate, or sent with no answer', () { + const defaultTheme = 'immutable-default-map-theme'; + + Future settle() async { + for (var i = 0; i < 10; i++) { + await _settle(); + } + } + + Future<(ProviderContainer, List>)> open() async { + final page = _page('page-1', 0); + server = _FakeServer(page.publicId); + final opened = + await openOnRealQueue(page, themeProfileId: defaultTheme); + await settle(); + return opened; + } + + Future drained(ProviderContainer container) async { + await _until( + () => container.read(strategyOpQueueProvider).pending.isEmpty); + await settle(); + } + + test("a teammate's map stays when the user then edits the page", + () async { + final (container, _) = await open(); + server.teammateSetsMap('haven'); + showServer(); + await settle(); + + container.read(agentProvider.notifier).addAgent(PlacedAgent( + id: 'agent-1', + type: AgentType.jett, + position: const Offset(40, 40), + )); + await settle(); + await container + .read(strategyProvider.notifier) + .forceSaveNow('cloud-strategy'); + await drained(container); + + expect(server.strategyPatches, isEmpty); + expect(server.strategyMap, 'haven'); + expect(container.read(mapProvider).currentMap, MapValue.haven); + }); + + test( + 'a map change sent with no answer, then set back, is written back ' + 'on the server too', () async { + final (container, batches) = await open(); + repository.loseNextAnswer = () {}; + container.read(mapProvider.notifier).updateMap(MapValue.bind); + await _until(() => + batches.any((batch) => batch.any((op) => op is StrategyPatchOp))); + await settle(); + // The send failed; the server took it all the same. + expect(server.strategyMap, 'bind'); + + container.read(mapProvider.notifier).updateMap(MapValue.ascent); + await settle(); + await container + .read(strategyProvider.notifier) + .forceSaveNow('cloud-strategy'); + await container.read(strategyOpQueueProvider.notifier).flushNow(); + await drained(container); + showServer(); + await settle(); + + expect(container.read(mapProvider).currentMap, MapValue.ascent); + expect(server.strategyMap, 'ascent'); + }); + }); + test('offline edits to a group nobody changed land on reconnect', () async { final page = _page('page-1', 0); server = _FakeServer(page.publicId, lineups: [fanIn(page.publicId)]); From 3413721a4f9c3355eed23cd7971edd47a129c779 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 21:27:11 -0500 Subject: [PATCH 2/5] Forget the user's map change once the outbox holds it Left remembered after it landed, it let a later save write that map over a teammate's change that arrived mid-drag. --- lib/providers/strategy_provider.dart | 52 ++++++++++++------- test/strategy_page_session_provider_test.dart | 31 +++++++++++ 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index ee1f6ce35..e2a4f0c73 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -163,6 +163,10 @@ class StrategyProvider extends Notifier { /// changed the server's (see [syncStrategyMetadata]). String? _mapOrThemeChangedIn; + /// Counts the user's map and theme changes, so a change made while an + /// earlier one is being stored in the outbox is not taken as stored too. + int _mapOrThemeChanges = 0; + void _registerPersistenceTrackingListeners() { _listenForPageBackedState(agentProvider); _listenForPageBackedState(abilityProvider); @@ -201,6 +205,7 @@ class StrategyProvider extends Notifier { return; } _mapOrThemeChangedIn = state.strategyId; + _mapOrThemeChanges += 1; _markStrategyBackedStateUnsaved(); }); } @@ -683,25 +688,34 @@ class StrategyProvider extends Notifier { return; } - await ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( - entityKey: const EntitySyncKey.strategy(), - desiredOp: StrategyPatchOp( - opId: const Uuid().v4(), - expectedStrategyRevision: header.revision, - payload: { - 'mapData': mapData, - if (strategyTheme.profileId != null) - 'themeProfileId': strategyTheme.profileId - else - 'clearThemeProfileId': true, - if (themeOverridePalette != null) - 'themeOverridePalette': themeOverridePalette - else - 'clearThemeOverridePalette': true, - }, - ), - flushImmediately: flushImmediately, - ); + final changes = _mapOrThemeChanges; + final queueNotifier = ref.read(strategyOpQueueProvider.notifier); + final failuresBefore = queueNotifier.persistenceFailureCount; + await queueNotifier.syncDesiredGenericOp( + entityKey: const EntitySyncKey.strategy(), + desiredOp: StrategyPatchOp( + opId: const Uuid().v4(), + expectedStrategyRevision: header.revision, + payload: { + 'mapData': mapData, + if (strategyTheme.profileId != null) + 'themeProfileId': strategyTheme.profileId + else + 'clearThemeProfileId': true, + if (themeOverridePalette != null) + 'themeOverridePalette': themeOverridePalette + else + 'clearThemeOverridePalette': true, + }, + ), + flushImmediately: flushImmediately, + ); + // The outbox holds the change now. Left set, it would let a later save + // write this map over a teammate's that landed after this one did. + if (changes == _mapOrThemeChanges && + queueNotifier.persistenceFailureCount == failuresBefore) { + _mapOrThemeChangedIn = null; + } } void setUnsaved() { diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 2e6039b4b..18bfd6dd5 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -10046,6 +10046,37 @@ void main() { expect(container.read(mapProvider).currentMap, MapValue.haven); }); + test( + "a teammate's map stays when it lands after the user's own change " + 'did, and the user then edits the page', () async { + final (container, _) = await open(); + container.read(mapProvider.notifier).updateMap(MapValue.bind); + await _until(() => server.strategyMap == 'bind'); + await drained(container); + + // Mid-drag, so the teammate's map waits to be drawn. + container.read(editorPointersProvider.notifier).down(1); + server.teammateSetsMap('haven'); + showServer(); + await settle(); + expect(container.read(mapProvider).currentMap, MapValue.bind); + + container.read(agentProvider.notifier).addAgent(PlacedAgent( + id: 'agent-1', + type: AgentType.jett, + position: const Offset(40, 40), + )); + container.read(editorPointersProvider.notifier).release(1); + await settle(); + await container + .read(strategyProvider.notifier) + .forceSaveNow('cloud-strategy'); + await drained(container); + + expect(server.strategyMap, 'haven'); + expect(container.read(mapProvider).currentMap, MapValue.haven); + }); + test( 'a map change sent with no answer, then set back, is written back ' 'on the server too', () async { From 1b5f85dbc833c72db5c9249643f0a00a6ad79488 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 21:35:32 -0500 Subject: [PATCH 3/5] Send only the part of the map and theme the user changed A theme change carried the canvas's map with it. When a teammate's map had arrived but was not drawn yet, that wrote the old map back over it. --- lib/providers/strategy_provider.dart | 109 ++++++++++-------- test/strategy_page_session_provider_test.dart | 34 +++++- 2 files changed, 97 insertions(+), 46 deletions(-) diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index e2a4f0c73..97b86018e 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -157,15 +157,17 @@ class StrategyProvider extends Notifier { bool _cloudStrategyMutationSyncScheduled = false; int _persistenceTrackingSuspensionCount = 0; - /// The strategy whose map or theme the user changed on the canvas, until - /// the server or the outbox holds what the canvas shows. A canvas that - /// differs from the server without it was not changed here: a teammate - /// changed the server's (see [syncStrategyMetadata]). - String? _mapOrThemeChangedIn; + /// The map and theme, whichever the user changed on the canvas of + /// [_partsChangedIn], until the server or the outbox holds what the canvas + /// shows. Only these are sent: a part the canvas shows differently from + /// the server without the user changing it was changed by a teammate (see + /// [syncStrategyMetadata]). + final Set<_StrategyPart> _partsChanged = {}; + String? _partsChangedIn; /// Counts the user's map and theme changes, so a change made while an /// earlier one is being stored in the outbox is not taken as stored too. - int _mapOrThemeChanges = 0; + int _partChanges = 0; void _registerPersistenceTrackingListeners() { _listenForPageBackedState(agentProvider); @@ -186,8 +188,11 @@ class StrategyProvider extends Notifier { _listenForPageBackedState(strategySettingsProvider); _listenForPageBackedState(mapProvider.select((map) => map.isAttack)); - _listenForStrategyBackedState(mapProvider.select((map) => map.currentMap)); - _listenForStrategyBackedState(strategyThemeProvider); + _listenForStrategyBackedState( + mapProvider.select((map) => map.currentMap), + _StrategyPart.map, + ); + _listenForStrategyBackedState(strategyThemeProvider, _StrategyPart.theme); } void _listenForPageBackedState(ProviderListenable provider) { @@ -199,13 +204,18 @@ class StrategyProvider extends Notifier { }); } - void _listenForStrategyBackedState(ProviderListenable provider) { + void _listenForStrategyBackedState( + ProviderListenable provider, + _StrategyPart part, + ) { ref.listen(provider, (_, __) { if (!_shouldTrackPersistedEditorMutation()) { return; } - _mapOrThemeChangedIn = state.strategyId; - _mapOrThemeChanges += 1; + if (_partsChangedIn != state.strategyId) _partsChanged.clear(); + _partsChangedIn = state.strategyId; + _partsChanged.add(part); + _partChanges += 1; _markStrategyBackedStateUnsaved(); }); } @@ -632,21 +642,21 @@ class StrategyProvider extends Notifier { refreshAutosaveScheduling(); } - /// Queues the canvas's map and theme for the server, when the user changed - /// them. Every save asks, so this is where a difference between the canvas - /// and the server is told apart: + /// Queues the user's change to the map or theme for the server. Every + /// save asks, so this is where a difference between the canvas and the + /// server is told apart: /// /// - The outbox already holds what the canvas shows: that change waits as /// it is. /// - The server holds what the canvas shows: nothing is sent, and a change /// still waiting is dropped, unless one may still land and undo that - /// (see strategyPatchMayLand); then the canvas's values are written - /// behind it. - /// - They differ and the user changed neither: a teammate changed the - /// server's. Nothing is sent, and the canvas takes the server's when the - /// page is next brought up to date. - /// - Otherwise the user's change is queued, against the revision this - /// device knows the server holds. + /// (see strategyPatchMayLand); then the canvas's map and theme are + /// written behind it. + /// - Otherwise only what the user changed is queued, against the revision + /// this device knows the server holds. A part they left alone that + /// differs from the server's was changed by a teammate: it is not sent, + /// and the canvas takes the server's when the page is next brought up + /// to date. Future syncStrategyMetadata({bool flushImmediately = false}) async { final strategyId = state.strategyId; final snapshot = ref.read(remoteEditorSnapshotProvider).valueOrNull; @@ -664,32 +674,35 @@ class StrategyProvider extends Notifier { themeProfileId: strategyTheme.profileId, themeOverridePalette: themeOverridePalette, ); + final changed = + _partsChangedIn == strategyId ? {..._partsChanged} : <_StrategyPart>{}; // The server's, as this device knows it holds them. final header = knownStrategyHeader(ref, snapshot.header); final queue = ref.read(strategyOpQueueProvider); + final queueNotifier = ref.read(strategyOpQueueProvider.notifier); final waiting = waitingStrategyPatch(queue, strategyId); if (waiting != null && shows(header.withPatch(waiting.payload))) { - _mapOrThemeChangedIn = null; + _partsChanged.clear(); return; } if (shows(header)) { - _mapOrThemeChangedIn = null; + _partsChanged.clear(); if (!strategyPatchMayLand(queue, strategyId)) { - await ref.read(strategyOpQueueProvider.notifier).syncDesiredGenericOp( - entityKey: const EntitySyncKey.strategy(), - desiredOp: null, - flushImmediately: flushImmediately, - ); + await queueNotifier.syncDesiredGenericOp( + entityKey: const EntitySyncKey.strategy(), + desiredOp: null, + flushImmediately: flushImmediately, + ); return; } - } else if (waiting == null && _mapOrThemeChangedIn != strategyId) { - return; + // Whatever lands is undone whole. + changed.addAll(_StrategyPart.values); } + if (changed.isEmpty) return; - final changes = _mapOrThemeChanges; - final queueNotifier = ref.read(strategyOpQueueProvider.notifier); + final changes = _partChanges; final failuresBefore = queueNotifier.persistenceFailureCount; await queueNotifier.syncDesiredGenericOp( entityKey: const EntitySyncKey.strategy(), @@ -697,24 +710,26 @@ class StrategyProvider extends Notifier { opId: const Uuid().v4(), expectedStrategyRevision: header.revision, payload: { - 'mapData': mapData, - if (strategyTheme.profileId != null) - 'themeProfileId': strategyTheme.profileId - else - 'clearThemeProfileId': true, - if (themeOverridePalette != null) - 'themeOverridePalette': themeOverridePalette - else - 'clearThemeOverridePalette': true, + if (changed.contains(_StrategyPart.map)) 'mapData': mapData, + if (changed.contains(_StrategyPart.theme)) ...{ + if (strategyTheme.profileId != null) + 'themeProfileId': strategyTheme.profileId + else + 'clearThemeProfileId': true, + if (themeOverridePalette != null) + 'themeOverridePalette': themeOverridePalette + else + 'clearThemeOverridePalette': true, + }, }, ), flushImmediately: flushImmediately, ); - // The outbox holds the change now. Left set, it would let a later save - // write this map over a teammate's that landed after this one did. - if (changes == _mapOrThemeChanges && + // The outbox holds the change now. Left remembered, it would let a later + // save write it over a teammate's change that landed after this one did. + if (changes == _partChanges && queueNotifier.persistenceFailureCount == failuresBefore) { - _mapOrThemeChangedIn = null; + _partsChanged.clear(); } } @@ -3001,3 +3016,7 @@ class _PageCopyRoots { final Set placedRoots; final Set lineUpRoots; } + +/// A part of a cloud strategy's header the canvas shows and the user can +/// change. +enum _StrategyPart { map, theme } diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 18bfd6dd5..d6d356dee 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -1701,7 +1701,8 @@ void main() { op.payload, containsPair('mapData', Maps.mapNames[MapValue.bind]), ); - expect(op.payload, containsPair('clearThemeProfileId', true)); + // Only what the user changed goes: the theme is the server's to keep. + expect((op.payload! as Map).keys, ['mapData']); }); test('cloud page reorder is persisted as a page descriptor op', () async { @@ -10077,6 +10078,37 @@ void main() { expect(container.read(mapProvider).currentMap, MapValue.haven); }); + test( + "a theme change leaves a teammate's map the canvas has not drawn " + 'yet', () async { + final (container, _) = await open(); + // Mid-drag, so the teammate's map waits to be drawn. + container.read(editorPointersProvider.notifier).down(1); + server.teammateSetsMap('haven'); + showServer(); + await settle(); + expect(container.read(mapProvider).currentMap, MapValue.ascent); + + container + .read(strategyThemeProvider.notifier) + .setProfile(MapThemeProfilesProvider.immutableValorantProfileId); + container.read(editorPointersProvider.notifier).release(1); + await settle(); + await container + .read(strategyProvider.notifier) + .forceSaveNow('cloud-strategy'); + await drained(container); + showServer(); + await settle(); + + expect(server.strategyMap, 'haven'); + expect(server.strategyThemeProfileId, + MapThemeProfilesProvider.immutableValorantProfileId); + expect(container.read(mapProvider).currentMap, MapValue.haven); + expect(container.read(strategyThemeProvider).profileId, + MapThemeProfilesProvider.immutableValorantProfileId); + }); + test( 'a map change sent with no answer, then set back, is written back ' 'on the server too', () async { From d1f6c93907b472278006c63bd50d04001ea72384 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 21:51:23 -0500 Subject: [PATCH 4/5] Carry the waiting strategy change in the one queued over it The outbox keeps one change to the strategy and takes each new one as the whole of what is wanted. A change naming only the theme, queued behind one naming only the map, dropped the map. --- lib/providers/strategy_provider.dart | 36 ++++++++++++++++++---------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index 97b86018e..e67c8a330 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -702,6 +702,28 @@ class StrategyProvider extends Notifier { } if (changed.isEmpty) return; + // The outbox keeps one change to the strategy, so this one carries what + // the change still waiting does, with the user's newer values over it. + final payload = {...?waiting?.payload}; + if (changed.contains(_StrategyPart.map)) payload['mapData'] = mapData; + if (changed.contains(_StrategyPart.theme)) { + payload + ..remove('themeProfileId') + ..remove('clearThemeProfileId') + ..remove('themeOverridePalette') + ..remove('clearThemeOverridePalette'); + if (strategyTheme.profileId != null) { + payload['themeProfileId'] = strategyTheme.profileId; + } else { + payload['clearThemeProfileId'] = true; + } + if (themeOverridePalette != null) { + payload['themeOverridePalette'] = themeOverridePalette; + } else { + payload['clearThemeOverridePalette'] = true; + } + } + final changes = _partChanges; final failuresBefore = queueNotifier.persistenceFailureCount; await queueNotifier.syncDesiredGenericOp( @@ -709,19 +731,7 @@ class StrategyProvider extends Notifier { desiredOp: StrategyPatchOp( opId: const Uuid().v4(), expectedStrategyRevision: header.revision, - payload: { - if (changed.contains(_StrategyPart.map)) 'mapData': mapData, - if (changed.contains(_StrategyPart.theme)) ...{ - if (strategyTheme.profileId != null) - 'themeProfileId': strategyTheme.profileId - else - 'clearThemeProfileId': true, - if (themeOverridePalette != null) - 'themeOverridePalette': themeOverridePalette - else - 'clearThemeOverridePalette': true, - }, - }, + payload: payload, ), flushImmediately: flushImmediately, ); From 7fe64a578a216a2aabf77244793b61b34a22f5f1 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 23:38:31 -0500 Subject: [PATCH 5/5] Keep the canvas's map and theme each only while its own change waits A redraw kept both whenever any strategy change waited. With only a theme change waiting, that drew the canvas's old map over a teammate's newer one when the user took the cloud's version of an item. --- .../strategy_page_session_provider.dart | 65 ++++++++++--------- lib/providers/strategy_provider.dart | 1 + test/strategy_page_session_provider_test.dart | 47 ++++++++++++++ 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/lib/providers/strategy_page_session_provider.dart b/lib/providers/strategy_page_session_provider.dart index 429670e16..912056ff0 100644 --- a/lib/providers/strategy_page_session_provider.dart +++ b/lib/providers/strategy_page_session_provider.dart @@ -998,7 +998,7 @@ class StrategyPageSessionNotifier extends Notifier { required String strategyId, required bool Function() canApply, required RemoteEditorSnapshot loadedRemoteSnapshot, - ({MapValue map, StrategyThemeState theme})? preservedMetadata, + ({MapValue? map, StrategyThemeState? theme})? preservedMetadata, }) async { final pageId = pageData.pageId; final liveSync = ref.read(activePageLiveSyncProvider.notifier); @@ -1053,28 +1053,28 @@ class StrategyPageSessionNotifier extends Notifier { ); } - /// The strategy's map and theme on screen, while a change to them is still - /// on its way: loading a page must keep them rather than take the older - /// server copy (and so drop the change). - ({MapValue map, StrategyThemeState theme})? _unsentLocalMetadata() { - final hasPendingMetadata = ref.read(strategyOpQueueProvider).pending.any( - (pending) { - final op = pending.op; - return op is StrategyPatchOp && - op.payload.keys.any((key) => - key == 'mapData' || - key == 'themeProfileId' || - key == 'clearThemeProfileId' || - key == 'themeOverridePalette' || - key == 'clearThemeOverridePalette'); - }, + /// The strategy's map and theme on screen, each while a change to it is + /// still on its way: loading a page must keep it rather than take the + /// older server copy (and so drop the change). A change names only what + /// the user changed, so each is kept on its own: with only a theme change + /// waiting, the map is the server's, which may be a teammate's newer one. + ({MapValue? map, StrategyThemeState? theme})? _unsentLocalMetadata() { + final waiting = { + for (final pending in ref.read(strategyOpQueueProvider).pending) + if (pending.op case StrategyPatchOp(:final payload)) ...payload.keys, + }; + final mapWaits = waiting.contains('mapData'); + final themeWaits = waiting.any(const { + 'themeProfileId', + 'clearThemeProfileId', + 'themeOverridePalette', + 'clearThemeOverridePalette', + }.contains); + if (!mapWaits && !themeWaits) return null; + return ( + map: mapWaits ? ref.read(mapProvider).currentMap : null, + theme: themeWaits ? ref.read(strategyThemeProvider) : null, ); - return hasPendingMetadata - ? ( - map: ref.read(mapProvider).currentMap, - theme: ref.read(strategyThemeProvider), - ) - : null; } bool get isApplyingPage => state.isApplyingPage; @@ -1240,7 +1240,7 @@ class StrategyPageSessionNotifier extends Notifier { _RemotePageHydrationKey? hydrationKey, bool preserveTextDrafts = false, RemoteEditorSnapshot? loadedRemoteSnapshot, - ({MapValue map, StrategyThemeState theme})? preservedMetadata, + ({MapValue? map, StrategyThemeState? theme})? preservedMetadata, bool Function()? canApply, OpenAttempt? attempt, }) async { @@ -1254,11 +1254,12 @@ class StrategyPageSessionNotifier extends Notifier { final preserveHistory = source == StrategySource.cloud && _lastHydratedRemotePageKey?.strategyPublicId == strategyId && _lastHydratedRemotePageKey?.pageId == pageData.pageId; - final themeProfileId = preservedMetadata != null - ? preservedMetadata.theme.profileId + final preservedTheme = preservedMetadata?.theme; + final themeProfileId = preservedTheme != null + ? preservedTheme.profileId : _resolveThemeProfileId(source, strategyId); - final themeOverridePalette = preservedMetadata != null - ? preservedMetadata.theme.overridePalette + final themeOverridePalette = preservedTheme != null + ? preservedTheme.overridePalette : _resolveThemeOverridePalette(source, strategyId); state = state.copyWith( @@ -1767,7 +1768,7 @@ class StrategyPageSessionNotifier extends Notifier { required String strategyId, required RemoteEditorSnapshot snapshot, Set? holding, - ({MapValue map, StrategyThemeState theme})? preservedMetadata, + ({MapValue? map, StrategyThemeState? theme})? preservedMetadata, }) { final pageId = pageData.pageId; final liveSync = ref.read(activePageLiveSyncProvider.notifier); @@ -1780,11 +1781,11 @@ class StrategyPageSessionNotifier extends Notifier { pageData, changed: liveSync.remoteChangesSinceHydration(snapshot, pageId), holding: holding ?? ref.read(editorHeldEntitiesProvider)!, - themeProfileId: preservedMetadata != null - ? preservedMetadata.theme.profileId + themeProfileId: preservedMetadata?.theme != null + ? preservedMetadata!.theme!.profileId : _resolveThemeProfileId(StrategySource.cloud, strategyId), - themeOverridePalette: preservedMetadata != null - ? preservedMetadata.theme.overridePalette + themeOverridePalette: preservedMetadata?.theme != null + ? preservedMetadata!.theme!.overridePalette : _resolveThemeOverridePalette(StrategySource.cloud, strategyId), mapOverride: preservedMetadata?.map, ); diff --git a/lib/providers/strategy_provider.dart b/lib/providers/strategy_provider.dart index e67c8a330..3ac33db67 100644 --- a/lib/providers/strategy_provider.dart +++ b/lib/providers/strategy_provider.dart @@ -738,6 +738,7 @@ class StrategyProvider extends Notifier { // The outbox holds the change now. Left remembered, it would let a later // save write it over a teammate's change that landed after this one did. if (changes == _partChanges && + state.strategyId == strategyId && queueNotifier.persistenceFailureCount == failuresBefore) { _partsChanged.clear(); } diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 19f7e9f08..4b94f1cc4 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -10109,6 +10109,53 @@ void main() { MapThemeProfilesProvider.immutableValorantProfileId); }); + test( + "taking the cloud's version of an item draws a teammate's map, " + 'with only a theme change waiting', () async { + final page = _page('page-1', 0); + server = _FakeServer(page.publicId, elements: [ + _textElement(page.publicId, 'a', 'alpha', worldSized: true), + ]); + final (container, _) = + await openOnRealQueue(page, themeProfileId: defaultTheme); + await settle(); + final key = EntitySyncKey.element(page.publicId, 'a'); + StrategyOpQueueState queue() => container.read(strategyOpQueueProvider); + + // The server refuses the user's edit to the text: they are asked. + repository.refuse = (op) => op is ElementPatchOp + ? RejectedOpAck( + opId: op.opId, + rejectionReason: OpRejectionReason.revisionMismatch, + ) + : null; + container.read(textProvider.notifier).commitText('a', 'mine'); + await _until(() => queue().attentionByEntityKey.containsKey(key)); + await settle(); + + // A theme change waits, held on its way, when a teammate's map + // arrives. The canvas is not saved, so the map is not drawn yet. + repository.hold = Completer(); + container + .read(strategyThemeProvider.notifier) + .setProfile(MapThemeProfilesProvider.immutableValorantProfileId); + await settle(); + server.teammateSetsMap('haven'); + showServer(); + await settle(); + expect(container.read(mapProvider).currentMap, MapValue.ascent); + + await container + .read(strategyPageSessionProvider.notifier) + .useCloudVersionsForRejected(); + await settle(); + + expect(container.read(mapProvider).currentMap, MapValue.haven); + expect(container.read(strategyThemeProvider).profileId, + MapThemeProfilesProvider.immutableValorantProfileId); + repository.hold!.complete(); + }); + test( 'a map change sent with no answer, then set back, is written back ' 'on the server too', () async {