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

Fix popup for modifying string options #61284

Merged

Conversation

ZeroInternalReflection
Copy link
Contributor

Summary

None

Purpose of change

#61166 added a check and debugmsg to warn if a string was being trimmed to a length <= 0, because that suggests something strange is going on in the UI layout. This debugmsg pops up when setting string options. It can be ignored without a crash, but still suggests something odd is going on.

Before.mp4

Fixes #61266

Describe the solution

The root cause appears to be:

  1. String_input_popup trims the title before printing based on the variable titlesize. If the title was not set, titlesize will be 0
  2. While setting int/double values in the options menu sets the title, setting string values uses the same variable to set the description, and leaves the title unset. (Compare the colour of the prompt between an int value and the video above):
    Comparison

This PR:

  1. Checks if the string_input_popup has a title. If it doesn't then it no longer tries to print the title
  2. Sets the title rather than the description when modifying string options

The second change is because I felt the formatting of string options should match the formatting of int/double options.

Describe alternatives you've considered

  • Set a default title for string_input_popups, so that if .title() is not called, the popup will display "DEFAULT_TITLE" or something similar to indicate that it was missed
    There's probably a valid reason somewhere to have a blank title in the popup, and setting a default title would not prevent someone from calling .title("") and potentially ending up in the same spot.

Testing

I believe the report of the same debugmsg popup when modifying default gameplay shortcuts on Android should be fixed as well, but I am unable to test it.

I opened a variety of string_input_popups, including modifying string options:
After

At several window sizes, and in several non-English languages:
80x24_Ukrainian

A search of the code found two other locations where .title() is not called on a string_input_popup. Editing an item in the map editor:
Map_editor
And looking at a different faction's zones in debug mode
Zone_manager_debug_mode

Both locations would hit a debugmsg prior to this PR, both work correctly afterwards. I chose not to make any changes to their formatting.

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Sep 26, 2022
@dseguin dseguin merged commit 328359e into CleverRaven:master Sep 27, 2022
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` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Settings interface for default character name produces debug message for trim_by_length
2 participants