Dice evaluation in a Yahtzee-type game

Question

I just started using Godot, and I am attempting to build a Yahtzee-type game on my own. I created a function to evaluate the dice/roll and have it working correctly, I believe. I was hoping to get some more experienced eyes to look over my solution and provide some helpful feedback/criticism.

Instead of storing each die as an individual value, my first instinct was to group values, kind of tallying the total of each side rolled. I'm mainly curious if this is efficient enough and if someone can spot an edge case. But I'm also interested in improving it, so anything helpful is appreciated.

Edit:
I noticed this as well: When I duplicate the array in the chunk, I evaluating for "Pairs" and remove at that index. Using .Find(2) instead of .Any() in the following if statement will return true in cases where it shouldn't (only 1 pair). I looked at both functions in the class definition, and I am still confused about why they act like they do. Maybe someone can explain?

var ones: int = 0
var twos: int = 0
var threes: int = 0
var fours: int = 0
var fives: int = 0
var sixes: int = 0
var straightIndex: = 0

func evalute_roll(): 

var roll: Array[int] = [ones,twos,threes,fours,fives,sixes]

if roll.any(func(number): return number == 5): # Yatzhee
    rollText.text = "Yatzee!"

elif roll.any(func(number): return number == 4): # 4 of a Kind
    rollText.text = "4 of a Kind!"

elif roll.any(func(number): return number == 3): # Trips or Full House
    if roll.any(func(number): return number == 2):
        rollText.text = "Full House!"
    else:
        rollText.text = "3 of a Kind!"

elif roll.any(func(number): return number == 2): # Pair or 2Pair, Identify Pair Type.
    var side: String = ""
    var tempArray = roll.duplicate() 
    var index: int = tempArray.find(2)
    tempArray.remove_at(index)

    if tempArray.any(func(number): return number == 2):
        rollText.text = "2 Pair!"
    else:
        match index:
            0: side = "Ones"
            1: side = "Twos"
            2: side = "Threes"
            3: side = "Fours"
            4: side = "Fives"
            5: side = "Sixes"
        rollText.text = "Pair of " + side

elif roll.any(func(number): return number == 1): # Small and Large Straight
    for n in roll.size():
         if roll[n] == 1: 
              straightIndex += 1 
              if straightIndex == 4: 
                  rollText.text = "Small Straight!"
              elif straightIndex == 5: 
                  rollText.text = "Large Straight!"

         elif roll[n] == 0 and straightIndex != 0:
              straightIndex -= 1
else:
    rollText.text = "Scratch"

Answer 1

Clarity of variable names

I like the solution here, though I was thrown off at first by the roll and number variables. At first, I thought we were looking at the number of pips on the die face, rather than the number of dice that were rolled; I was confused why rolling a single 5 would give me a Yahtzee!

As you said, your instinct was to "group" the dice together based on the number of pips showing, and then tally (or "count") the number of dice in each group. Mirroring this language, I would go with roll_group_counts and count as variable names to replace roll and number.

var roll_group_counts: Array[int] = [ones,twos,threes,fours,fives,sixes]

if roll_group_counts.any(func(count): return count == 5): # Yahtzee
    ...

This makes it more clear that you're "grouping" the dice together, and then "counting" the number of dice within a single group to determine your score.

However, there's an even better solution...

Clarity of logic

What you're really doing here is grouping dice together based on the number of pips they're showing, and then checking for the size of the largest group. If the largest group has 5 dice in it, then you've got a Yahtzee. If it has 4 dice in it, then it's a 4-of-a-kind. 3 dice means you have either a 3-of-a-kind or a full house... etc.

Right now, this is obfuscated in your code due to the (relatively) complex use of any with a filtering function in each if statement. You could simplify this and improve the readability of your code by checking for the largest group once, storing the result in a clearly-named variable, and then reusing that variable in each if statement:

var largest_group_count: int = roll_group_counts.max()

if largest_group_count == 5: # Yahtzee
    ...
elif largest_group_count == 4: # 4-of-a-kind
    ...
elif ...

Bugs

There are a couple of bugs in your straight detection and scratch detection. I'll leave them as an exercise for you to find, but consider the following points:

• If you roll five dice and get 1, 2, 3, 5, 6 (which is a scratch), your code as-is will result in a "Small Straight!"
• Assuming everything works as expected, if your code execution reaches the final elif statement, the condition will always evaluate to true.
  In other words - again, assuming everything works as expected - the line rollText.text = "Scratch" will never be executed.

Answer 2

I really appreciate all the feedback, and would like to give credit to "cariehl" for noticing the bugs. By subtracting from "straightIndex", what happens looks like this " +1, +1, -1, +1, +1. Which results in the index being 4 after the loop. The solution is to set "straightIndex" to 0 instead, resetting the count.

Also it was probably bad copy/paste editing, but the "Scratch" condition just needs to be indented to align with the "for" statement above.

Here is the updated chunk for evaluating the straights (with the updated variable name "cariehl" suggested).

elif largest_group_count.max() == 1:
    var straightCount: int = 0
    for n in largest_group_count.size():
        if largest_group_count[n] == 1:
            straightCount += 1
        else: straightCount = 0
    if straightCount == 4:
        print("Small Straight")
    elif straightCount == 5:
        print("Large Straight")
    else:
        print("Scratch")

I do believe this is 100% working now, and is easier to understand.

Edit: This is not 100%. Example: [1,2,1,1,0,0] will result in a scratch.

Answer 3

There was a few issues with evaluating [1,2,1,1,0,0].

1. When evaluating for straights this will fail.
2. When evaluating, the code would evaluate this as just a "Pair". (Should result in "Pair" & "Small Straight")

Here is and updated and refactored version of the code.

class_name DiceManager
extends Control

@export var dice: Array[Die]
@export var holdButtons: Array[CheckBox]

func _ready():
    for n in dice.size():
        holdButtons[n].connect("pressed",dice[n].holdCall)
    
    var testTally: Array[int] = [1,2,1,1,0,0]
    print(testTally)
    evaluate_tally(testTally)

func _on_roll_button_pressed():
    var diceTally: Array[int] = [0,0,0,0,0,0]
    for die in dice.size():
        dice[die].roll()
        match dice[die].value:
            1: diceTally[0] += 1
            2: diceTally[1] += 1
            3: diceTally[2] += 1
            4: diceTally[3] += 1
            5: diceTally[4] += 1
            6: diceTally[5] += 1
    print(diceTally)
    evaluate_tally(diceTally)

func evaluate_tally(tally: Array[int]):
    print("Attemptin Evaluation")
    match tally.max():
        5: print("YahtZee!!")
        4: print("4 of a Kind!")
        3: check_fullhouse(tally)   
        2: check_pairs(tally)
        1: check_straights(tally)
        _: print("Cannot Evaluate")

func check_pairs(tally: Array[int]):
    var pairType = ""
    var tempTally = tally.duplicate()
    var index = tempTally.find(2)   
    tempTally.remove_at(index)
    if tempTally.any(func(number): return number == 2):
        print("2 Pair")
    else:
        match index:
            0: pairType = "Ones"
            1: pairType = "Twos"
            2: pairType = "Threes"
            3: pairType = "Fours"
            4: pairType = "Fives"
            5: pairType = "Sixes"
        print("Pair of " + str(pairType))
        check_straights(tally)

func check_straights(tally: Array[int]):
    var straightCount: int = 0
    var hasSmallcock: bool = false
    var hasLargecock: bool = false
    for n in tally.size():
        if tally[n] >= 1: 
            straightCount +=1
            if straightCount == 4: hasSmallcock = true
            elif straightCount == 5: hasLargecock = true
    if hasSmallcock: print("Small Straight")
    elif hasLargecock: print("Large Straight")
    else: print("Scratch")

func check_fullhouse(tally: Array[int]):
    if tally.any(func(number): return number == 2):
        print("Full House!")
    else: print("3 of a Kind")

Die script

class_name Die
extends TextureRect

@export var sides: Array[Texture2D]

var value: int = 1
var hold: bool = false
var holdCall = Callable(self,"set_hold")

func _ready():
    set_face()
    
func roll():
    if !hold: value = randi_range(1,6)
    set_face()

func set_face():
    self.texture = sides[value - 1]

func set_hold():
    hold = true if !hold else false