basile-henry / infinisweep

A Minesweeper clone with infinite grid written in Haskell.
MIT License
33 stars 3 forks source link

Add Nethack / vi-mode style movement options #2

Closed traytonwhite closed 8 years ago

traytonwhite commented 8 years ago

This version adds new Move data constructors to represent diagonals. I'm not sure if it would be better to have the diagonal keys trigger the separate movements in sequence versus this implementation.

Includes hjkl key movements for left, down, up, right. Also yubn for up left, up right, down left, and down right. Additionally, use number pad movements with similar layout.

Update README.md with new movement options.

basile-henry commented 8 years ago

Hey :) Very nice contribution! I thought of the vi style inputs but not using them myself I forgot about it. I love the keypad inputs! The diagonals make sense for faster input. Though I think there is a problem with the implementation of the diagonals. It is noticeably slower then the other inputs.

I think the problem is due to the way you implemented the diagonal in the "movePosition" function (the list of cells). To be fair I should have probably commented what this function does. It updates the position and also the "Panel" which is the region of the grid which is active. In this function I am calling "getEmptyCells" which recursively opens the cells that don't have mines around. It is actually a pretty expensive function. So I call it once on a cell that opens and also in this movePosition to open the cells at the edge of the new Panel (the Panel has moved). With the diagonal you are updating the whole panel instead of just one line on the new edge. ^^

I'll try to solve this small problem and then I'll merge your pull request. Thanks! :)

basile-henry commented 8 years ago

The diagonal issue was an easy fix. I've now merged your changes! :+1:

traytonwhite commented 8 years ago

Thank you for the fix, and the explanation! Definitely helped my understanding with what those list comprehensions were doing.