ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
61.15k stars 10.3k forks source link

Dynamic Error Modals - spawning them with runtime errors - handling their return values #7533

Open MathieuTuli opened 6 months ago

MathieuTuli commented 6 months ago

Version/Branch of Dear ImGui:

Version 1.90.6, Branch: master

Back-ends:

imgui_impl_glfw.h + imgui_impl_opengl3.h

Compiler, OS:

macOS + Clang 14

Full config/build information:

No response

Details:

My Question:

I'm having trouble figuring out the best way to spawn dynamic error modals. Mainly, I want to display a modal with the error and an option to click ok or report whenever one is caught. So the modal will have a dynamic title and error message that is generated at runtime and it may be spawned/opened from different places.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

Example of a function for the error modal split from the main loop:

uint8_t ErrorModal(const char * title, const char * err)
{
    ImGui::SetNextWindowPos(ImGui::GetMainViewport()->GetCenter(),
                            ImGuiCond_Appearing,
                            ImVec2(0.5f, 0.5f));
    ImGui::SetNextWindowSize(ImVec2( 200.f, 100.f), ImGuiCond_Always);
    ImGuiWindowFlags popup_flags = 0;
    popup_flags |= ImGuiWindowFlags_NoNav;
    popup_flags |= ImGuiWindowFlags_NoResize;
    uint8_t ret = 0;
    if (ImGui::BeginPopupModal(title, NULL, popup_flags))
    {
        ImGui::BeginChild("error message",
                          ImVec2(ImGui::GetContentRegionAvail().x * 0.95f, 30.f),
                          ImGuiChildFlags_None,
                          ImGuiWindowFlags_HorizontalScrollbar);

        ImGui::Text("something went wrong %s", err);
        ImGui::EndChild();
        ImGui::Separator();
        if(ImGui::Button("o.k."))
        {
            ImGui::CloseCurrentPopup();
            ret = 2;
        }
        ImGui::SameLine();
        if(ImGui::Button("report"))
        {
            ImGui::CloseCurrentPopup();
            ret = 1;
        }
        ImGui::EndPopup();
    }
    return ret;
}

Effectively, the ideal loop would look something like this:

void loop()
{
    ...
    ImGui::Begin("help me");
    if(ImGui::Button("click me"))
    {
        uint8_t ret = OpenSomeDialogOrSomething();
        if (ret == 0) { DoSomething(); }
        else if (ret == 1) { ErrorModal("woops", "oopsie"); ImGui::OpenPopup("woops"); }
    }
    ImGui::End();
    ...
}

But, of course, this doesn't work: the next frame wouldn't enter the error condition and would stop rendering the modal? (I'm new to imgui so not sure what the proper term is here lol). Now, you can do something like this:

void loop()
{
    ...
    ImGui::Begin("help me");
    if(ImGui::Button("click me"))
    {
        uint8_t ret = OpenSomeDialogOrSomething();
        if (ret == 0) { DoSomething(); }
        else if (ret == 1) { ImGui::OpenPopup("errorpop");  }
    }
    ErrorModal("errorpop", "oopsie");
    ImGui::End();
    ...
}

So that each loop, the error modal is rendered and I can just selectively open it upon error. Maybe I just define all my error modals in the main loop and call them when I need to, but then I can't do dynamic error messages, so this won't work.

I'm struggling to wrap my head around this and how to get this to work, maybe I'm just not educated enough on the IMGUI paradigm.

Appreciate any help :)

P.S. You'll also notice in my ErrorModal definition, that it might return a value depending on the button clicked. I realize also that handling that return is non-trivial. Perhaps it's better to directly call a handler rather than return a status code. Any comments on this would be useful also.

GamingMinds-DanielC commented 6 months ago

My favorite approach for modals like this is to register them for deferred display and handle the feedback in a lambda function. In your case the usage could look something like this:

if ( someError )
{
    ErrorModal( "title", "message", []( int result )
    {
        switch ( result )
        {
        case 0: // OK
            break;
        case 1: // cancel
            break;
        }
    } );
}

ErrorModal in this case would be a template function to allow for different types of callbacks (function pointer, callable object, non-capturing lambda, capturing lambda). The template function would not display the modal, but register it for later use. Title, message and configuration (if you want f.e. give it a combination of buttons to display) would be copied into an object or array (if you want to be able to have multiple open at once) to be handled later in the update loop.

If you are not familiar with advanced techniques like templates and lambdas, they will most likely be too tricky to get right. But if you are, you can make highly reusable and very easy to use code.

MathieuTuli commented 6 months ago

This makes so much sense, I like this a lot. Inline with what I was starting to piece together.

Yeah templates/lambdas won't be an issue, thanks!

MathieuTuli commented 6 months ago

Posting here for anyone that may come across this and wants to see a solution.

template <class... T>
struct DeferredDisplay {
  std::tuple<T...> args;
  std::function<int(T...)> callable;
};

template <class... T>
std::vector<DeferredDisplay<T...>> DeferredRegister;

template<typename Func, typename... Args>
void RegisterForDeferredRender(Func&& callable, Args&&... args) {
    using Signature = std::function<int(std::decay_t<Args>...)>;
    DeferredRegister<std::decay_t<Args>...>.push_back(DeferredDisplay<std::decay_t<Args>...>{
        std::make_tuple(std::forward<Args>(args)...), Signature(std::forward<Func>(callable))});
}

This defines the register and registeration function. I can then say create a function that defines my error modal:

int ErrorModal(const char *title, const char *err) {
  // ImGui::SetNextItemOpen(true, ImGuiCond_Always);
  ImGui::SetNextWindowPos(ImGui::GetMainViewport()->GetCenter(),
                          ImGuiCond_Appearing, ImVec2(0.5f, 0.5f));
  ImGui::SetNextWindowSize(ImVec2(200.f, 100.f), ImGuiCond_Always);
  ImGuiWindowFlags popup_flags = 0;
  popup_flags |= ImGuiWindowFlags_NoNav;
  popup_flags |= ImGuiWindowFlags_NoResize;
  int ret = 0;
  if (ImGui::BeginPopupModal(title, NULL, popup_flags)) {
    ImGui::BeginChild(
        "error message", ImVec2(ImGui::GetContentRegionAvail().x * 0.95f, 30.f),
        ImGuiChildFlags_None, ImGuiWindowFlags_HorizontalScrollbar);

    ImGui::Text("something went wrong %s", err);
    ImGui::EndChild();
    ImGui::Separator();
    if (ImGui::Button("o.k.")) {
      ImGui::CloseCurrentPopup();
      ret = 1;
    }
    ImGui::SameLine();
    if (ImGui::Button("report")) {
      ImGui::CloseCurrentPopup();
      ret = 2;
    }
    ImGui::EndPopup();
  }
  return ret;
}

This will return 0 normally, 1 if they just clicked ok, and 2 if they clicked the report button. Now in my main loop, I can do the following:

ImGui::Begin("Main Window");
...
// error occured somewhere in some nested logic
RegisterForDeferredRender([](const char *title, const char *msg){
        int ret = ErrorModal(title, msg);
        if (ret == 1){
        std::cout << "they clicked ok" << std::endl;
        }
        else if (ret == 2)
        {
        std::cout << "they clicked report" << std::endl;
        }
        return ret;
        }, "file upload error", "err");
ImGui::OpenPopup("file upload error");
...
int i = 0;
while (i < DeferredRegister<const char*, const char*>.size())
{
    int ret = std::apply(DeferredRegister<const char*, const char*>[i].callable,
                         DeferredRegister<const char*, const char*>[i].args);
    if (ret)
        DeferredRegister<const char*, const char*>.erase(DeferredRegister<const char*, const char*>.begin() + i);
    else
        ++i;
}
...
ImGui::End();

This will:

It gets me 90% of the way there at the moment, and only handles callbacks that return int but that's all I need for now. It's not perfect but it works. This example sacrifices readability, which I will most likely fix in my own code.

Please feel free to tell me how to improve it if you'd like.

GamingMinds-DanielC commented 6 months ago

Please feel free to tell me how to improve it if you'd like.

Your solution will depend on where the registered callbacks will be executed. ImGui::OpenPopup() will use the current ID stack (from the currently open window), so this approach is kinda dangerous. Nothing that can't be solved, just a potential pitfall.

MathieuTuli commented 6 months ago

For sure, yeah I've been ignoring details like that I'll have to handle that soon. Thanks for the tip

MathieuTuli commented 6 months ago

Okay so I've been thinking on this. I can see how this would break if say the registration was for e.g. wrapped in a TreeNode and thus the OpenPopup would have a different ID stack than the modal itself (which wouldn't have the TreeNode as part of its ID stack). Is there a "proper" way to handle this? I can see two paths:

  1. You just make sure you're aware of the stack whenever you register something for deferred rendering. For example, calling TreePop before registering inside a TreeNode or something idk. Ensuring the render/opening are on the same ID stack. This assumes that this behaviour can be enforced, although I may run into unforeseen issues.
  2. You additionally pass the current stack to the register and Push/Pop it before rendering it later. But, you would need to somehow resolve the difference in the stack at the time of rendering and the one registered, and only push the difference. I'm assuming here there is a way to get the current stack and perform this diff also (this could be a wrong assumption).

edit: ideally actually, over point 2, you could just register the full hased stack ID and then "set" the ID to that value at render time: but afaik there's no functionality to do this, nor do I think this is an intended use case of the stack ID

edit edit: I went digging for more, found this lengthy discussion https://github.com/ocornut/imgui/issues/331. This covers this potential problem in great detail so I suggest anyone who comes here to just go read that. The TLDR answer I found is to just update my lambda function as:

RegisterForDeferredRender([](const char *title, const char *msg){
        if (!ImGui::IsPopupOpen("file upload error"))
            ImGui::OpenPopup("file upload error");
        int ret = ErrorModal(title, msg);
        if (ret == 1){
        std::cout << "they clicked ok" << std::endl;
        }
        else if (ret == 2)
        {
        std::cout << "they clicked report" << std::endl;
        }
        return ret;
        }, "file upload error", "err");

now it seems there's still no official solution for this popup-stack-id-paradigm but there are some solutions discussed there that you can use.

ocornut commented 6 months ago

Maybe I am missing something I am confused about why everything posted is so over-engineered. You'll need to store the modal data/message if you want to call it in more outer parts of the code. Mostly the difficultly is that you probably want the result/output of the modal to be processed nearby the code which opened it so you have incentive to keep it mostly local anyhow.

// HELPERS
struct MyModalData
{
   ImGuiID id;
   Str title;
   Str message;
   bool newly_opened = true;
   int result = -1;
};

bool OpenMyModal(ImGuiID id, Str title, Str message)
{
    my_modals.push_back({ modal_id, "Some error", "oops" });
}
MyModalData* BeginMyModal(ImGuiID id)
{
   if (my_modals.empty())
      return NULL;
   MyModalData& modal = &my_modals.back();
   if (modal->id != id)
       return NULL;
    if (BeginPopupModal(modal->title))
    {
        // display modal, handle interaction
    }
    // on modal closure or interaction pop_back() from list
    return modal;
}

// USAGE
ImGuiID error_modal_id = GetID("error_modal");
// some inner loop
{
   if (error)
     OpenMyModal(error_modal_id, "title", "message",etc..);
}

// outside of loop but in a rather local location
if (MyModalData* modal = BeginPopupModal(error_modal_id))
    if (modal->result != -1)
        // handle result
MathieuTuli commented 6 months ago

Over engineered slightly perhaps, it was just a v0, but still necessary in the most part I think. Ultimately it's just an abstraction of the solution you posted (albeit not as elegant in some parts), allowing for more dynamic modals/modal arguments and enabling simultaneous modals to be shown (if that's desired). I can also process the result/output of the modal directly where I defined it using the lambda example I showed, allowing for the localization you mentioned.

I was running under the assumption that I would need such abstraction moving forward, which may be solving a problem that doesn't exist, however I was still curious regardless of if I make full use of this abstraction or not.