zep
zep copied to clipboard
Potential problems with Zep::ZepTheme::GetColor() API change
Describe the bug
When integrating a more recent version of Zep, I was hit by a bug where my whole editor went black. This did not happen when compiling in debug mode, which made it a bit tricky to track down. I finally realised it was because Zep::ZepTheme::GetColor
now returned a Zep::NVec4f const &
(as opposed to a Zep::NVec4f
previously), so when overriding that method I had to make sure I did not return a reference to a temporary because the compiler was (rightfully) assuming I did not care about the returned value.
I studied bf53e9cb9563d3d67d555bf32bf2a4c1d8f993d2 and could not understand a reason behind returning a reference. From what I found so far, the returned value is always eventually copied to a Zep::Nvec4f
.
This is of course a very common mistake that every C++ programmer should avoid, but it is easily overlooked and I realise that the copiler did not warn me about it.
Also there is currently a real problem in ZepTheme
due to that: since SetLightTheme()
allocates more colors than SetDarkTheme()
, any value returned by GetColor
may be invalidated by a call to SetLightTheme()
.
My personal issue is solved but I’m still suggesting reverting the API change to avoid surprises for other users.
OK, that makes sense; I was really just trying to save time here, since GetColor is called all the time during display. I didn't intend for the color to be kept around for very long; so didn't think about the Theme stuff; I'll revert it. I still have work to do to make the themes cleaner.
I've decided to leave the const& return; it saves a lot of copying around of data when drawing the window. I have also fixed the mismatch in style counts.