Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set minimum sizes for missions menu window #78811

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Kilvoctu
Copy link
Contributor

@Kilvoctu Kilvoctu commented Dec 28, 2024

Summary

SUMMARY: Interface "Set minimum sizes for missions menu UI"

Purpose of change

Set a minimum width and height for the missions window so information does not become unreadable at small terminal sizes
Fixes #78806
Fixes #74766

Describe the solution

Set the window to not go below the minimum terminal sizes of 80 by 24. Otherwise, it'll retain its current behavior.
Update the affected code as per @moxian's comment in linked issue.

Describe alternatives you've considered

Set a static size for the missions window

Testing

Game compiles and loads
Open missions window at maximum terminal size, minimum terminal size, 640x480 pixels size, and a bunch of random sizes
Test again on notiles

Additional context

Max terminal size
Screenshot 2024-12-28 013306

Min terminal size
Screenshot 2024-12-28 013249

A random size
Screenshot 2024-12-28 031542

@github-actions github-actions bot added Info / User Interface Game - player communication, menus, etc. Missions Quests and missions [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) labels Dec 28, 2024
src/mission_ui.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added the json-styled JSON lint passed, label assigned by github actions label Dec 28, 2024
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Dec 28, 2024
@mqrause
Copy link
Contributor

mqrause commented Dec 28, 2024

I'm not sure, but I don't think we enforce an actual minimum size in pixels? So with this it might be possible to have the ui be larger than the game window if you use smaller than default font sizes. It might be better to calculate minimum size from minimum terminal size. But that might just be irrelevant fringe cases we can ignore as well.

Copy link
Contributor

@moxian moxian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that it works in curses build as well (the Notiles in visual studio).
It definitely should, but I'm very slightly fearful.

src/mission_ui.cpp Outdated Show resolved Hide resolved
@Kilvoctu
Copy link
Contributor Author

I'm not sure, but I don't think we enforce an actual minimum size in pixels? So with this it might be possible to have the ui be larger than the game window if you use smaller than default font sizes. It might be better to calculate minimum size from minimum terminal size. But that might just be irrelevant fringe cases we can ignore as well.

Yeah... a cursory search through codebase, I didn't see anything like that lol. I wanted to keep it simple, but i'll fiddle around with it to try get something more proper and standardized. Would like for it to be all good, as there are other places with similar issues I'd like to address.

@Kilvoctu Kilvoctu marked this pull request as draft December 28, 2024 08:25
use only references to terminal sizes
use clamp
@Kilvoctu Kilvoctu marked this pull request as ready for review December 28, 2024 09:46
src/mission_ui.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the astyled astyled PR, label is assigned by github actions label Dec 28, 2024
Update src/mission_ui.cpp

Co-Authored-By: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Dec 28, 2024
@Kilvoctu Kilvoctu force-pushed the min_mission_ui_size branch from d34ed15 to 23aca45 Compare December 28, 2024 10:21
Copy link
Contributor

@moxian moxian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 28, 2024
@Brambor
Copy link
Contributor

Brambor commented Dec 28, 2024

src/mission_ui.cpp Outdated Show resolved Hide resolved
Co-authored-by: mqrause <[email protected]>
size_t window_width = str_width_to_pixels( TERMX ) / 2;
size_t window_height = str_height_to_pixels( TERMY ) / 2;
size_t table_column_width = window_width / 2;
float window_width = std::clamp( float( str_width_to_pixels( 80 ) ),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use EVEN_MINIMUM_TERM_WIDTH from game_constants.h.

The height should use EVEN_MINIMUM_TERM_HEIGHT.

We already have these defined as constants, so no additional work is needed besides using the existing constexpr.

Copy link
Contributor Author

@Kilvoctu Kilvoctu Dec 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented and tested.

Sorry for wasting everyone's time on something that's supposed to be simple..
I've looked at most of the UI code during developing my fork, and the execution of each window is so different, I don't know what's considered standard.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're not wasting anybody's time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions Missions Quests and missions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Increase missions menu UI size Mission window is needlesly small when screen is small
5 participants