Skip to content

fix: add listener lifecycle management and confirmSetup error handling #59

Merged
merged 1 commit into from
Apr 8, 2026
+59 −31
Merged

fix: add listener lifecycle management and confirmSetup error handling #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 44 additions & 25 deletions android/src/main/java/com/group14/regicidechess/android/AndroidFirebase.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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;
}
}
}