klotski-solver improvement

BoardHasher.h

@@ -1,107 +1,75 @@
 #pragma once
+#include "puzzle.h"
 
-#include <numeric>
 #include <random>
-#include <vector>
-
-#include "puzzle.h"
+#include <algorithm> // std::find
 
 // Need a hash that's both order invariant & block-id invariant
 // i.e. 2 blocks with different ids at the same position should still hash the same
 // i.e. hash should be the same regardless of the order in which the blocks on the board are hashed
 
-// As we only update 1 block at a time, this would be perfect to update incrementally
-// But i'll add this to the TODO list :-) ...
-
-struct BlockSizeType
-{
-    int width;
-    int height;
-    bool operator==(const BlockSizeType &other)
-    {
-        return width == other.width && height == other.height;
-    }
-};
+// TODO : As we only update 1 block at a time, this would be perfect to update incrementally
 
-template <typename HashType = int>
-struct BoardHasher
-{
-    // TODO: provide implementation for other HashTypes
-    template <int BlockCount>
-    HashType hash(const BoardState<BlockCount> &state)
-    {
-        HashType result{};
-        result ^= runnerCode(state.m_runner);
-        for (const auto &block : state.m_blocks)
-        {
-            result ^= blockStateCode(block);
-        }
-        return result;
-    };
+template <typename HashType/* = int*/>
+class BoardHasher {
+public:
+	HashType hash(const BoardState& boardState) {
+		HashType result{};
+		result ^= runnerHash(boardState.runner);
+		for (const auto& block : boardState.blocks) {
+			result ^= blockHash(block);
+		}
+		return result;
+	};
 
-    template <int BlockCount>
-    BoardHasher(const Puzzle<BlockCount> &puzzle)
-        : m_width{ puzzle.m_dimensions.m_x }
-        , m_height{ puzzle.m_dimensions.m_y }
-        , m_codes{}
-        , m_blockTypes{}
-    {
-        // Should use unordered set here, but compiler complains
-        // std::unordered_set<BlockSizeType> uniqueBlocks;
-        for (const auto &block : puzzle.m_initialState.m_blocks)
-        {
-            const BlockSizeType blockType{ block.m_sizeX, block.m_sizeY };
-            if (end(m_blockTypes) == std::find(begin(m_blockTypes), end(m_blockTypes), blockType))
-            {
-                m_blockTypes.push_back(blockType);
-            }
-        }
+	BoardHasher(const Puzzle& puzzle)
+		: dimensions{ puzzle.dimensions }
+		, numberOfPuzzlePos{ puzzle.dimensions.x * puzzle.dimensions.y }
+		, hashList{}
+		, blockPointSetSet{}
+	{
+		for (const auto block : puzzle.boardState->blocks) {
+			blockPointSetSet.insert(block.pointSet);
+		}
 
-        std::random_device device{};
-        std::mt19937 generator{ device() };
-        std::uniform_int_distribution<HashType> distribution{};
+		std::mt19937 generator{ (std::random_device{})() };
+		std::uniform_int_distribution<HashType> distribution{};
 
-        const auto numberOfStates = (m_blockTypes.size() + 1) * puzzle.m_dimensions.m_x * puzzle.m_dimensions.m_y;
-        for (auto i = 0u; i < numberOfStates; ++i)
-        {
-            m_codes.push_back(distribution(generator));
-        }
-    }
+		const auto numberOfBlockSetPos = (blockPointSetSet.size() + 1) * numberOfPuzzlePos;
+		for (auto blockSetPos = 0u; blockSetPos < numberOfBlockSetPos; ++blockSetPos) {
+			hashList.push_back(distribution(generator));
+		}
+	}
 
 private:
-    int blockStateCode(const Block &block)
-    {
-        const auto blockType = std::distance(begin(m_blockTypes), std::find_if(
-            begin(m_blockTypes),
-            end(m_blockTypes),
-            [&](const auto &type)
-        {
-            return type.width == block.m_sizeX && type.height == block.m_sizeY;
-        }));
+	int blockHash(const Block& block) {
+		const auto blockPos = std::distance(
+			begin(blockPointSetSet),
+			std::find_if(
+				begin(blockPointSetSet),
+				end(blockPointSetSet),
+				[&](const auto& b) { return b == block.pointSet; }
+			)
+		);
 
-        // FIrst blockType is runner, so index offset of the other blocks is 1
-        return m_codes[
-            ((blockType + 1) * (m_width * m_height))
-                + (block.m_startX * m_height)
-                + block.m_startY];
-    }
+		// First hash is runner, so index offset of the other blocks is 1
+		return hashList[((blockPos + 1) * numberOfPuzzlePos) + (block.shift.x * dimensions.y) + block.shift.y];
+	}
 
-    int runnerCode(const Block &runner)
-    {
-        // First blockType is the runner
-        return m_codes[(runner.m_startX * m_height) + runner.m_startY];
-    }
+	int runnerHash(const Block& runner) {
+		// First blockType is the runner : blockPos == 0
+		return hashList[(runner.shift.x * dimensions.y) + runner.shift.y];
+	}
 
-private:
-    // Dimensions of the playing field
-    int m_width;
-    int m_height;
+	// Dimensions of the playing field
+	const Point dimensions;
+	const int numberOfPuzzlePos;
 
-    // unique code for each blocktype positioned on the board
-    // Note: indexing scheme = [blockType][x][y]
-    std::vector<HashType> m_codes;
+	// unique hash for each blocktype positioned on the board
+	// Note: indexing scheme = [blockType][x][y]
+	std::vector<HashType> hashList;
+
+	// Unique hash for each block size
+	std::set<std::set<Point>> blockPointSetSet;
+};
 
-    // Unique code for each block size
-    std::vector<BlockSizeType> m_blockTypes;
-
-};

CMakeLists.txt

@@ -1,5 +1,21 @@
 cmake_minimum_required (VERSION 2.8.11)
 project (KLOTSKI-SOLVER)
 
-add_executable (run-tests test.cpp solver.cpp printer.cpp block.cpp MoveDiscovery.cpp MoveValidation.cpp puzzle.cpp)
-add_executable (solve main.cpp solver.cpp block.cpp MoveDiscovery.cpp MoveValidation.cpp puzzle.cpp)
+add_executable (solve      main.cpp MoveDiscovery.cpp MoveValidation.cpp block.cpp printer.cpp puzzle.cpp solver.cpp)
+add_executable (unit-tests test.cpp MoveDiscovery.cpp MoveValidation.cpp block.cpp printer.cpp puzzle.cpp solver.cpp)
+
+target_compile_options(solve PRIVATE
+#	-std=c++11
+	-pipe
+
+#	-O0
+
+	-g
+
+	-Wall
+	-Wextra
+	-Wpedantic
+
+	-Wno-unused-function
+)
+

MoveDiscovery.cpp

@@ -1,6 +1,36 @@
 #include "MoveDiscovery.h"
 
-namespace
-{
-    constexpr auto numberOfDirections = static_cast<int>(Direction::Number_of_dirs);
-}
+#include "MoveValidation.h"
+
+std::vector<Move> MoveRunnerFirst::gatherMoves(
+	const Point dimensions,
+	const std::shared_ptr<BoardState>& boardState,
+	const std::set<Point>& invalidPositions,
+	const Block* lastMovedBlock
+) const {
+	std::vector<Move> result{};
+
+	auto moveBlockIfPossible = [&](const Block& block) {
+		const size_t moveEffort = ((nullptr != lastMovedBlock) && (lastMovedBlock->id == block.id)) ? 0 : 1;
+		for (auto dir = 0; dir < static_cast<int>(Direction::Number_of_dirs); ++dir) {
+				if (validBlockPosition(
+					block.move(static_cast<Direction>(dir)),
+					dimensions,
+					*boardState,
+					invalidPositions
+				)) {
+					result.emplace_back(boardState, block, moveEffort, static_cast<Direction>(dir));
+				}
+			}
+	};
+
+	// Move runner first
+	moveBlockIfPossible(boardState->runner);
+
+	// Move other blocks second
+	for (const auto& block : boardState->blocks) {
+		moveBlockIfPossible(block);
+	}
+	return result;
+}
+

MoveDiscovery.h

@@ -1,47 +1,27 @@
 #pragma once
 
-#include <vector>
-#include <memory>
-
-#include "block.h"
 #include "puzzle.h"
-#include "MoveValidation.h"
 
-template <typename Validation = DefaultMoveValidation>
-struct MoveRunnerFirst
-{
-    template <int BlockCount>
-    static std::vector<Move<BlockCount>> gatherMoves(
-        const Point dimensions,
-        const std::shared_ptr<BoardState<BlockCount>> &currentState,
-        const std::vector<Point> &invalidPositions,
-        Direction previousDirection = Direction::Number_of_dirs)
-    {
-        std::vector<Move<BlockCount>> newMoves{};
+#include <vector>
+#include <set>
 
-        auto moveBlockIfPossible = [&](const Block &block)
-        {
-            for (auto dir = 0; dir < static_cast<int>(Direction::Number_of_dirs); ++dir)
-            {
-                if (dir != static_cast<int>(previousDirection))
-                {
-                    const auto newBlock = move(block, static_cast<Direction>(dir));
-                    if (Validation::validBlockPosition(newBlock, dimensions, *currentState, invalidPositions))
-                    {
-                        newMoves.emplace_back(currentState, block, static_cast<Direction>(dir));
-                    }
-                }
-            }
-        };
+class MoveDiscovery {
+public:
+	virtual std::vector<Move> gatherMoves(
+		const Point dimensions,
+		const std::shared_ptr<BoardState>& boardState,
+		const std::set<Point>& invalidPositions,
+		const Block* lastMovedBlock
+	) const = 0;
+};
 
-        // Move runner first
-        moveBlockIfPossible(currentState->m_runner);
+class MoveRunnerFirst : public MoveDiscovery {
+public:
+	virtual std::vector<Move> gatherMoves(
+		const Point dimensions,
+		const std::shared_ptr<BoardState>& boardState,
+		const std::set<Point>& invalidPositions,
+		const Block* lastMovedBlock
+	) const;
+};
 
-        // Move other blocks second
-        for (const auto &block : currentState->m_blocks)
-        {
-            moveBlockIfPossible(block);
-        }
-        return newMoves;
-    }
-};

MoveValidation.cpp

@@ -2,26 +2,58 @@
 
 #include <algorithm>
 
-namespace detail
-{
-    bool overlapsWithInvalidSpaces(const Block &block, const std::vector<Point> &invalidSpots)
-    {
-        return std::any_of(
-            begin(invalidSpots),
-            end(invalidSpots),
-            [&](const auto &spot)
-        {
-            return overlaps(block, Block{ spot.m_x, spot.m_y, 1, 1, block.id});
-        });
-    }
+bool overlapsWithInvalidSpaces(const Block& block, const std::set<Point>& invalidSpotSet) {
+	return std::any_of(
+		begin(invalidSpotSet),
+		end(invalidSpotSet),
+		[&](const Point& invalidSpot) {
+			return block.overlaps(invalidSpot);
+		}
+	);
+}
+
+bool overlapsWithBorder(const Block& block, const Point& dims) {
+	if (
+		(block.shift.x < 0)
+		||
+		(block.shift.y < 0)
+	) {
+			return true;
+	}
+	return std::any_of(
+		begin(block.pointSet),
+		end(block.pointSet),
+		[&](const Point& blockPoint) {
+			return
+				((block.shift.x + blockPoint.x) >= dims.x)
+				||
+				((block.shift.y + blockPoint.y) >= dims.y)
+			;
+		}
+	);
+}
 
-    bool overlapsWithBorder(const Block &block, const Point &dims)
-    {
-        return block.m_startX < 0 ||
-            block.m_startY < 0 ||
-            (block.m_startX + block.m_sizeX) > dims.m_x ||
-            (block.m_startY + block.m_sizeY) > dims.m_y;
-    }
+bool overlapsWithOtherBlocks(const Block& block, const BoardState& boardState) {
+	return
+		((block.id != boardState.runner.id) && block.overlaps(boardState.runner))
+		||
+		std::any_of(
+			begin(boardState.blocks),
+			end(boardState.blocks),
+			[&](const auto& other) { return (other.id != block.id) && block.overlaps(other); }
+		)
+	;
 }
 
+bool validBlockPosition(
+	const Block& block,
+	const Point& dims,
+	const BoardState& boardState,
+	const std::set<Point> invalidPositions
+) {
+	return !overlapsWithBorder(block, dims)
+		&& !overlapsWithInvalidSpaces(block, invalidPositions)
+		&& !overlapsWithOtherBlocks(block, boardState)
+	;
+}