Skip to content

Commit 6e4335e

Browse files
DOne
1 parent 786fe68 commit 6e4335e

4 files changed

Lines changed: 36 additions & 125 deletions

File tree

core/multiplayer/src/main/kotlin/com/battleship/fleetcommand/core/multiplayer/repository/FirebaseMatchRepositoryImpl.kt

Lines changed: 13 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,6 @@ class FirebaseMatchRepositoryImpl @Inject constructor(
216216
}
217217
}
218218

219-
/**
220-
* Legacy single-field write. Kept to satisfy the interface contract and for test
221-
* compatibility. In production the VM calls [commitShotAndFlipTurn] instead.
222-
*/
223219
override suspend fun writeShotResult(
224220
gameId: String,
225221
shooterUid: String,
@@ -233,44 +229,16 @@ class FirebaseMatchRepositoryImpl @Inject constructor(
233229
val pushKey = snapshot.children.elementAtOrNull(shotIndex)?.key ?: return
234230

235231
val shotNode = shotsRef.child(pushKey)
232+
// Discrete setValue calls avoid the multi-path updateChildren null/delete rule bug
236233
shotNode.child(FirebaseSchema.SHOT_RESULT).setValue(result.toSchemaString()).await()
237-
shotNode.child(FirebaseSchema.SHOT_SHIP_ID).setValue(shipId).await()
234+
if (shipId != null) {
235+
shotNode.child(FirebaseSchema.SHOT_SHIP_ID).setValue(shipId).await()
236+
}
238237
} catch (e: Exception) {
239238
Timber.w(e, "writeShotResult failed for game=$gameId shooter=$shooterUid index=$shotIndex")
240239
}
241240
}
242241

243-
/**
244-
* Writes the shot result + shipId AND flips currentTurn using TWO sequential writes.
245-
*
246-
* ── WHY NOT a single updateChildren() from game root? ──
247-
* The Firebase Security Rules for /shots/{shooterUid} grant write access ONLY to
248-
* auth.uid === $shooterUid (the ATTACKER). This function is called by the DEFENDER
249-
* (the person whose ships were hit). Even though the nested
250-
* /shots/{shooterUid}/{pushKey}/result node has its own rule allowing the defender
251-
* to write THAT specific field, a multi-path updateChildren() rooted at the game
252-
* node includes paths under /shots/{shooterUid}/ — Firebase evaluates the ancestor
253-
* rule (auth.uid === $shooterUid) and rejects the ENTIRE batch write for the
254-
* defender. Result: result is never written, turn never flips, game is stuck.
255-
*
256-
* Fix: Two separate writes scoped to the exact paths each party is allowed to write:
257-
* Write 1 — defender writes result + shipId directly on the shot's push-key node.
258-
* The $shotPushKey parent rule allows the defender to write here
259-
* (see database.rules.json — $shotPushKey allows both parties to write
260-
* when the shot is unresolved, and result sub-rule allows non-shooter).
261-
* Write 2 — defender writes meta/currentTurn (allowed: any authenticated player
262-
* in the game may write to /meta per its .write rule).
263-
*
264-
* The intermediate state (result written, turn not yet flipped) is observable for
265-
* ~100ms on slow connections, but this is far preferable to the turn being
266-
* permanently stuck. The attacker's ViewModel guards against acting on the
267-
* intermediate state: isMyTurn is set to false IMMEDIATELY when they fire (before
268-
* Firebase confirms), so there is no "turn re-enables" jank on the attacker side.
269-
*
270-
* Firebase path writes:
271-
* Call 1: games/{gameId}/shots/{shooterUid}/{pushKey}/updateChildren({result, shipId})
272-
* Call 2: games/{gameId}/meta/currentTurn = nextTurnUid
273-
*/
274242
override suspend fun commitShotAndFlipTurn(
275243
gameId: String,
276244
shooterUid: String,
@@ -280,28 +248,24 @@ class FirebaseMatchRepositoryImpl @Inject constructor(
280248
nextTurnUid: String,
281249
): Result<Unit> {
282250
return try {
283-
// ── Step 1: Resolve push-key for this shot by index ─────────────
284251
val shotsRef = database.getReference(
285252
"${FirebaseSchema.GAMES}/$gameId/${FirebaseSchema.SHOTS}/$shooterUid"
286253
)
287254
val snapshot = shotsRef.get().await()
288255
val pushKey = snapshot.children.elementAtOrNull(shotIndex)?.key
289256
?: return Result.failure(Exception("Shot push-key not found at index $shotIndex"))
290257

291-
// ── Step 2: Write result + shipId on the shot push-key node ─────
292-
// updateChildren() scoped to the push-key node — the defender is allowed
293-
// to write result here per security rules. shipId inherits the push-key
294-
// node's rule which now permits non-shooter writes (see database.rules.json).
295258
val shotNodeRef = shotsRef.child(pushKey)
296-
val shotUpdate = mapOf<String, Any?>(
297-
FirebaseSchema.SHOT_RESULT to result.toSchemaString(),
298-
FirebaseSchema.SHOT_SHIP_ID to shipId,
299-
)
300-
shotNodeRef.updateChildren(shotUpdate).await()
259+
260+
// ── Step 1: Write result ─────────────────────────────────────────
261+
shotNodeRef.child(FirebaseSchema.SHOT_RESULT).setValue(result.toSchemaString()).await()
262+
263+
// ── Step 2: Write shipId ONLY if not null ────────────────────────
264+
if (shipId != null) {
265+
shotNodeRef.child(FirebaseSchema.SHOT_SHIP_ID).setValue(shipId).await()
266+
}
301267

302-
// ── Step 3: Flip currentTurn to the defender ────────────────────
303-
// nextTurnUid = myUid on the defender's device (it becomes the defender's
304-
// turn to fire after they resolve the attacker's shot).
268+
// ── Step 3: Flip currentTurn to the defender ─────────────────────
305269
database.getReference(
306270
"${FirebaseSchema.GAMES}/$gameId/${FirebaseSchema.META}/${FirebaseSchema.CURRENT_TURN}"
307271
).setValue(nextTurnUid).await()

feature/game/src/main/kotlin/com/battleship/fleetcommand/feature/game/battle/BattleScreen.kt

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,12 @@ fun BattleScreen(
4343
val snackbarHostState = remember { SnackbarHostState() }
4444
var showResignDialog by remember { mutableStateOf(false) }
4545

46-
// Pass & Play: observe saved state set by HandOffScreen when returning to this screen
4746
val passAndPlayResumeP1 = navController.currentBackStackEntry
4847
?.savedStateHandle
4948
?.getStateFlow<Boolean?>("passAndPlayResumeP1", null)
5049
?.collectAsState()
5150
LaunchedEffect(passAndPlayResumeP1?.value) {
5251
val resumeP1 = passAndPlayResumeP1?.value ?: return@LaunchedEffect
53-
// Clear the key so it doesn't fire again on recomposition
5452
navController.currentBackStackEntry?.savedStateHandle?.remove<Boolean>("passAndPlayResumeP1")
5553
if (resumeP1) {
5654
viewModel.onEvent(BattleViewModel.UiEvent.PassAndPlayResumeP1Turn)
@@ -59,7 +57,6 @@ fun BattleScreen(
5957
}
6058
}
6159

62-
// Section 12 BackHandler — shows resign dialog
6360
BackHandler { viewModel.onEvent(BattleViewModel.UiEvent.ResignGame) }
6461

6562
LaunchedEffect(Unit) {
@@ -70,8 +67,6 @@ fun BattleScreen(
7067
popUpTo<com.battleship.fleetcommand.navigation.MainMenuRoute> { inclusive = false }
7168
}
7269
is BattleViewModel.UiEffect.NavigateToPassAndPlayHandOff -> {
73-
// FIX: pass phase="BATTLE" so HandOffScreen uses popBackStack() + savedStateHandle
74-
// (correct for mid-game turn handoffs) rather than the SETUP forward-navigate path
7570
navController.navigate(
7671
HandOffRoute(
7772
gameId = effect.gameId,
@@ -87,7 +82,6 @@ fun BattleScreen(
8782
is BattleViewModel.UiEffect.ShowSunkAnimation -> {
8883
val shipName = effect.shipId.name
8984
.lowercase().replaceFirstChar { it.uppercase() }
90-
val isMyShot = uiState.isMyTurn || uiState.mode == com.battleship.fleetcommand.core.domain.model.GameMode.AI
9185
val message = if (uiState.mode == com.battleship.fleetcommand.core.domain.model.GameMode.LOCAL) {
9286
"${if (uiState.isMyTurn) uiState.opponentName else uiState.myName}'s $shipName was sunk!"
9387
} else {
@@ -134,7 +128,6 @@ fun BattleScreen(
134128
},
135129
actions = {
136130
IconButton(onClick = { viewModel.onEvent(BattleViewModel.UiEvent.ResignGame) }) {
137-
// Flag icon is in extended icons only — Close is semantically "exit/resign"
138131
Icon(Icons.Default.Close, contentDescription = "Resign")
139132
}
140133
},
@@ -164,9 +157,6 @@ fun BattleScreen(
164157
color = MaterialTheme.colorScheme.primary,
165158
)
166159
Box {
167-
// Pass null when not the player's turn — GameCell removes the click modifier
168-
// entirely so no pointer events can fire. This is the strongest possible
169-
// spam guard: no lambda, no clickable(), no event routing at all.
170160
val canFire = uiState.isMyTurn && !uiState.isAnimating && !uiState.isAiThinking
171161
GameGrid(
172162
board = uiState.opponentBoard,

feature/game/src/main/kotlin/com/battleship/fleetcommand/feature/game/online/OnlineBattleScreen.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ fun OnlineBattleScreen(
3838
var showResignDialog by remember { mutableStateOf(false) }
3939
var showDisconnectDialog by remember { mutableStateOf(false) }
4040

41-
// Show a snackbar whenever a ship is sunk (attacker = "You sunk the X!" / defender = "Your X was sunk!")
4241
LaunchedEffect(uiState.sunkNotificationMessage) {
4342
val msg = uiState.sunkNotificationMessage ?: return@LaunchedEffect
4443
snackbarHostState.showSnackbar(
@@ -92,10 +91,9 @@ fun OnlineBattleScreen(
9291
)
9392
}
9493

95-
// 📶 DISCONNECT DIALOG — Gives player the option to take the win if opponent abandons
9694
if (showDisconnectDialog) {
9795
AlertDialog(
98-
onDismissRequest = { }, // Cannot dismiss without making a choice
96+
onDismissRequest = { },
9997
title = { Text("Opponent Disconnected") },
10098
text = { Text("Your opponent has been disconnected for over 30 seconds. Do you want to claim victory by default or keep waiting?") },
10199
confirmButton = {

0 commit comments

Comments
 (0)