qt client on Mac - "Game" menu missing some menu items
ddeanbrown Without a Mac to reproduce and debug I don't know what I can do to this. Checking the source code for anything obvious, but not much else.
While cannot debug, can at least make it easier for someone else to do. First such a refactoring patch -> #43996
There's also a small chance that some such refactoring for cleaner code removes actual bug.
That data member named 'menu' (name that is not very namespace-safe) makes me wonder if there could be variable shadowing by some system header in-between. Even --enable-debug does not currently check for shadow warnings on C++ code (as headers of some of the supported Qt versions have those internally)
Second refactoring patch: #44062
Reply To (Anonymous)
https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar
So, on Mac, any menu entry named "Options" gets moved to "Application Menu | Preferences" ?
What we do in that case? Accept that the current behavior is correct for Mac? The existence of this ticket indicates that it's not obvious even for Mac users where the options could be found.
Also, it's "Options" only in English. I guess the behavior will be different when the menu is constructed by translated texts.
ddeanbrown Mac 3.0.0 version of freeciv with qt client does not have a menu item "Preferences" in the application menu (which is called "freeciv"). That menu has an "About freeciv" menu item, and the other items are generic MacOS stuff not related to freeciv. Because there are other Game menu items missing in addition to "Options", I suspect there is something else causing the problem, and possibly the other refactoring patches may fix it.
Reply To ddeanbrown
Because there are other Game menu items missing in addition to "Options"
What else is it missing? Note that it isn't even supposed to have as many items as, e.g. , gtk-clients. Qt-client is still missing number of features.
Reply To ddeanbrown
Mac 3.0.0 version of freeciv with qt client does not have a menu item "Preferences" in the application menu (which is called "freeciv").
That documentation speaks of moving "Options" (matched by the string) to applications menu, where it would fire like it was in the original location. Now, freeciv's "Options" is not an item that would fire, but a submenu that one should be able to browse from the main menu. The documentation does not mention how Qt is supposed to handle that corner case - it's quite possible that it doesn't, but has a buggy results like removing the original "Options" but then being unable to add the replacement.
One thing we could test is just renaming the item from "Options" to something that does not trigger that special behavior. Can you test with the attached test patch "SettableThings.patch"? Has textual dependency on #43996 and #44062
alienvalkyrie https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar
There's a line there saying
You can override this behavior by using the QAction::menuRole() property.
which sounds like it should be possible to solve the whole issue that way; avoiding moving the menu entries / submenus at all.
Reply To alienvalkyrie
https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar
There's a line there saying
You can override this behavior by using the QAction::menuRole() property.
which sounds like it should be possible to solve the whole issue that way; avoiding moving the menu entries / submenus at all.
Except that the menuRole is property of QAction, not QMenu (submenu).
ddeanbrown We have a winner. I tried renaming "Options" and that fixes the problem, even without the other patches. Then I tested using Unicode zero-width space character in the renaming, and that works, so the rename is invisible. Obviously needs a comment. Here's my suggested fix, line 1020 of menu.cpp
// \u200B is Unicode zero-width space character, prevents qt from trying,
// and failing, to move a menu named "Options" to the application menu's
// "preferences" item on MacOS.
// see https://doc-snapshots.qt.io/qt5-5.15/qmenubar.html#qmenubar-as-a-global-menu-bar
menu = menu->addMenu(_("\u200BOptions"));
I'm not savvy enough with git to fix this myself, better if someone else handles it. And can then close this ticket.
Screen shot of fix attached.
ddeanbrown)
Evolved the fix a bit - keep the translatable string as is + have this hack on MacOS only. It would be good if you can confirm that this still works.
Also a version that I plan to still push to S2_6
ddeanbrown Yes that works and is better. Wasn't sure if you could do #ifdef for OS. And good to keep translatable.
In the end, there's still some differences between qt and gtk - gtk has "Clear Chat Log", "Save Chat Log" and "Reload Tileset", which qt lacks & qt has "Shortcuts" and "Load another tileset", which gtk lacks. Not a big deal.
- S3_1 & S3_0 version that applies without #44062
Reply To ddeanbrown
In the end, there's still some differences between qt and gtk - gtk has "Clear Chat Log", "Save Chat Log" and "Reload Tileset", which qt lacks & qt has "Shortcuts" and "Load another tileset", which gtk lacks. Not a big deal.
Qt-client does not have those Chat Log related functionalities, and it implements Reload Tileset functionality as a shortcut. "Shortcuts" and "Load another tileset" are more or less broken Qt-client only features, and also in conflict with some new features we'd like to implement to freeciv.
Cloning this ticket from hostedredmine. Seen in 3.0.0
Game menu of the freeciv-qt client has these 4 items
Save Game As…
Save Map to Image
-----
Leave Game
gtk* clients have 10 items. The biggest missing one is the "Options" item which leads to the "Local Client" and "Remote Server" sub-items, so no way to change Local Client options while playing a game.
Chippo chiphog@gmail.com says it works OK for him "In the QT client on Loonix", so may be Mac only or just some Unices.