epezent / implot

Immediate Mode Plotting
MIT License
4.7k stars 521 forks source link

Misuse of IM_ASSERT_USER_ERROR #593

Open ocornut opened 1 week ago

ocornut commented 1 week ago

IM_ASSERT_USER_ERROR() was designed to specifically cover errors that are safely recoverable. So e.g.

IM_ASSERT_USER_ERROR(gp.CurrentPlot != nullptr && !gp.CurrentPlot->SetupLocked,
                     "Setup needs to be called after BeginPlot and before any setup locking functions (e.g. PlotX)!");    // get plot and axis
ImPlotPlot& plot = *gp.CurrentPlot;

The gp.CurrentPlot != nullptr part of it is incorrect.

I suggest not to take action right now as I am in the process of reworking this system to provide new error handling mechanisms. I will try to post some update here eventually, but felt it was safer to leave the info down here.

ocornut commented 4 days ago

Pushed better error recovery support to imgui master: https://github.com/ocornut/imgui/issues/1651#issuecomment-2379728579

Calling IM_ASSERT_USER_ERROR() followed by a crash (e.g. in the case gp.CurrentPlot == NULL in my example above) is a little misleading, but not more damaging than an assert. Ideally the idea is that IM_ASSERT_USER_ERROR() should be in path where your code won't crash. I recommend only using it in entry points. Aka don't spam codebase with random calls to this in all leaf functions, but be considerate of what can the user errors cases be.

ImGui::Begin("....");
[00001] [imgui-error] (current settings: Assert=1, Log=1, Tooltip=1)
[00001] [imgui-error] In window 'Oops!': Missing End()
Assertion failed: (0) && "Missing End()", file c:\omar\work\imgui\imgui.cpp, line 10501

image

image

I started working on a new https://github.com/ocornut/imgui/wiki/Error-Handling page too.