Two-player Yahtzee game

Question

I am working on a Yahtzee game development for two players that consists of all the categories Full house, Yahtzee, ones, twos, threes, etc.

I have implemented the interface that works for two players and displays the winner at the end of the game, but I don't think it is efficient need feedback and suggestions to improve my code.

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Scanner;

public class YatzyUI {

    public static void main(String[] args) {
        Scanner scanner = new Scanner(System.in);
        System.out.println("Enter the name of the first player");
        String firstPlayerName = scanner.next();
        System.out.println("Enter the name of the second player");
        String secondPlayerName = scanner.next();
        Player p1 = new Player(firstPlayerName);
        Player p2 = new Player(secondPlayerName);
        List<Player> players = new ArrayList<>();
        players.add(p1);
        players.add(p2);
        int score = 0;
        try {
            for (int i = 1; i <= 15; i++) {
                p1.firstRollDice(firstPlayerName);
                p1.secondAndThirdRollDice();
                p1.selectAndSetCategoryScore(p1.getDiceArray());
            }
        } catch (NullPointerException e) {
            System.out.println("Please enter a valid category ,the category chosen is invalid ");
            p1.selectAndSetCategoryScore(p1.getDiceArray());
        }
        try {
            for (int i = 1; i <= 15; i++) {
                p2.firstRollDice(secondPlayerName);
                p2.secondAndThirdRollDice();
                p2.selectAndSetCategoryScore(p1.getDiceArray());
            }

        } catch (NullPointerException e) {
            System.out.println("Please enter a valid category ,the category chosen is invalid ");
            p2.selectAndSetCategoryScore(p1.getDiceArray());
        }

        Collections.sort(players, new ScoreComparartor());
        Player winner = players.remove(1);
        System.out.println("Following is the winner of the game :" + winner.getPlayerName() + " with the score : " + winner.getPlayerScore());
    }
}

Player Class

import YatzyGameCategories.Categories;

import java.util.Map;
import java.util.Scanner;

public class Player {

    public String getPlayerName() {
        return playerName;
    }

    public void setPlayerName(String playerName) {
        this.playerName = playerName;
    }

    private String playerName;
    Map<Integer, Categories> remainingCategories = Categories.getCategories();

    public int getPlayerScore() {
        return playerScore;
    }

    public void setPlayerScore(int score) {
        this.playerScore = score;
    }

    private int playerScore;

    public int[] getDiceArray() {
        return diceArray;
    }

    public void setDiceArray(int[] diceArray) {
        this.diceArray = diceArray;
    }

    int[] diceArray;

    public Player(String name)
    {
        this.playerName=name;
    }

    public void firstRollDice() {
        System.out.println(playerName + "'s turn. Click 'Roll Dice' button to roll the dice.");
        diceArray= RandomDieGenerator.randomDieValueGenerator(5);
        for (int i : diceArray)
            System.out.println(i);
    }

    public void secondAndThirdRollDice() {
        for (int i = 0; i < 2; i++) {
            System.out.println("Select the dice you wish to re-roll");  // 3 turns
            System.out.println("Enter the die position separated by ,");
            Scanner sc = new Scanner(System.in);
            String s = sc.next();
            String dicesToReRoll[] = s.split(",");
            int dicesReRolled[] = new int[dicesToReRoll.length];
            for (int j = 0; j < dicesToReRoll.length; j++) {
                dicesReRolled[j] = Integer.parseInt(String.valueOf(dicesToReRoll[j]));
                diceArray[dicesReRolled[j] - 1] = RandomDieGenerator.randomDieValueGenerator(dicesReRolled.length)[j];
            }
            setDiceArray(diceArray);
            for (int k : diceArray)
                System.out.println(k);

        }
    }

    public void selectAndSetCategoryScore(int ...diceArray) {

        remainingCategories.forEach((key, value) -> System.out.println(key + ":" + value.getName(key)));
        System.out.println("Select a Category for this roll.");
        Scanner sc = new Scanner(System.in);
        int category = 0;

            category = sc.nextInt();
            Categories cat = remainingCategories.get(category);
            remainingCategories.remove(category);
            remainingCategories.forEach((key, value) -> System.out.println(key + ":" + value.getName(key)));
            //throws error if user selects a category other than values present in the remaining categories
            playerScore = playerScore + cat.score(diceArray);
            setPlayerScore(playerScore);



    }
    }

Answer 1

This is just the main method that controls the game. All the interesting code is missing.

Instead of reading the names of a fixed number of players and storing the players to two distinct method variables, read a name and create a player into the list in a loop, until the user provides an empty line to denote en of input.

Variable score is unnecessary.

There are 13 rounds in a Yahtzee game (note the spelling), not 15.

p2.firstRollDice(secondPlayerName);

Passing the player name to this method seems pointless. It was already passed in the constructor and it has nothing to do with dice rolling.

p1.secondAndThirdRollDice();

Second and third rolls are identical in implementation. They should be implemented as one method named rerollDice and called twice from the main method. On the other hand, if you want to combine the game logic into the player, you should combine the first roll and category selection into the same method too, so it becomes p1.performTurn().

p1.selectAndSetCategoryScore(p1.getDiceArray());

This breaks encapsulation. It is pointless to fetch data from inside of p1 just for the sake of passing it back to p1 as a parameter. The player already knows the hand it has rolled. This method needs no parameters.

} catch (NullPointerException e) {

This is both bad practise and a bug. NPE has a very specific meaning and it should not be used to convey information of invalid input. Catching NPE is always a result of programming error. Also, catching this breaks the loop that was supposed to run 15 times, and then exeutes the last method call from the loop once. In no way can it be intended functionality if the loop exits from first round.

Yahtzee is supposed to be played so that each player plays one round a time. In the code one player plays all 15 rounds immediately before passing turn to next player. Something like this:

for (int turn = 1; turn <= 15; turn++) {
    for (Player: players) {
        // Play one turn.
    }
}

I don't see the scanner being passed to the Player objects so they can read the input for the category selection. I hope they are not constructing their own scanners that read from System.in...