Skip to content

Commit

Permalink
fix: add listener lifecycle management and confirmSetup error handling (
Browse files Browse the repository at this point in the history 
#52, #53)

- Track all Firebase listeners via cleanup runnables in AndroidFirebase
- Add removeAllListeners() to FirebaseAPI interface
- Add stop() to GameNetworkHandler, called from GameScreen hide/dispose
- Fix confirmSetup: propagate errors to onError callback instead of
  silently calling onSuccess on failure paths
- Update SetupFlowController to pass error callback to confirmSetup
  • Loading branch information
Francin Vincent committed Apr 6, 2026
1 parent 96800a0 commit 7d71a4b
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 31 deletions.
69 changes: 44 additions & 25 deletions android/src/main/java/com/group14/regicidechess/android/AndroidFirebase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,17 @@
public class AndroidFirebase implements FirebaseAPI, API {

private final DatabaseReference db = FirebaseDatabase.getInstance().getReference();
private final List<Runnable> cleanupActions = new ArrayList<>();

private ValueEventListener trackValue(DatabaseReference ref, ValueEventListener listener) {
cleanupActions.add(() -> ref.removeEventListener(listener));
return listener;
}

private ChildEventListener trackChild(DatabaseReference ref, ChildEventListener listener) {
cleanupActions.add(() -> ref.removeEventListener(listener));
return listener;
}

// ── API (Legacy) ──────────────────────────────────────────────────────────

Expand Down Expand Up @@ -74,8 +85,8 @@ public void joinLobby(String gameId, Callback<Lobby> onSuccess, Callback<String>

@Override
public void listenForOpponentReady(String gameId, Runnable onReady) {
db.child("lobbies").child(gameId).child("status")
.addValueEventListener(new ValueEventListener() {
DatabaseReference statusRef = db.child("lobbies").child(gameId).child("status");
statusRef.addValueEventListener(trackValue(statusRef, new ValueEventListener() {
@Override
public void onDataChange(DataSnapshot snapshot) {
if ("joined".equals(snapshot.getValue(String.class))) {
Expand All @@ -84,7 +95,7 @@ public void onDataChange(DataSnapshot snapshot) {
}
}
@Override public void onCancelled(DatabaseError e) {}
});
}));
}

@Override
Expand All @@ -94,8 +105,8 @@ public void startGame(String gameId) {

@Override
public void listenForGameStart(String gameId, Runnable onStart) {
db.child("lobbies").child(gameId).child("status")
.addValueEventListener(new ValueEventListener() {
DatabaseReference statusRef = db.child("lobbies").child(gameId).child("status");
statusRef.addValueEventListener(trackValue(statusRef, new ValueEventListener() {
@Override
public void onDataChange(DataSnapshot snapshot) {
if ("started".equals(snapshot.getValue(String.class))) {
Expand All @@ -104,13 +115,13 @@ public void onDataChange(DataSnapshot snapshot) {
}
}
@Override public void onCancelled(DatabaseError e) {}
});
}));
}

// ── Setup ─────────────────────────────────────────────────────────────────

@Override
public void confirmSetup(String gameId, boolean isWhite, int[][] board, Runnable onSuccess) {
public void confirmSetup(String gameId, boolean isWhite, int[][] board, Runnable onSuccess, Callback<String> onError) {
String player = isWhite ? "white" : "black";
DatabaseReference gameRef = db.child("games").child(gameId);

Expand Down Expand Up @@ -141,11 +152,12 @@ public void onDataChange(DataSnapshot snapshot) {
}
onSuccess.run();
}
@Override public void onCancelled(DatabaseError e) { onSuccess.run(); }
@Override public void onCancelled(DatabaseError e) { onError.call(e.getMessage()); }
})
)
.addOnFailureListener(e -> onError.call(e.getMessage()))
)
.addOnFailureListener(e -> onSuccess.run());
.addOnFailureListener(e -> onError.call(e.getMessage()));
}

@Override
Expand All @@ -172,8 +184,8 @@ public void getOpponentBoard(String gameId, boolean localIsWhite, Callback<int[]

@Override
public void listenForBothSetupReady(String gameId, Runnable onBothReady) {
db.child("games").child(gameId).child("bothReady")
.addValueEventListener(new ValueEventListener() {
DatabaseReference bothReadyRef = db.child("games").child(gameId).child("bothReady");
bothReadyRef.addValueEventListener(trackValue(bothReadyRef, new ValueEventListener() {
@Override
public void onDataChange(DataSnapshot snapshot) {
Boolean bothReady = snapshot.getValue(Boolean.class);
Expand All @@ -183,7 +195,7 @@ public void onDataChange(DataSnapshot snapshot) {
}
}
@Override public void onCancelled(DatabaseError e) {}
});
}));
}

// ── Moves ─────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -221,8 +233,8 @@ public void listenForOpponentMove(String gameId, Callback<int[]> onMove) {
lastExistingKey[0] = child.getKey();
}

db.child("games").child(gameId).child("moves")
.addChildEventListener(new ChildEventListener() {
DatabaseReference movesRef = db.child("games").child(gameId).child("moves");
movesRef.addChildEventListener(trackChild(movesRef, new ChildEventListener() {
private boolean seenStart = (lastExistingKey[0] == null);

@Override
Expand Down Expand Up @@ -257,12 +269,12 @@ public void onChildAdded(DataSnapshot snapshot, String prev) {
@Override public void onChildRemoved(DataSnapshot s) {}
@Override public void onChildMoved (DataSnapshot s, String p) {}
@Override public void onCancelled (DatabaseError e) {}
});
}));
})
.addOnFailureListener(e -> {
// Fallback: attach listener without filtering (original behaviour)
db.child("games").child(gameId).child("moves")
.addChildEventListener(new ChildEventListener() {
DatabaseReference movesRef = db.child("games").child(gameId).child("moves");
movesRef.addChildEventListener(trackChild(movesRef, new ChildEventListener() {
@Override
public void onChildAdded(DataSnapshot snapshot, String prev) {
int fromCol = getInt(snapshot, "fromCol");
Expand All @@ -277,7 +289,7 @@ public void onChildAdded(DataSnapshot snapshot, String prev) {
@Override public void onChildRemoved(DataSnapshot s) {}
@Override public void onChildMoved (DataSnapshot s, String p) {}
@Override public void onCancelled (DatabaseError e) {}
});
}));
});
}

Expand All @@ -296,12 +308,13 @@ public void listenForHeartbeat(String gameId, boolean listenForWhite,
String player = listenForWhite ? "white" : "black";
Handler handler = new Handler(Looper.getMainLooper());
Runnable timeoutRunnable = onTimeout::run;
cleanupActions.add(() -> handler.removeCallbacks(timeoutRunnable));

// Start timeout timer — reset each time a valid heartbeat arrives
handler.postDelayed(timeoutRunnable, timeoutMs);

db.child("games").child(gameId).child("heartbeat").child(player)
.addValueEventListener(new ValueEventListener() {
DatabaseReference heartbeatRef = db.child("games").child(gameId).child("heartbeat").child(player);
heartbeatRef.addValueEventListener(trackValue(heartbeatRef, new ValueEventListener() {
@Override
public void onDataChange(DataSnapshot snapshot) {
Long timestamp = snapshot.getValue(Long.class);
Expand All @@ -319,7 +332,7 @@ public void onDataChange(DataSnapshot snapshot) {
onHeartbeat.call(latency);
}
@Override public void onCancelled(DatabaseError e) {}
});
}));
}

// ── Game over ─────────────────────────────────────────────────────────────
Expand All @@ -331,8 +344,8 @@ public void signalGameOver(String gameId, String reason) {

@Override
public void listenForGameOver(String gameId, Callback<String> onGameOver) {
db.child("games").child(gameId).child("gameOver")
.addValueEventListener(new ValueEventListener() {
DatabaseReference gameOverRef = db.child("games").child(gameId).child("gameOver");
gameOverRef.addValueEventListener(trackValue(gameOverRef, new ValueEventListener() {
@Override
public void onDataChange(DataSnapshot snapshot) {
String reason = snapshot.getValue(String.class);
Expand All @@ -342,7 +355,7 @@ public void onDataChange(DataSnapshot snapshot) {
}
}
@Override public void onCancelled(DatabaseError e) {}
});
}));
}

// ─────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -370,4 +383,10 @@ public void unconfirmSetup(String gameId, boolean isWhite, Runnable onSuccess, C
})
.addOnFailureListener(e -> onError.call(e.getMessage()));
}
}

@Override
public void removeAllListeners() {
for (Runnable cleanup : cleanupActions) cleanup.run();
cleanupActions.clear();
}
}
6 changes: 4 additions & 2 deletions core/src/main/java/com/group14/regicidechess/database/FirebaseAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public interface FirebaseAPI {
* Saves this player's board layout and marks them as setup-ready.
* Sets games/{gameId}/bothReady = true once BOTH players have called this.
*/
void confirmSetup(String gameId, boolean isWhite, int[][] board, Runnable onSuccess);
void confirmSetup(String gameId, boolean isWhite, int[][] board, Runnable onSuccess, Callback<String> onError);

/**
* Unconfirms the setup, marking this player as not ready.
Expand Down Expand Up @@ -80,9 +80,11 @@ void listenForHeartbeat(String gameId, boolean listenForWhite, long timeoutMs,
void signalGameOver(String gameId, String reason);
void listenForGameOver(String gameId, Callback<String> onGameOver);

void removeAllListeners();

// ─────────────────────────────────────────────────────────────────────────

interface Callback<T> {
void call(T value);
}
}
}
6 changes: 5 additions & 1 deletion core/src/main/java/com/group14/regicidechess/screens/game/GameNetworkHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ public void start() {
startHeartbeat();
}

public void stop() {
DatabaseManager.getInstance().getApi().removeAllListeners();
}

// ─────────────────────────────────────────────────────────────────────────
// Outgoing
// ─────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -148,4 +152,4 @@ private void updateConnectionUI(long latencyMs) {
else connectionIcon.setColor(Color.RED);
connectionLabel.setText(latencyMs + " ms");
}
}
}
4 changes: 3 additions & 1 deletion core/src/main/java/com/group14/regicidechess/screens/game/GameScreen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,12 +438,14 @@ public void resize(int width, int height) {
@Override
public void hide() {
inputHandler.clearObservers();
networkHandler.stop();
inMatchState.exit();
}

@Override
public void dispose() {
networkHandler.stop();
stage.dispose();
boardRenderer.dispose();
}
}
}
5 changes: 3 additions & 2 deletions core/src/main/java/com/group14/regicidechess/screens/setup/SetupFlowController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ public void confirm() {
gameId,
localPlayer.isWhite(),
encoded,
this::listenForBothReady
this::listenForBothReady,
error -> Gdx.app.postRunnable(() -> listener.onError("Setup failed: " + error))
);
}

Expand Down Expand Up @@ -144,4 +145,4 @@ private void fetchOpponentBoard() {
public boolean isConfirmed() {
return isConfirmed;
}
}
}

0 comments on commit 7d71a4b

Please sign in to comment.