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

[ImGui] Apply color theme from config/base_colors.json #76195

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

Conversation

vetall812
Copy link
Contributor

@vetall812 vetall812 commented Sep 4, 2024

Summary

Interface "Change ImGui style according to base colors definitions"

Purpose of change

I really like the integration of ImGui, but the interface elements created with this library stand out from the classic interface style. I wanted ImGui to use not only the font but also the basic colors for backgrounds or buttons.

Describe the solution

Unfortunately, I don’t possess sufficient knowledge of C++ to write proper code, so please treat this as a demonstration of capability. The base colors are loaded into windowsPalette in a specific order. I extract five colors from there, break them down into their components, and add transparency to create the desired effects. However, I believe there might be a better way to achieve this. Perhaps using ccolors, but my skills reach their limit here.

Describe alternatives you've considered

with the help from @mlange-42 :

  1. color_loader.h:
    move this part:
        ColorType ccolor( const std::string &color ) const {
            const auto it = consolecolors.find( color );
            if( it == consolecolors.end() ) {
                throw std::runtime_error( std::string( "requested non-existing color " ) + color );
            }
            return it->second;
        }

from private to public
2) sdltiles.cpp:
change code from this:

    color_loader<SDL_Color>().load( windowsPalette );
    init_colors();

    ImGuiStyle &style = ImGui::GetStyle();

    SDL_Color BLACK = windowsPalette[0];
    float BlackR = static_cast<float>( BLACK.r ) / 255.0f;
    float BlackG = static_cast<float>( BLACK.g ) / 255.0f;
    float BlackB = static_cast<float>( BLACK.b ) / 255.0f;

to that:

    color_loader<SDL_Color> loader = color_loader<SDL_Color>();
    loader.load(windowsPalette);
    init_colors();

    ImGuiStyle &style = ImGui::GetStyle();

    float BlackR = static_cast<float>( loader.ccolor("BLACK").r ) / 255.0f;
    float BlackG = static_cast<float>( loader.ccolor("BLACK").g ) / 255.0f;
    float BlackB = static_cast<float>( loader.ccolor("BLACK").b ) / 255.0f;

Testing

Additional context

dark
default
vector

@github-actions github-actions bot added [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 labels Sep 4, 2024
vetall812 and others added 6 commits September 4, 2024 15:01
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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 Sep 4, 2024
@vetall812 vetall812 marked this pull request as draft September 4, 2024 15:43
vetall812 and others added 7 commits September 4, 2024 22:17
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@vetall812
Copy link
Contributor Author

vetall812 commented Sep 4, 2024

I have tried to put it into init_colors function, but it became even more ugly. So I just left it as is for those who actually knows C++.

@vetall812 vetall812 marked this pull request as ready for review September 4, 2024 19:47
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 5, 2024
@db48x
Copy link
Contributor

db48x commented Sep 5, 2024

I‌ like the results! This will fix #75699 too.

However, I think that we should avoid transparent colors for the present. We’re not currently redrawing the whole UI every frame, so transparency tends to have some odd effects.

We can probably make the code a bit less repetitive too.

This should be easier to follow and easier to maintain.
@db48x
Copy link
Contributor

db48x commented Sep 6, 2024

I‌ sent you a little pull request on your own repository; see if you like it :)

@vetall812
Copy link
Contributor Author

added

I‌ sent you a little pull request on your own repository; see if you like it :)

That would be great, as I have little to no experience in C++ !

@github-actions github-actions bot removed the BasicBuildPassed This PR builds correctly, label assigned by github actions label Sep 6, 2024
@peter4370
Copy link
Contributor

I really like the 2077 color theme in last screeshot. Did you make it? Would you mind share the file?

@vetall812
Copy link
Contributor Author

I‌ like the results! This will fix #75699 too.

Could you please take a look, what could be wrong with it?

@db48x
Copy link
Contributor

db48x commented Sep 7, 2024

Could you please take a look, what could be wrong with it?

Sure. The test suite is failing because you put the code in init_colors which is called too early, before the ImGui context is created for the first time.

I would split it off into a separate function, perhaps called init_imgui_theme or something. You can then insert a call to it just after the ImGui context is created, which happens in cataimgui::client::client over in cata_imgui.cpp. That will ensure that everything happens in the right order.

@Maleclypse
Copy link
Member

Whatever this is doing the tests I can't kick them

@Procyonae
Copy link
Contributor

Sure. The test suite is failing because you put the code in init_colors which is called too early, before the ImGui context is created for the first time.

I would split it off into a separate function, perhaps called init_imgui_theme or something. You can then insert a call to it just after the ImGui context is created, which happens in cataimgui::client::client over in cata_imgui.cpp. That will ensure that everything happens in the right order.

I'm probably just being dumb but I'm having quite a bit of trouble getting this seemingly quite simple change to actually function, I'm just getting results like this where the font colours work but everything else is invisible (the PR as is works fine for me)
image

@db48x
Copy link
Contributor

db48x commented Oct 26, 2024

Wow, that actually looks really cool. Very cyberpunk. Could make a whole game aesthetic out of that.

Can you break in using a debugger and verify that the colors are all opaque? It looks like everything is 100% transparent, aside from the text.

@Procyonae
Copy link
Contributor

Wow, that actually looks really cool. Very cyberpunk. Could make a whole game aesthetic out of that.

Can you break in using a debugger and verify that the colors are all opaque? It looks like everything is 100% transparent, aside from the text.

Presuming this is what you were after (I've never used watch before, this feel like hacking to me ;D) most of the colours are just zerod (each entry here corresponds to the ImGuiCol_ enum, also the 4th cut off alpha number is also 0) which seems really weird, this is with leaving the default dark theme loading first so I think all the ones being overwritten with the ccolors are zeroing? I don't understand what could be causing that when the way the PR is currently works tho
image

@Procyonae
Copy link
Contributor

Ig it is a pretty messy process bc it's got the macros and the explicit conversion of the ccolors to imvec4s so maybe there's just something not reliable there that's falling apart?

@db48x
Copy link
Contributor

db48x commented Oct 28, 2024

That shouldn’t be happening.

@CLIDragon
Copy link
Contributor

The issue was because cataimgui::client::client is called before load_colors, so there are no colors to use.

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 [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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants