Java MVC游戏Player模型内置分数及访问器为何被判定不安全？

Let's break down exactly why your current setup raises red flags—this is a classic MVC design pitfall where model responsibilities get mixed up with game logic, leading to security gaps and maintainability headaches.

1. Uncontrolled Mutability = Cheating & Logic Errors #

Your Player class exposes an addScore() method with zero validation. Any code that holds a Player instance (like a rogue controller, or even a bug in your game logic) can call addScore(-100) or addScore(9999) at any time, directly altering the score without respecting game rules. There's no gatekeeping to ensure score changes align with actual in-game actions (like pocketing a coin, or penalizing a foul).

For example: If a careless line of code accidentally calls addScore() twice for a single coin pocket, your game's integrity is broken—no checks exist to prevent this. In a multiplayer scenario, this opens the door to trivial cheating if players can manipulate their Player instances.

2. Violates the Single Responsibility Principle #

A Player model should represent a player's core identity and static attributes (name, turn order, win status)—not handle game state logic like score tracking, foul counts, or consecutive misses. By embedding these in Player, you're forcing the model to manage business rules, which:

• Makes Player harder to maintain (if your game's scoring rules change, you have to modify the Player class instead of a dedicated game logic component).
• Ties game behavior to the entity model, creating tight coupling that's hard to refactor.

3. No Audit Trail or Immutable State Guarantees #

Once a game ends, scores should be immutable—no one should be able to tweak them after the fact. But right now, any code with access to a Player can call addScore() even post-game. There's also no way to track why or when a score changed, which is critical for debugging or anti-cheating.

If you wanted to log every score adjustment (e.g., "Player Alice gained 5 points for pocketing a black coin"), you'd have to cram that logic into Player—which adds even more unrelated responsibilities to the model.

4. Exposes Internal State to External Dependencies #

The getScore() method returns the raw score value directly, meaning external code (like UI controllers) becomes tightly coupled to the Player's internal state. If you later decide to modify how scores are calculated (e.g., adding a multiplier for streaks), every piece of code that uses getScore() will break or need updates.

Instead, external components should request scores through the GameModel, which can apply any game-specific logic before returning the value.

5. Concurrency Risks (For Multiplayer Scenarios) #

If you ever expand this to a multiplayer game with concurrent turns, the addScore() and increment methods have no synchronization. Multiple threads could modify the same Player's score at the same time, leading to race conditions and incorrect score values. Centralizing state management in GameModel makes it easy to add thread safety where needed.

How to Fix This #

The solution is to split responsibilities: keep Player as a pure entity, and move all game state tracking (scores, fouls, misses) to your GameModel (or a dedicated ScoreManager class if you want to further decouple).

Revised Player Class

public class Player implements Comparable<Player>{
    private String name;
    private int turn;
    private boolean win;

    public Player() {
        this.win = false;
    }

    public void setWin(boolean win) { this.win = win; }
    public boolean getWin() { return this.win; }
    public void setPlayerName(String name) { this.name = name; }
    public void setTurn(int turn) { this.turn = turn; }
    public int getTurn() { return this.turn; }
    public String getName() { return this.name; }

    @Override
    public int compareTo(Player comparePlayer) {
        // Delegate score comparison to GameModel
        int compareScore = GameModel.getPlayerScore(comparePlayer);
        return compareScore - GameModel.getPlayerScore(this);
    }
}

Updated GameModel with State Management

package com.tiffany.CleanStrike_1.models;
import java.util.HashMap;
import java.util.Map;

public class GameModel {
    private int player_count;
    private Player[] players;
    private Player current_player;
    private Player winner = new Player();
    private gameState game_state;
    private boolean draw = false;
    private carromBoard carrom_board;
    // Track game state separately from Player entities
    private Map<Player, Integer> playerScores;
    private Map<Player, Integer> foulCounts;
    private Map<Player, Integer> consecutiveMisses;

    public GameModel(int player_count, int black_coin_count, int red_coin_count, int black_val, int red_val) {
        this.game_state = gameState.DORMANT;
        this.players = new Player[player_count];
        this.playerScores = new HashMap<>();
        this.foulCounts = new HashMap<>();
        this.consecutiveMisses = new HashMap<>();

        for(int i=0;i<player_count;i++) {
            this.players[i] = new Player();
            // Initialize state for each player
            playerScores.put(players[i], 0);
            foulCounts.put(players[i], 0);
            consecutiveMisses.put(players[i], 0);
        }

        this.setPlayerCount(player_count);
        Coin black_coin = new Coin(CoinColour.BLACK, black_val);
        carrom_board = new carromBoard();
        this.carrom_board.addCoin(black_coin, black_coin_count);
        Coin red_coin = new Coin(CoinColour.RED, red_val);
        this.carrom_board.addCoin(red_coin, red_coin_count);
    }

    // Score management with validation
    public void addPlayerScore(Player player, int points) {
        if (points < 0) {
            throw new IllegalArgumentException("Negative points are not allowed");
        }
        // Add game-specific rules here (e.g., max score cap)
        int currentScore = playerScores.getOrDefault(player, 0);
        playerScores.put(player, currentScore + points);
    }

    public int getPlayerScore(Player player) {
        return playerScores.getOrDefault(player, 0);
    }

    // Foul count management with built-in game logic
    public void resetFoulCount(Player player) {
        foulCounts.put(player, 0);
    }

    public void incrementFoulCount(Player player) {
        int currentFouls = foulCounts.getOrDefault(player, 0);
        foulCounts.put(player, currentFouls + 1);
        // Example: Penalty after 3 fouls
        if (foulCounts.get(player) >= 3) {
            addPlayerScore(player, -5);
            resetFoulCount(player);
        }
    }

    public int getFoulCount(Player player) {
        return foulCounts.getOrDefault(player, 0);
    }

    // Repeat pattern for consecutive misses...

    // ... rest of your existing GameModel methods ...
}

This approach:

• Prevents unauthorized score changes by centralizing validation in GameModel.
• Keeps your Player class focused on its core role.
• Makes it easy to add logging, thread safety, or rule changes without touching the entity model.
• Maintains game integrity by ensuring all state changes align with game rules.

内容的提问来源于stack exchange，提问作者Valkyrie