search

michaelbutler / minesweeper

WebWorker powered Minesweeper game written in JavaScript, HTML and CSS.
https://michaelbutler.github.io/minesweeper/
GNU General Public License v3.0
39 stars 31 forks source link

Added custom dimension validation/rationalisation #11

Closed FixItDik closed 8 years ago

FixItDik commented 8 years ago

This change introduces limits for the dimensions of custom mine fields.

The limits have been set to be between 1 and 30, exceeding either limit (so less than 1 or more than 30) forces the code to modify that dimension to the limit it has passed. The limit for the number of mines is set to be the number of squares in the minefield minus one (i.e. there is always one square you can click on that is not a mine). The lower limit is hard-coded to 1 but the upper limits are set in variables at the top of the code (MAX_X and MAX_Y) so others may change them if they wish.

This resolves #6.

FixItDik commented 8 years ago

No need for you to apologise at all Michael, it is me causing the problem, hopefully fixed it but at the same time I have also learnt that, until you accept the pull request, any other changes I make get included so this version of the code also implements the ctrl/shift modifier of the mouse click (so to clear multiple cells around a solved cell, hold down ctrl or shift key while left-clicking on cell, behaviour is same as using both left and right click simultaneously which obviously remains in the code ). However, in the process I did open the file in a Microsoft IDE and it got converted to DOS format, I have undone that but since then I am getting this compile error which I cannot find enough detail on to determine a solution. Worst case I may revert the code to the previous version and try implementing changes one by one until it goes wrong again but I was wondering if you can see what this error is all about?

michaelbutler commented 8 years ago

If you're talking about the red X, don't worry about it right now, I just activated that feature for this project, but it's not set up yet so it will fail every time.

FixItDik commented 8 years ago

Morning Michael, yup, it was the red cross that was worrying me :) Fixed the IF statements, sorry again about that :) As you can probably tell I am working on this during commute or during breaks, as much as anything I am really enjoying learning about GitHub, thank you again!

michaelbutler commented 8 years ago

Hey Dik, I've completed the travis-ci integration for this project. Now all pull requests will be automatically run, so you can see if your committed code passes. You'll have to pull down the latest master branch and update your branch to continue. You can read about "how to update my branch with master" on google for more information.

If you visit https://github.com/michaelbutler/minesweeper you'll see a green "build passing" icon.

michaelbutler commented 8 years ago

After you update with the master branch, you can run these commands locally to test your build locally: (First install nodejs and npm on your machine using a guide) npm install grunt travis -v

It should give you a list of errors.

FixItDik commented 8 years ago

Hi Michael, so I installed the GIT desktop (as you can't do a rebase through the web site) and ran the "git rebase $REVISION" command, no errors reported but then I tried to run the NPM and GRUNT commands and they weren't even recognised (if I just enter "npm install" I get "npm is not a recognized cmdlet..." or if I enter "git npm install" I get "git: npm is not a git command", same with grunt). I am tempted to get you to ditch this pull request all together, I'll delete all the GIT files I have and then start again, it's only a dozen lines or so that I have changed and this feels like a lot of effort just to get those lines checked in so I can do a few more. Sorry mate, a little frustrated by this GIT stuff now as I can't work out what part is on the server and what part is here on my PC (so perhaps the rebase only updated my local files but not the ones on the server or something) all very confusing - was happier just taking the files form the server, changing them, putting them back on the server then asking you to accept the changes. The desktop is telling me there are no differences between my local files and the ones on the server but the pull request is still showing errors - eek! If you would rather I just start again just say, I will uninstall GIT desktop and go back to the simple world :)

FixItDik commented 8 years ago

OK - just closed and opened desktop and got "Updates from michaelbutler" which I clicked and is said there were 2 conflicting files - very easy to resolve (the readme wasn't even a conflict so the merge just didn't work correctly - it was just some new lines you added) - I have clicked the Sync option on the desktop and it is now saying no conflicts so have come back here to see what it thinks :)

michaelbutler commented 8 years ago

@FixItDik ok glad you were able to update and resolve the conflicts -- using Git desktop is a good idea, for now! You definitely do not want to "download files, make changes, and upload them" ... this is very not inefficient and goes against all workflow improvements over the past 20 years.

You can see that the build failed still -- click "Details". It will give you why it failed. Mostly just wrong indentations (must use 4 spaces, no tab characters) and some == or != instead of === and !==. 😄

Keep it up I'm sure you'll get there!

FixItDik commented 8 years ago

Woo hoo - got there, not only that but I learnt a lot - if you are happy to accept this pull request I'll start another one for the last couple of suggested improvements and then leave you alone to get on with some real work for a while :) Thank you again for all your patience!

michaelbutler commented 8 years ago

Looks good! Thanks for the improvements. Merging now.