asdfjkl / jfxchess

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

Nice Work #42

Closed Spill-The-Tea closed 5 years ago

Spill-The-Tea commented 5 years ago

Hi Dominik,

Just wanted to say that I like what you did removing the ribbon bar. It looks much cleaner. In the near future, I'll send a pull request to update the osx project file to reflect the changes (i.e. removing all the ribbon/files).

Also, I noticed that I happened to miss a forward slash in the previous pull request for icon paths in main_window.cpp, lines 67-68:

ifdef APPLE

QString resDir = QCoreApplication::applicationDirPath().append("../Resources"); 

It should instead be :

ifdef APPLE

QString resDir = QString resDir = ResourceFinder::getPath().append("../Resources");

Darn details. Also, I'm not sure how they look on your end, but the icons are disproportionally large on mine. So I typically change main_window.cpp, line 324 to multiply by 0.75 (not 1.5), to look like this:

QSize iconSize = toolbar->iconSize() this->devicePixelRatio() 0.75;

Anywho, thanks.

asdfjkl commented 5 years ago

thx very much for your patches!

cheers

Spill-The-Tea commented 5 years ago

Okay, I took a look and played around with it again, and you are right about main_window.cpp The second #ifdef is not needed and can be simplified to look like this (lines 704-717):

QAction* MainWindow::createAction(QString name, const QString &displayName, QSize &iconSize) {

    QString resDir = ResourceFinder::getPath();
    resDir.append("/res/icons/");
    resDir.append(name);
    resDir.append(".svg");
    QPixmap *iconPixmap = this->fromSvgToPixmap(iconSize, resDir);
    QAction *action = new QAction(QIcon(*iconPixmap), displayName);
    return action;
}

But it is absolutely needed in piece_images.cpp. I also previously attempted to create a patch for all directory paths to be defined in resource_finder, so that it could be easily modified in the future, but my endeavors failed. That said, I agree that would be a better solution. As is, it works, but makes the code a little messy.

Concerning the icon size, here is an image of the gui, before the change (... * 1.5):

Screen Shot 2019-04-28 at 4 57 17 PM

And here is the gui after the change ( * 0.75):

Screen Shot 2019-04-28 at 4 59 25 PM

The other issue I am running/looking into, is controlling the size / location of the engine window. This must be a problem of compiling it on mac (something something cocoa interface). Just for comparison, here is a screenshot of v3.06 (the last time Jerry.dmg was precompiled):

Screen Shot 2019-04-28 at 5 09 15 PM

Unfortunately, when I attempt to compile it myself, the engine window takes up the whole bottom, underneath the chessboard, making the board smaller. Furthermore, it can't be reduced in size any further, other than completely removing it... which isn't very helpful.

asdfjkl commented 5 years ago

managed to get ahold of an (ancient) Macbook. Currently setting things up, so I'll have the ability to debug myself. Would be highly appreciated if you could test a new build then... Dark mode is an issue thoug, since the latest macOS version this thing can run is High Sierra...

Spill-The-Tea commented 5 years ago

Hi Dominik, Will do. If I don't see it right away when you make any changes, just let me know and I can test it on my end.

I also created a pull request of minor fixes, (e.g. changed #elseif statements to #elif where applicable). Can you confirm it compiles without modification on your end (linux)?

Also, I didn't want to include modifications to icon size (in the tool bar of both main app menu, and database menu) in the pull request, because I'm not certain how it looks on other computers, or other platforms. But can you check out, how reducing the size changes how things look on your end also? Something like this:

dialog_database.cpp, line 34: QSize iconSize = toolbar->iconSize() * this->devicePixelRatio()*0.75;

main_window.cpp, line 323 or 324: QSize iconSize = toolbar->iconSize() * this->devicePixelRatio() * 0.75;

Lastly, I noticed that you removed the icon option to change the board and chess pieces appearance from the main menu. Do you have plans to present this differently?

asdfjkl commented 5 years ago

I've created several patches and a new build environment on macOS. You can try a new alpha build here. https://github.com/asdfjkl/jerry/releases/tag/Unsorted_Alphas Will also consider regular macOS builds from now on, as I finally have a test machine. I will probably also close this issue soon, as there are too many things mixed by now within this thread.

The new layout (all engine output below board) is necessary to get more space on the right pane (#todo: opening book explorer), game progress graph etc. It's a no issue on other platforms, but vertical space is tight on macOS, especially on low res screens. Added some patches to get more vertical space.

Board and Chess Piece Icons: I'd consider this is not an option users use frequently i.e. removed it from the toolbar. It's of course available from the menu. Will see how big (horizontal) the toolbar turns out on various platforms and screen sizes, and may add it back later on...