Issue with hold function in Yatzy bot

Question

I'm stuck with a 'hold' function when developing a Yatzy bot. Everything else works, but the logic for this function seems to fail in some instances. Basically, the idea is to hold all the numbers given, and then roll the dice that don't match the numbers given.

[00:04] @Dessimat0r: .roll
[00:04] YatzyBot: #1: dice: [2, 5, 3, 4, 1], scores: [ 1 2 3 4 5 6 1P 2P 3K 4K SS LS H Y C ]
[00:04] @Dessimat0r: .hold 2 1
[00:04] YatzyBot: #2: dice: [2, 5, 3, 4, 1], scores: [ 1 2 3 4 5 6 1P 2P 3K 4K SS LS H Y C ]
[00:04] @Dessimat0r: .hold 2 1
[00:04] YatzyBot: #3: dice: [2, 5, 3, 4, 1], scores: [ 1 2 3 4 5 6 1P 2P 3K 4K SS LS H Y C ]

As can be seen, all numbers are being held instead of just the selected few (This is not coincidence from the dice rolls). The code is below:

} else if (event.getMessage().startsWith(".hold")) {
    if (y.getTurn() != null && event.getUser().getNick().equals(y.getTurn().getPlayer().getName())) {
        String[] tokens = event.getMessage().split(" ");
        if (tokens[0].equals(".hold")) {
            boolean failed = false;
            try {
                if (tokens.length == 1) {
                    bot.sendMessage(CHANNEL, "Must choose some dice to hold!");
                    return;
                }
                ArrayList<Integer> dice = new ArrayList<Integer>();
                ArrayList<Integer> holdnums = new ArrayList<Integer>();
                ArrayList<Integer> rollnums = new ArrayList<Integer>();

                for (Die d : y.getDice()) {
                    dice.add(d.getFaceValue());
                }

                // parse other numbers
                for (int i = 1; i < tokens.length; i++) {
                    int num = Integer.parseInt(tokens[i]);
                    holdnums.add(num);
                }
                ListIterator<Integer> diter = dice.listIterator();
                dice: while (diter.hasNext()) {
                    Integer d = diter.next();

                    if (holdnums.isEmpty()) {
                        rollnums.add(d);
                        diter.remove();
                        continue;
                    }
                    ListIterator<Integer> iter = holdnums.listIterator();
                    while (iter.hasNext()) {
                        int holdnum = iter.next().intValue();
                        if (holdnum == d) {
                            iter.remove();
                            diter.remove();
                            continue dice;
                        }
                    }

                }

                if (!holdnums.isEmpty()) {
                    bot.sendMessage(CHANNEL, "Hold nums not found: " + holdnums);
                    failed = true;
                }

                if (!failed) {
                    y.getTurn().rollNumbers(convertIntegers(rollnums));

                    Map<Scoring, Integer> scores = y.getRollScores();

                    Map<Scoring, Integer> unchosen = new EnumMap<Scoring, Integer>(Scoring.class);
                    Map<Scoring, Integer> chosen = new EnumMap<Scoring, Integer>(Scoring.class);

                    for (Entry<Scoring, Integer> entry : scores.entrySet()) {
                        if (y.getTurn().getPlayer().getTotals().get(entry.getKey()) == -1) {
                            unchosen.put(entry.getKey(), entry.getValue());
                        } else {
                            chosen.put(entry.getKey(), entry.getValue());
                        }
                    }
                    bot.sendMessage(CHANNEL, "#" + y.getTurn().getRolls() + ": dice: " + y.getDiceStr() + ", scores: " + getDiceStr(y.getTurn().getPlayer().getTotals(), scores));
                }
            } catch (TurnException e1) {
                bot.sendMessage(CHANNEL, e1.getMessage());
            } catch (RollException e2) {
                bot.sendMessage(CHANNEL, e2.getMessage());
            } catch (YahtzyException e3) {
                bot.sendMessage(CHANNEL, e3.getMessage());
            } catch (NumberFormatException e4) {
                bot.sendMessage(CHANNEL, e4.getMessage());
            }
        }
    }
}

Edit: Fixed it all. Updated code:

} else if (event.getMessage().startsWith(".hold")) {
    if (y.getTurn() != null && event.getUser().getNick().equals(y.getTurn().getPlayer().getName())) {
        String[] tokens = event.getMessage().split(" ");
        if (tokens[0].equals(".hold")) {
            boolean failed = false;
            try {
                if (tokens.length == 1) {
                    bot.sendMessage(channel, "Must choose some dice to hold!");
                    return;
                }
                ArrayList<Integer> holdnums = new ArrayList<Integer>();
                ArrayList<Integer> rollnums = new ArrayList<Integer>();

                // parse other numbers
                for (int i = 1; i < tokens.length; i++) {
                    int num = Integer.parseInt(tokens[i]);
                    holdnums.add(num);
                }
                for (int i = 0; i < y.getDice().length; i++) {
                    int d = y.getDice()[i].getFaceValue();

                    if (holdnums.isEmpty()) {
                        rollnums.add(d);
                        continue;
                    }

                    ListIterator<Integer> iter = holdnums.listIterator();

                    boolean found = false;
                    while (iter.hasNext()) {
                        int holdnum = iter.next().intValue();
                        if (holdnum == d) {
                            iter.remove();
                            found = true;
                            break;
                        }
                    }
                    if (!found) {
                        rollnums.add(d);
                    }
                }

                if (!holdnums.isEmpty()) {
                    bot.sendMessage(channel, "Hold nums not found: " + holdnums);
                    failed = true;
                }

                if (!failed) {
                    boolean[] rolled = y.getTurn().rollNumbers(convertIntegers(rollnums));

                    Map<Scoring, Integer> scores = y.getRollScores();

                    Map<Scoring, Integer> unchosen = new EnumMap<Scoring, Integer>(Scoring.class);
                    Map<Scoring, Integer> chosen = new EnumMap<Scoring, Integer>(Scoring.class);

                    for (Entry<Scoring, Integer> entry : scores.entrySet()) {
                        if (y.getTurn().getPlayer().getTotals().get(entry.getKey()) == -1) {
                            unchosen.put(entry.getKey(), entry.getValue());
                        } else {
                            chosen.put(entry.getKey(), entry.getValue());
                        }
                    }
                    bot.sendMessage(channel, "#" + y.getTurn().getRolls() + ": dice: " + diceToString(rolled) + ", scores: " + getDiceStr(y.getTurn().getPlayer().getTotals(), scores));
                }
            } catch (TurnException e1) {
                bot.sendMessage(channel, e1.getMessage());
            } catch (RollException e2) {
                bot.sendMessage(channel, e2.getMessage());
            } catch (YahtzyException e3) {
                bot.sendMessage(channel, e3.getMessage());
            } catch (NumberFormatException e4) {
                bot.sendMessage(channel, e4.getMessage());
            }
        }
    }
}

Evgeny Kluev · Accepted Answer · 2012-10-30T15:53:40.160

2

Everything looks OK, but this check for holdnum is not in the right place:

    if (absent) {
      bot.sendMessage(CHANNEL, "Hold num not found: " + holdnum);
      failed = true;
      break dice;
    }

It is too early to say here if given hold number is right or wrong, so it is better to remove this piece of code. To check validity of holdnums you could look at this list after the "dice" loop finishes: if holdnums is not empty, it contains some items, that were not found.

For updated question:

I see one problem: rollnums.add(d) should not be called in nested while to avoid adding the same value several times. Instead, it should be called once, after this loop is completed.

edited Oct 30 '12 at 15:53

answered Oct 27 '12 at 17:37

Evgeny Kluev

24,287
7
55
98

Thanks for this. It wasn't exactly the issue, but it helped anyway. The dice were being iterated over again, so I put them into another list to be removed once processed. This seemed to do the trick. I'll update the code above to reflect fixes. If you have any optimisation suggestions they'd be welcome :) – Chris Dennett Oct 30 '12 at 15:18
1

@ChrisDennett: I've updated the answer. As for optimisation, as far as I know, there are only 5 dices in Yatzy, so optimising this part of code unlikely gives any effect. You could sort both `dice` and `holdnums` lists, then iterate both in one loop, advancing iterator with smaller value. – Evgeny Kluev Oct 30 '12 at 15:50
I found some other issue when testing, please update answer if possible :D – Chris Dennett Oct 31 '12 at 00:34
1

@ChrisDennett: the problem is still with `rollnums.add(d)`. You've removed it from the nested loop, but didn't add it after the loop, as I suggested. – Evgeny Kluev Oct 31 '12 at 08:16
I don't see how it's possible, since `rollnums.add(d)` is picking up the die face value from the loop? `dice: while (diter.hasNext()) { Integer d = diter.next(); ... }`. Please clarify :D – Chris Dennett Oct 31 '12 at 17:16
@ChrisDennett: just add `rollnums.add(d)` as the last thing in the dice loop. – Evgeny Kluev Oct 31 '12 at 17:24
Maybe not. It seems to be holding the same number multiple times. I'll refactor it again, I guess. – Chris Dennett Oct 31 '12 at 19:47

Issue with hold function in Yatzy bot

1 Answers1