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

Text on the debug "Add effect" in "Edit player/NPC" function clips out of window #78895

Open
JaxMutt opened this issue Jan 1, 2025 · 7 comments
Labels
(S1 - Need confirmation) Report waiting on confirmation of reproducibility

Comments

@JaxMutt
Copy link

JaxMutt commented Jan 1, 2025

Describe the bug

When opening the window for editing the effects on the player the text is clipping out of the window.

Attach save file

N/A

Steps to reproduce

  1. Open debug menu
  2. Select "Player..."
  3. Select "Edit Player/NPC"
  4. Select "You" or an NPC
  5. Select "Add an effect"

Expected behavior

After selecting the option to edit the effects on an NPC or yourself, a ImGui window will open and the text displayed will clip out of the window.

Screenshots

image

Versions and configuration

  • OS: Windows 11
  • Game Version: Cataclysm-DDA experimental build 2024-12-28-1716 (64-bit)
  • Graphics version: Tiles
  • Game Language: System language [English]
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth],
    No Hope [no_hope],
    Culinary Days Ahead [culinary_days_ahead],
    Alternative Map Key [alt_map_key],
    SpeedyDex [speedydex],
    Stats Through Skills [StatsThroughSkills],
    Stats Through Kills [stats_through_kills],
    Tamable Wildlife [Tamable_Wildlife]
    ]

Additional context

No response

@JaxMutt JaxMutt added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Jan 1, 2025
@Kilvoctu
Copy link
Contributor

Kilvoctu commented Jan 2, 2025

Same core issue as #78316 and #78758

Coincidentally I've been fiddling around in this area of the code today (it's in uilist::calc_data()) but my solutions have been rather janky so far..
Screenshot 2025-01-01 184146
Screenshot 2025-01-01 184214

@JaxMutt
Copy link
Author

JaxMutt commented Jan 2, 2025

maybe it's something I can fix in the ImGui demo settings? or maybe the terminal settings?
I also tried hopping between fullscreen mode settings and it didn't fix it.

@JaxMutt JaxMutt closed this as completed Jan 2, 2025
@JaxMutt JaxMutt reopened this Jan 2, 2025
@Kilvoctu
Copy link
Contributor

Kilvoctu commented Jan 2, 2025

From what I understand after staring at the code, is that there are many checks to ensure the window will display all contained information, but no checks against the width of the terminal (ImGui::GetMainViewport()->Size.x).

This is the janky fix I've been experimenting with Kilvoctu@c7d632b. It seems to work for this and other windows I've been testing with. However, idunno C++ and am not familiar with ImGui, so it's probably bad and illogical.
Need more experienced dev to chime in

@JaxMutt
Copy link
Author

JaxMutt commented Jan 2, 2025

Sounds good, I'll wait a few weeks and re-update the build and see if it's fixed.
Thanks for the help :)

@moxian
Copy link
Contributor

moxian commented Jan 2, 2025

@Kilvoctu Clipping to ImGui::GetMainViewport()->Size.x "works" but it obviously cuts off the extra text which is not desirable.
The proper solution here would be to allow word-wrapping - see the arguments to ImGui::CalcTextSize. But to use that we need to rewrite calc_data() almost from scratch so that it first determines the width of the popup and only then decides on the height of the individual elements (with account for word-wrapping).

I tried to do so myself, but ran into issue that i couldn't quite get the padding right (see pic in #78771 for the in-progress not-quite-working state) . There are like four different kinds of padding, and it's not clear when to use which one. I just now pushed the code to moxian/Cataclysm-DDA@dcd2e1d but it's been a couple of days since i last touched it, and i'm not sure if it even builds in the current state - there might be a syntax error or something.

While taking a shower, I got an idea that my padding woes can perhaps be fixed by telling ImGui to auto-resize the window appropriately (and stop trying to set the margins by hand), but I never got around to investigating how to do this, and if it's even possible

Idk if this helps, but if you want to get this code into workable state - you're more than welcome to (no pressure tho).

@Kilvoctu
Copy link
Contributor

Kilvoctu commented Jan 2, 2025

Idk if this helps, but if you want to get this code into workable state - you're more than welcome to (no pressure tho).

It's all very helpful, thanks! I'll continue fiddling around with it, though it's probably outside my skill level. My only decent skill is reading and interpreting code so providing examples is great.

@db48x
Copy link
Contributor

db48x commented Jan 14, 2025

telling ImGui to auto-resize the window appropriately

Sadly, that will just make ImGui enlarge the window to fit whatever gigantic piece of text we’re trying to display, which the exact same problem we already have. :) Autoresize is usually for windows whose contents change size as the user interacts with it.

Most of the complexity of the uilist exists because it is trying to be all things to all callers. That said, now that we have cataimgui::TextColoredParagraph() which wraps paragraphs containing color tags correctly we can go back and update uilist::calc_data to rely on text wrapping when the desired_bounds give the window a specific width. But that is an ambitious fix, and can easily wait.

Currently most callers wrap the text to a convenient width before giving it to the uilist to display. This is a much simpler fix to implement. For example, take a look at this code in handle_action.cpp:

auto wrap60 = []( const std::string & text ) {
return string_join( foldstring( text, 60 ), "\n" );
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(S1 - Need confirmation) Report waiting on confirmation of reproducibility
Projects
None yet
Development

No branches or pull requests

4 participants