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

refactor: Consolidate multiline strings in feature code #143

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

rjwignar
Copy link
Contributor

Summary of Changes

Following our discussion in #119 (comment), I searched for and handled consecutive ImGui::Text() calls in the feature code
(in the src folder).
I used git grep to find module files that have ImGui::Text() calls and consolidated tooltips that use consecutive ImGui::Text() calls.

I modified tooltips in the following modules:

  • DistantTreeLighting:

    • Complex Tree LOD Tooltip
    • Subsurface Scattering tooltip
  • ExtendedMaterials:

    • Complex Material Checkbox tooltip
    • Enable Parallax tooltip
    • High Quality checkbox tooltip
    • Blend Range slider tooltip
    • Enable Shadows checkbox tooltip
  • GrassLighting:

    • Subsurface Scattering (SSS) Amount slider tooltip
  • LightLimitFix:

    • Lights Visualization Mode combo box tooltip

I noticed all tooltips above originally contained newline characters, \n.
I removed them in my changes, but I commented out the original tooltips for reference in case I need to preserve the newline characters.
If the newlines don't need to be preserved, I can remove the commented-out code and update the PR.

Thank you for considering these changes.

DistantTreeLighting:
* Complex Tree LOD Tooltip
* Subsurface Scattering tooltip

ExtendedMaterials:
* Complex Material Checkbox tooltip
* Enable Parallax tooltip
* High Quality checkbox tooltip
* Blend Range slider tooltip
* Enable Shadows checkbox tooltip

GrassLighting:
* Subsurface Scattering (SSS) Amount slider tooltip

LightLimitFix:
* Lights Visualization Mode combo box tooltip
@alandtse
Copy link
Collaborator

Thanks. I think the newlines can be removed but @doodlum, what do you think?

@alandtse alandtse changed the title Consolidated multiline strings in feature code refactor: Consolidate multiline strings in feature code Nov 19, 2023
@rjwignar
Copy link
Contributor Author

Build available at https://github.com/rjwignar/skyrim-community-shaders/releases/tag/menuMultilines for testing.
I also recently bought Skyrim SE on Steam. I'll need some time to configure SKSE and Community Shaders though.

@doodlum
Copy link
Owner

doodlum commented Nov 19, 2023

Thanks. I think the newlines can be removed but @doodlum, what do you think?

I believe that the new lines were included to make the text easier to read/digestible. But we did also have issues with text wrapping previously.

@alandtse
Copy link
Collaborator

Ok, let's drop the newlines. \n is probably not a good way to handle text formatting. I wonder if ImGui actually supports paragraph tags and more advanced syntax for grouping.

 * ExtendedMaterials.cpp
 * GrassLighting.cpp
 * LightLimitFix.cpp
@rjwignar
Copy link
Contributor Author

I've updated the PR, with the commented-out original tooltips (that used newlines) removed.
Latest build available at https://github.com/rjwignar/skyrim-community-shaders/releases/tag/menuMultilines.

src/Features/DistantTreeLighting.cpp Outdated Show resolved Hide resolved
src/Features/GrassLighting.cpp Outdated Show resolved Hide resolved
@rjwignar rjwignar requested a review from alandtse November 22, 2023 05:18
@alandtse alandtse merged commit a83f83d into doodlum:dev Nov 22, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants