This looks better to me and confirms scribbles on paper

1 files changed, 59 insertions, 65 deletions

diff --git a/include/action_list.c b/include/action_list.c
index 9e987bc..fd45afd 100644
--- a/include/action_list.c
+++ b/include/action_list.c
@@ -177,44 +177,43 @@ void action_take(action_list_t *action) {
 			dir = action->data1 >> 4;
 		int8_t delta = deltas[dir];
 
-		// Unfortunately num == 1 is a special case
-		if (num > 1) {
-			uint8_t steps, mask = 1<<(num-2), gaps_prime = gaps;
-			// Use the Kernighan method to count the set bits
-			for (steps = 1; gaps_prime; steps++) gaps_prime &= gaps_prime - 1;
-			// then from the destination to the source
-			for (uint8_t d = 0; d < num; d++) {
-				// transfer the top colour
-				colours[loc+steps*delta] <<= 1;
-				colours[loc+steps*delta] |= colours[loc] & 1;
-				colours[loc] >>= 1;
-				// increase the stone count, copy top stone if appropriate
-				celldat[loc+steps*delta] += NUM_INC;
-				if (d==0) {
-					celldat[loc+steps*delta] &= NUM_MASK;
-					celldat[loc+steps*delta] |= STONE_AT(loc);
-					celldat[loc] &= NUM_MASK;
-					celldat[loc] |= STONE_FLAT; // this should be optimised out :)
-				}
-				// decrement the count, the top colour
-				celldat[loc] -= NUM_INC;
+		// Use the Kernighan method to count the set bits
+		uint8_t steps = 1;
+		for (uint8_t _gaps = gaps; _gaps > 0; steps++) _gaps &= _gaps - 1;
 
-				if (gaps & mask) steps--;
-				mask >>= 1;
-			}
-		} else {
-			// Oh well
-			colours[loc+delta] <<= 1;
-			colours[loc+delta] |= colours[loc] & 1;
-			celldat[loc+delta] += NUM_INC;
-			celldat[loc+delta] &= NUM_MASK;
-			celldat[loc+delta] |= STONE_AT(loc);
+		// Move top stone type to destination
+		celldat[loc+steps*delta] &= NUM_MASK; // necessary for crushing
+		celldat[loc+steps*delta] |= STONE_AT(loc);
+		celldat[loc] &= NUM_MASK;
+		celldat[loc] |= STONE_FLAT; // should be optimised out
 
-			colours[loc] >>= 1;
-			celldat[loc] &= NUM_MASK;
-			celldat[loc] |= STONE_FLAT; // this should be optimised out :)
-			celldat[loc] -= NUM_INC;
+		uint8_t gap_bit = 0, total = 0;
+		// num == 1 is a special case
+		if (num > 1) {
+			gap_bit = 1 << (num - 2);
+			total = 1;
+		}
+		// move stuff starting at destination
+		for (uint8_t d = 0; d + 1 < num; d++, total++, gap_bit >>= 1) {
+			// We took a step, move everything over so far
+			if (gaps & gap_bit) {
+				colours[loc+steps*delta] <<= total;
+				colours[loc+steps*delta] |= colours[loc] & ((1 << total) - 1);
+				colours[loc] >>= total;
+				celldat[loc+steps*delta] += total*NUM_INC;
+				celldat[loc] -= total*NUM_INC;
+				// Reset for next step
+				total = 0;
+				steps--;
+			}
 		}
+		total++;
+		// Move what remains (steps == 1 here always, so we simplify)
+		colours[loc+delta] <<= total;
+		colours[loc+delta] |= colours[loc] & ((1 << total) - 1);
+		colours[loc] >>= total;
+		celldat[loc+delta] += total*NUM_INC;
+		celldat[loc] -= total*NUM_INC;
 	}
 	// Next ply
 	ply++;
@@ -250,41 +249,36 @@ void action_undo(action_list_t *action) {
 	} else {
 		// See action_take for comments, this is the time reversal
 		const uint8_t gaps = action->data0,
-			num = action->data1 & 0x0F, // unpack
+			num = action->data1 & 0x0F,
 			dir = action->data1 >> 4;
 		const int8_t delta = deltas[dir];
 
-		uint8_t steps = 1, mask = 1;
-		if (num > 1) {
-			for (uint8_t d = 0; d < num; d++) {
-				colours[loc] <<= 1;
-				colours[loc] |= colours[loc+steps*delta] & 1;
-				colours[loc+steps*delta] >>= 1;
-
-				celldat[loc] += NUM_INC;
-				if (d + 1 == num) {
-					celldat[loc] &= NUM_MASK;
-					celldat[loc] |= STONE_AT(loc+steps*delta);
-					celldat[loc+steps*delta] &= NUM_MASK;
-					celldat[loc+steps*delta] |= STONE_FLAT;
-				}
-				celldat[loc+steps*delta] -= NUM_INC;
-
-				if (gaps & mask) steps++;
-				mask <<= 1;
+		uint8_t gap_bit = 1, total = (num > 1) ? 1 : 0, steps = 1;
+		for (uint8_t d = 0; d + 1 < num; d++, total++, gap_bit <<= 1) {
+			// We took a step, move everything over so far
+			if (gaps & gap_bit) {
+				colours[loc] <<= total;
+				colours[loc] |= colours[loc+steps*delta] & ((1 << total) - 1);
+				colours[loc+steps*delta] >>= total;
+				celldat[loc] += total*NUM_INC;
+				celldat[loc+steps*delta] -= total*NUM_INC;
+				// Reset for next step
+				total = 0;
+				steps++;
 			}
-		} else {
-			colours[loc] <<= 1;
-			colours[loc] |= colours[loc+delta] & 1;
-			celldat[loc] += NUM_INC;
-			celldat[loc] &= NUM_MASK;
-			celldat[loc] |= STONE_AT(loc+delta);
-
-			colours[loc+delta] >>= 1;
-			celldat[loc+delta] &= NUM_MASK;
-			celldat[loc+delta] |= STONE_FLAT;
-			celldat[loc+delta] -= NUM_INC;
 		}
+		total++;
+		// Move what remains
+		colours[loc] <<= total;
+		colours[loc] |= colours[loc+steps*delta] & ((1 << total) - 1);
+		colours[loc+steps*delta] >>= total;
+		// celldat[loc] &= NUM_MASK; // not necessary, assumed STONE_FLAT
+		celldat[loc] += total*NUM_INC;
+		celldat[loc+steps*delta] -= total*NUM_INC;
+		// Top stone type
+		celldat[loc] |= STONE_AT(loc+steps*delta);
+		celldat[loc+steps*delta] &= NUM_MASK;
+		celldat[loc+steps*delta] |= STONE_STANDING; // optimised
 	}
 }