HiveBrain v1.2.0
Get Started
← Back to all entries
patternrustModerate

Shift and merge numbers as in 2048 game code

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
shiftmergenumberscodegameand2048

Problem

I started to learn Rust last night. Given its parallelization capabilities, I thought a good starting point would be porting my C++ AI for 2048 to Rust.

This is the function implementing the shift of one line (from high to low indices) of the 2048 board, ported verbatim from C++. It is supposed to shift the numbers in the line and return the score made by that move.

static SCORE_MERGE_FACTOR: f32 = 1.2f32;

fn shift_line(line: [&mut u8, ..4]) -> u64 {
    let mut result: u64 = 0;
    let mut i = 0;
    while i  0 &&
           *line[i-1] == *line[i]
        {
            *line[i-1] += 1;
            *line[i] = 0;
            merged = true;
            result += *line[i-1] as u64;
        }
        if *line[i] == 0 {
            let mut shifted = false;
            let mut j = i+1;
            while j < line.len() {
                if *line[j] != 0 {
                    *line[i] = *line[j];
                    *line[j] = 0;
                    shifted = true;
                    break;
                }
                j += 1;
            }
            if !shifted || merged {
                i += 1;
            }
        } else {
            i += 1;
        }
    }

    (result as f32 * SCORE_MERGE_FACTOR).round() as u64
}


I would love a general review with respect to Rust's paradigms, but I feel especially uncomfortable for not using iterators and having to use mutable variables all over the place.

I also find these explicit casts quite cumbersome, are they really necessary?

Solution

With both scoring and merging

Oops! I did indeed neglect to include merging. This is my mistake; I'm not a big fan of returning values via mutable parameters, so I think my brain shut off. Fortunately, while working on the merging logic, I made the scoring logic shorter.

// Comment #1
fn compress_zeroes(line: [u8, ..4]) -> (u8, u8, u8, u8) {
    let mut nums: Vec = line.iter().map(|x| *x).filter(|x| *x != 0).collect();
    nums.resize(4, 0);
    (nums[0], nums[1], nums[2], nums[3])
}

// Comment #2
fn score_line(line: [u8, ..4]) -> u64 {
    let line = compress_zeroes(line);
    let line = (line.0 as u64, line.1 as u64, line.2 as u64, line.3 as u64);

    let r = match line {
        (a, b, c, d) if a == b && c == d && a != 0 && c != 0 => a+1 + c+1,
        (a, b, _, _) if a == b && a != 0                     => a+1,
        (_, a, b, _) if a == b && a != 0                     => a+1,
        (_, _, a, b) if a == b && a != 0                     => a+1,
        _                                                    => 0,
    };

    (r as f32 * SCORE_MERGE_FACTOR).round() as u64
}

fn squish_line(line: [u8, ..4]) -> [u8, ..4] {
    let line = compress_zeroes(line);

    // Comment #3
    match line {
        (a, b, c, d) if a == b && c == d && a != 0 && c != 0 => [a+1, c+1, 0,   0],
        (a, b, c, d) if a == b && a != 0                     => [a+1, c,   d,   0],
        (a, b, c, d) if b == c && b != 0                     => [a,   b+1, d,   0],
        (a, b, c, d) if c == d && c != 0                     => [a,   b,   c+1, 0],
        (a, b, c, d)                                         => [a,   b,   c,   d],
    }
}


  • This is probably the best enhancement. Since we don't care about zero values, we can simply remove all the zeroes and then add them back at the end of the array. This removes 3 cases from each match statement!



  • Conceptually unchanged from the earlier solution, just smaller due to being able to ignore zeroes.



  • Similar to score_line, we use pattern matching to combine consecutive values that are the same (and non-zero). In this case, we need to track all of the values, so I changed up the naming a bit to leave each letter in the same position.



I definitely prefer having two distinct methods here. In the original, the method is called score_line, but it should actually be called score_and_merge_line. I think that added to my original confusion.

Original answer

There's a lot of code in the original to move things around, just to ignore all that and count how many squares would be merged. I made use of Rust's pattern matching to express the small number of interesting cases that exist in a given line.

fn score_line(line: [u8, ..4]) -> u64 {
    // Comment #1
    let a0 = line[0] as u64;
    let a1 = line[1] as u64;
    let a2 = line[2] as u64;
    let a3 = line[3] as u64;

    // Comment #2
    let r = match (a0, a1, a2, a3) {
        (a, b, c, d) if a == b && c == d && a != 0 && c != 0 => a+1 + c+1,
        (a, b, _, _) if a == b && a != 0 => a+1,
        (_, a, b, _) if a == b && a != 0 => a+1,
        (_, _, a, b) if a == b && a != 0 => a+1,
        (a, 0, b, _) if a == b && a != 0 => a+1, // Comment #3
        (a, 0, 0, b) if a == b && a != 0 => a+1,
        (_, a, 0, b) if a == b && a != 0 => a+1,
        _ => 0, // Comment #4
    };

    (r as f32 * SCORE_MERGE_FACTOR).round() as u64
}


  • We start by unpacking each of the 8-bit numbers and casting them to a larger type. When we add two squares together, it's possible to require more than 8 bits. A u16 would suffice here, but the original method returned a u64, so we will stick with that.



  • We've unpacked the array to separate variables, but we actually want to pattern match against all of them at once. We create a tuple to cram them all back together and match on that.



  • All the pattern match lines are similar, but this one has the most variety. a and b will be bound to the value of the number in the respective position in the tuple. _ means that we don't care about the value in that position, and 0 requires the literal value 0 in that position. We also use a pattern guard (if a == b...) to further restrict the pattern match to pairs of numbers that equal each other. The expression after the => will be the result of the match if that particular branch is chosen.



  • This is the catch-all branch, which is taken when none of the above branches are picked.



I think that this solution is improved from the original because:

  • Removed unneeded mutability. No variables are required to be mutated after they are created, which helps me as a programmer reason about the code better.



  • Removed unused functionality. The function is designed to return the score, it doesn't need to actually move numbers around into the future positions. This relates to the lack of mutability.



  • Pattern matching replaces what could be a complicated series of if-else checks with something that has more concise and ha

Code Snippets

// Comment #1
fn compress_zeroes(line: [u8, ..4]) -> (u8, u8, u8, u8) {
    let mut nums: Vec<_> = line.iter().map(|x| *x).filter(|x| *x != 0).collect();
    nums.resize(4, 0);
    (nums[0], nums[1], nums[2], nums[3])
}

// Comment #2
fn score_line(line: [u8, ..4]) -> u64 {
    let line = compress_zeroes(line);
    let line = (line.0 as u64, line.1 as u64, line.2 as u64, line.3 as u64);

    let r = match line {
        (a, b, c, d) if a == b && c == d && a != 0 && c != 0 => a+1 + c+1,
        (a, b, _, _) if a == b && a != 0                     => a+1,
        (_, a, b, _) if a == b && a != 0                     => a+1,
        (_, _, a, b) if a == b && a != 0                     => a+1,
        _                                                    => 0,
    };

    (r as f32 * SCORE_MERGE_FACTOR).round() as u64
}

fn squish_line(line: [u8, ..4]) -> [u8, ..4] {
    let line = compress_zeroes(line);

    // Comment #3
    match line {
        (a, b, c, d) if a == b && c == d && a != 0 && c != 0 => [a+1, c+1, 0,   0],
        (a, b, c, d) if a == b && a != 0                     => [a+1, c,   d,   0],
        (a, b, c, d) if b == c && b != 0                     => [a,   b+1, d,   0],
        (a, b, c, d) if c == d && c != 0                     => [a,   b,   c+1, 0],
        (a, b, c, d)                                         => [a,   b,   c,   d],
    }
}
fn score_line(line: [u8, ..4]) -> u64 {
    // Comment #1
    let a0 = line[0] as u64;
    let a1 = line[1] as u64;
    let a2 = line[2] as u64;
    let a3 = line[3] as u64;

    // Comment #2
    let r = match (a0, a1, a2, a3) {
        (a, b, c, d) if a == b && c == d && a != 0 && c != 0 => a+1 + c+1,
        (a, b, _, _) if a == b && a != 0 => a+1,
        (_, a, b, _) if a == b && a != 0 => a+1,
        (_, _, a, b) if a == b && a != 0 => a+1,
        (a, 0, b, _) if a == b && a != 0 => a+1, // Comment #3
        (a, 0, 0, b) if a == b && a != 0 => a+1,
        (_, a, 0, b) if a == b && a != 0 => a+1,
        _ => 0, // Comment #4
    };

    (r as f32 * SCORE_MERGE_FACTOR).round() as u64
}

Context

StackExchange Code Review Q#49331, answer score: 10

Revisions (0)

No revisions yet.