Blake-Madden / Wisteria-Dataviz

Wisteria Dataviz Library
BSD 3-Clause "New" or "Revised" License
14 stars 3 forks source link

Library fails to build with wxWidgets built with WXWIN_COMPATIBILITY_3_0 set to 0 #12

Closed PBfordev closed 2 years ago

PBfordev commented 2 years ago

When building the library using wxWidgets built with WXWIN_COMPATIBILITY_3_0 set to 0, this code fails to compile:

https://github.com/Blake-Madden/Wisteria-Dataviz/blob/28a670ba46bb9b61a6bbcc29b6120e98ff2638b3/src/util/logfile.cpp#L113-L153

Error C2039 'timestamp': is not a member of 'wxLogRecordInfo' Wisteria D:\dev\Desktop\Lib\Wisteria-Dataviz-GIT\src\util\logfile.cpp 115

It is because the deprecated wxLogRecordInfo::timestamp is not available anymore: https://github.com/wxWidgets/wxWidgets/blob/17371d911d9ec68b0c2d98c7f56977523803b392/include/wx/log.h#L151-L153

It is probably simple to fix by just using wxLogRecordInfo::timestampMS divided by 1000, but it could be better to not duplicate wxLogFormatter code and use it (perhaps customized) instead.

BTW, is LogFile::DoLogTextAtLevel() ever called? In wxLog classes it is usually called from DoLogRecord() but you do not call it from there nor call the base method. But I may be missing something.

Blake-Madden commented 2 years ago

Thanks, I changed it to use timestampMS, wxDateTime excepts that no problem.

I don't think DoLogTextAtLevel() is called, but it's part of the virtual API for some reason (or at least it used to be). I'll re-investigate that...

PBfordev commented 2 years ago

Fixed by Fix wx 3.2 compile error

Blake-Madden commented 2 years ago

OK, so DoLogTextAtLevel() is part of the base virtual API. Not sure when the framework calls it, but I suppose I have to provide an override for it just in case.

PBfordev commented 2 years ago

Not sure when the framework calls it.

As I wrote before, I think it never does outside from the base DoLogRecord(), which you do not call. See also the method descriptions at https://github.com/wxWidgets/wxWidgets/blob/351b76ace03a7923e2e761e985a5d3b0a5f3f877/include/wx/log.h#L565

Either way, it's not that your override is harmful, just unused.