Skip to content
30 changes: 30 additions & 0 deletions src/main/java/org/example/TravelGameController.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.Button;
import javafx.scene.control.DialogPane;
import javafx.scene.control.Label;
import javafx.scene.control.ListView;
import javafx.scene.image.Image;
Expand All @@ -26,6 +27,7 @@

public class TravelGameController {

@FXML private DialogPane winConLabel;
@FXML private ImageView mapView;
@FXML private StackPane drawingPane;
@FXML private Pane gridLayer;
Expand All @@ -44,6 +46,7 @@ public class TravelGameController {
@FXML private Label currentTurnLabel;
@FXML private Label currentLocationLabel;
@FXML private Label destinationLabel;
@FXML private Label currentPointLabel;

private MapVisualizer visualizer;
private JourneyService journeyService;
Expand Down Expand Up @@ -346,6 +349,7 @@ private void updateHud() {

currentCreditsLabel.setText(current.getMoney() != null ? current.getMoney().toPlainString() : "-");
currentTurnLabel.setText(String.valueOf(current.getTurnCount()));
currentPointLabel.setText(String.valueOf(current.getPlayerScore()));

currentLocationLabel.setText("[" + clampToGrid(current.getPlayerPosX()) + "," + clampToGrid(current.getPlayerPosY()) + "]");

Expand Down Expand Up @@ -483,10 +487,12 @@ private void doMove(Long travelerId, PossibleMoves chosen) {
+ " kom fram till " + targetName
+ " (rolled=" + journey.getDistanceMoved() + ")"
);
managed.increaseScore();
}

tx.commit();
players.set(currentPlayerIndex, managed);
doesPlayerWin();

if (!wonGame) {
currentPlayerIndex = (currentPlayerIndex + 1) % players.size();
Expand All @@ -501,6 +507,28 @@ private void doMove(Long travelerId, PossibleMoves chosen) {
}
}

private void doesPlayerWin() {
for (Traveler player : players) {
if (player.checkScore()) {
wonGame = true;
System.out.println(player.getPlayerName() + " Wins the game. Congratulations");
winConLabel.setContentText(player.getPlayerName() + " Wins the game. Congratulations");
}
}
}
Comment on lines +510 to +518

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Loop continues after finding a winner; includes debug statement.

Two issues:

  1. The loop doesn't return or break after setting wonGame = true. If multiple players meet the win condition simultaneously, winConLabel will be overwritten multiple times.
  2. System.out.println should be removed or replaced with proper logging for production code.
Proposed fix
     private void doesPlayerWin() {
         for (Traveler player : players) {
             if (player.checkScore()) {
                 wonGame = true;
-                System.out.println(player.getPlayerName() + " Wins the game. Congratulations");
                 winConLabel.setContentText(player.getPlayerName() + " Wins the game. Congratulations");
+                return;
             }
         }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void doesPlayerWin() {
for (Traveler player : players) {
if (player.checkScore()) {
wonGame = true;
System.out.println(player.getPlayerName() + " Wins the game. Congratulations");
winConLabel.setContentText(player.getPlayerName() + " Wins the game. Congratulations");
}
}
}
private void doesPlayerWin() {
for (Traveler player : players) {
if (player.checkScore()) {
wonGame = true;
winConLabel.setContentText(player.getPlayerName() + " Wins the game. Congratulations");
return;
}
}
}
🤖 Prompt for AI Agents
In `@src/main/java/org/example/TravelGameController.java` around lines 378 - 386,
In doesPlayerWin(), stop the loop after the first winner is found to avoid
overwriting winConLabel and multiple state changes: after setting wonGame = true
and updating winConLabel with player.getPlayerName(), break out of the for loop
(or return from the method) so only the first matching Traveler (from players)
wins; also remove the System.out.println debug call and replace it with a proper
logger call (or delete it) to use production logging instead of stdout.



private List<Location> distinctLocationsById(List<Location> locations) {
java.util.Map<Long, Location> byId = new java.util.LinkedHashMap<>();
for (Location l : locations) {
if (l != null && l.getId() != null) {
byId.putIfAbsent(l.getId(), l);
}
}
return new java.util.ArrayList<>(byId.values());
}
Comment on lines +521 to +529

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Search for all usages of distinctLocationsById in Java files
rg -n "distinctLocationsById" --type=java

Repository: ithsjava25/project-jpa-grupp-4-g

Length of output: 208


🏁 Script executed:

# Check the actual file content around lines 521-529 to verify the method definition
sed -n '515,535p' src/main/java/org/example/TravelGameController.java | cat -n

Repository: ithsjava25/project-jpa-grupp-4-g

Length of output: 921


🏁 Script executed:

# Check updateGraphicsWithMoves method around line 434 to see if it uses .distinct()
sed -n '430,445p' src/main/java/org/example/TravelGameController.java | cat -n

Repository: ithsjava25/project-jpa-grupp-4-g

Length of output: 621


Remove unused method distinctLocationsById().

This method is defined at lines 521-529 but is never called anywhere in the codebase. The similar deduplication logic in updateGraphicsWithMoves() uses the stream .distinct() method instead, making this dead code that should be removed.

🤖 Prompt for AI Agents
In `@src/main/java/org/example/TravelGameController.java` around lines 521 - 529,
Remove the dead private helper distinctLocationsById(List<Location>) since it is
unused; delete the method declaration (private List<Location>
distinctLocationsById(...)) from TravelGameController and ensure there are no
remaining references to it elsewhere; no functional changes needed because
updateGraphicsWithMoves() already handles deduplication with .distinct().



private void highlightSelectedMoveButton(Button selected) {
for (var node : movesBox.getChildren()) {
if (node instanceof Button b) b.setStyle("");
Expand Down Expand Up @@ -539,6 +567,8 @@ private void doContinueJourney(Long travelerId) {
);
} else {
logList.getItems().add("✅ " + safeName(managed) + " kom fram till " + targetName + "!");
managed.increaseScore();
doesPlayerWin();
}

tx.commit();
Comment on lines +570 to 574

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent placement of doesPlayerWin() relative to transaction commit.

In doContinueJourney(), doesPlayerWin() is called before tx.commit() (line 574), whereas in doMove() it's called after the commit (line 495). This inconsistency could lead to issues:

  • If the transaction rolls back after doesPlayerWin() sets wonGame = true and updates the UI, the game state becomes inconsistent.
  • The players list isn't updated until line 575, so doesPlayerWin() may check stale entity references.
Proposed fix: Move win check after commit for consistency
             } else {
                 logList.getItems().add("✅ " + safeName(managed) + " kom fram till " + targetName + "!");
                 managed.increaseScore();
-                doesPlayerWin();
             }

             tx.commit();
             players.set(currentPlayerIndex, managed);
+            
+            if (!managed.isTravelling()) {
+                doesPlayerWin();
+            }

             if (!managed.isTravelling() && !wonGame) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
managed.increaseScore();
doesPlayerWin();
}
tx.commit();
} else {
logList.getItems().add("✅ " + safeName(managed) + " kom fram till " + targetName + "!");
managed.increaseScore();
}
tx.commit();
players.set(currentPlayerIndex, managed);
if (!managed.isTravelling()) {
doesPlayerWin();
}
if (!managed.isTravelling() && !wonGame) {
🤖 Prompt for AI Agents
In `@src/main/java/org/example/TravelGameController.java` around lines 570 - 574,
doContinueJourney() currently calls doesPlayerWin() before tx.commit(), unlike
doMove() which calls doesPlayerWin() after committing; move the doesPlayerWin()
invocation in doContinueJourney() to after tx.commit() and after the players
list is refreshed/updated so the win check uses committed state and up-to-date
player entities (ensure wonGame/UI updates only happen post-commit).

Expand Down
14 changes: 0 additions & 14 deletions src/main/java/org/example/player.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,10 @@ abstract class Player {
private int playerScore = 0;
int credits = 10000;

//set the player name
public void setPlayerName(String playerName) {
this.playerName = playerName;
}

public String getPlayerName() {
return playerName;
}

public void setPlayerCredits(int credits) {
if (credits < 0){
System.out.println("Invalid, credits cannot be under zero!");
this.credits = 0;
} else {
this.credits = credits;
}
}

public int getCredits() {
return credits;
}
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/org/example/playerMovement.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ record destinationPos(int destinationX, int destinationY) {}
private destinationPos destination;
private int playerTurnCount = 0;

public int getTransDiceCount(){
return selectedTransportDice;
}

public int getTurns(){
return playerTurnCount;
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/resources/travelGame.fxml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
<Label fx:id="currentCreditsLabel" text="0" style="-fx-text-fill: white;"/>
<Label text="Turn:" style="-fx-text-fill: white; -fx-font-weight: bold;"/>
<Label fx:id="currentTurnLabel" text="0" style="-fx-text-fill: white;"/>
<Label text="Points:" style="-fx-text-fill: white; -fx-font-weight: bold;"/>
<Label fx:id="currentPointLabel" text="0" style="-fx-text-fill: white;"/>
</HBox>

<HBox spacing="10">
Expand All @@ -75,6 +77,7 @@
<Label text="Destination:" style="-fx-text-fill: white; -fx-font-weight: bold;"/>
<Label fx:id="destinationLabel" text="-" style="-fx-text-fill: white;"/>
</HBox>
<DialogPane fx:id="winConLabel" style="-fx-font-size: 20px; -fx-font-weight: bold;"/>
</VBox>

<Region HBox.hgrow="ALWAYS"/>
Expand Down