asdfjkl / jfxchess

JFXChess - Chess Program
https://asdfjkl.github.io/jfxchess
GNU General Public License v2.0
100 stars 22 forks source link

setup_position_dragndrop #129

Closed TTorell closed 2 years ago

TTorell commented 2 years ago

Hi again Dominik!

Programming chess GUIs may seem completely unimportant in these times, but that's none the less what I've been doing in my spare time for a couple of days.

I got interested (before the invasion of Ukraine) in how the dragn'drop functionality was implemented in JerryFX. It all ended up in an implementation of dragn'drop in your DialogEnterPosition.java. I remembered that was on your TODO-list.

Start by having a look at the code in that class. There is a long comment in EnterPosBoard.java describing how it's supposed to work. Then try it. See if you like it. Actually it's a combination of dragn'drop and clicking. I've tried to comment the changes in that class maybe overly well. Maybe a helptext for the user would be good, but most users will find out by "trial and error" how it works, probably.

Feel free to change it as you wish.

Changes:

GameModel

Important! Here I found a windows-filepath that seems to have been forgotten in the code. To make the program run on my linux I just changed it temporarily. Maybe it's just to remove it and the comments around the rest of the method.

EnterPosBoard and GrabbedPiece:

My first approach was to only make changes inside EnterPosBoard, but...

After experimenting for a while it got messy and I decided to organize it a little better. A few of the the "loose" variables in enterPosBoard were tightly connected to the grabbing of a piece, so I extended the GrabbedPiece class and put them there, added two grab()-methods, one for the board and one for the pieceSelector. This made it easier for me and made the code in enterPosBoard a little more easy to read/understand, I think. I haven't changed the Chessboard class at all, it only uses the grabbedPiece just as it did before, for now.

I added some assert() statements to methods only used in the new mode. Then I learned that assert is default turned off even during debug in java, but at least you can see which methods are only meant for dragNDropMode.

I got some problems with the fact that we create a new Board-instance when we reset or clear the board. It was not really necessary, but I decided to make the board-reference inside enterPosBoard private and final so it becomes impossible to manipulate from the outside. Because of this slightly more object-oriented approach I had to add a number of wrapper-methods around the board-instance inside EnterposBoard.

Board

Here I added a very small improvement of isConsistent(). Pawns on their promotion-rank are not allowed anymore.

I needed some more ways to change the board without making a new instance, so I added the clear() and copy() methods. And I got rid of a few compilation warnings.

DialogEnterPosition

Some of the changes are just there to get rid of the warning: "static methods should be called in a static way" (Instead of instance.method() they can be called by class.method() and apparently we get a warning otherwise). The more important changes are that we don't create new instances when resetting the position and we call the wrapper-methods I mentioned before, instead of using enterPosBoard.board.method(). Just to fit my design that is. I'm not saying it's wrong to let the garbage collector work a little.

Important maybe! I couldn't understand the meaning of the currentBoard-reference, so I removed it. As it was implemented, it just didn't make sense to me. Now the user will get back the original position if he presses the current-position button. Maybe that's not what you intended. (If it was meant as some backup after we reset the position it would only work once as it was implemented.) It was basically only used as some kind of return value. I made a method instead to get the current position from the dialog after it has been closed by the OK-button.

DialogSearchGames

Was also affected by the changes described above. It seems to work well in this dialog too.

Backup

The intention is that setting the boolean dragNDropMode to false should make it work just as before.

asdfjkl commented 2 years ago

Thanks! This looks really nice. Just please give me some time to merge, currently quite busy with work...