Conversation
The layout caches the menu table's size hint and nothing invalidated it when the contents changed, so a menu window that the core reuses (the inventory window, WIN_INVEN) kept the size of the first menu ever shown in it. Invalidate the cached hint explicitly and size the dialog from the fresh value. Resizing directly instead of via adjustSize() also drops adjustSize()'s two-thirds-of-screen cap, which is too small for a long inventory in a large font; use 90% of the available screen instead, and leave room for the vertical scroll bar when the list is taller than that. Text windows get the same treatment. Replaces the FIXME/TEMPORARY notes about inventory window size in qt_menu.cpp.
crissman
marked this pull request as draft
September 7, 2026 00:44
crissman
marked this pull request as ready for review
September 7, 2026 01:14
Author
|
Is sizing Qt menu and text windows to their contents a change you’d welcome? I’d appreciate feedback on the approach or any revisions needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Menu windows in the Qt interface open far too small — often a few rows of a
long inventory — and the inventory window keeps whatever size it had the first
time it was shown, because the core reuses one window for WIN_INVEN and the
layout's cached size hint was never invalidated. The FIXME/TEMPORARY comments in
qt_menu.cppdescribe this.This change:
MenuResize()and sizes thedialog from the fresh hint, so reused windows track their current contents;
adjustSize(), whose two-thirds-of-screen cap istoo small for a long inventory in a large font — cap at 90% of the available
screen instead, adding the vertical scroll bar's width when the list is taller
than that;
NetHackQtTextWindow::Display()the same 90% cap.Tested on macOS 26, x86_64, Qt 6.11.1: a 21-row inventory in the "Huge" font now
opens at 623×731 with every row visible; shorter menus shrink to fit. Also
compile-checked against current
NetHack-5.0.🤖 Generated with Claude Code
(Reviewed by human Crissman)
https://claude.ai/code/session_01WJwWF29Bgj7KvkVqPrVr5M