search

novelrt / NovelRT

A cross-platform 2D game engine accompanied by a strong toolset for visual novels.
MIT License
184 stars 42 forks source link

Prepare NovelRT for C++ 20 #500

Open Pheubel opened 2 years ago

Pheubel commented 2 years ago

Although NovelRT compiles for C++ 17 on both Clang and MSVC, there are some obstacles that prevent the engine from moving forwards to C++ 20.

Status resolved

Fabulist

Fabulist currently fails to compile in C++ 20 due to a change in how std:accumulate() works. Relevant issue: https://github.com/novelrt/Fabulist/issues/14

NovelRT/LoggingService

Due to how C++ 20 behaves, the interaction with spdlog and fmt becomes a bit more rough. The following lines are affected: https://github.com/novelrt/NovelRT/blob/2559c00a896c77759f062d01143c3fe8805c2db8/include/NovelRT/LoggingService.h#L61-L76

One way to get around this issue is by wrapping the argument we want to pass in fmt::runtime(). Here is an example with logInfo():

template<typename I, typename... IRest> void logInfo(I current, IRest... next) const
{
    _logger->info(fmt::runtime(current), std::forward<IRest>(next)...);
}

Another option that can be explored is to have spdlog use C++'s std::format() by setting SPDLOG_USE_STD_FORMAT, it is to be noted that in the current release of spdlog, it is currently marked as experimental.

NovelRT/Graphics/IGraphicsMemoryRegionCollection

Right now something strange is going on within IGraphicsMemoryRegionCollection::DefaultMetadata::Clear(). with both clang and MSVC it currently compiles just fine on C++ 17, however when switching to C++ 20 MSVC will have issues with GetDevice(), it is no longer able to access it as expected. You will find that during building it will say a call of a non-static member function requires an object.

https://github.com/novelrt/NovelRT/blob/2559c00a896c77759f062d01143c3fe8805c2db8/include/NovelRT/Graphics/IGraphicsMemoryRegionCollection.h#L221-L237

It seems like a compiler bug and we will most likely have to wait until it is fixed unless we want to use the work around mentioned above.

After talk in the discord it seems like a template resolution issue and might be able to be worked around.

JsonCons

JsonCons seems to be causing trouble when compiling with C++ 20. After a bit of talk in Discord, it seems that a good option would be to replace it with nlohmann json, as it is already being used for Fabulist and seems to compile just fine for C++ 20.

With that out of the way, these are the current pain points I've found so far. I am not sure if this list is complete, but after applying the changes for Fabulist and NovelRT/LoggingService, I was able to build the project with C++ 20 with clang on Linux.

Pheubel commented 1 year ago

spdlog came out with a new release a couple of days ago, with it it should be able to get rid of the compilation error with the logger.

after a quick check, the original solution seems best for now.

RubyNova commented 1 year ago

I think we need to figure some of this out a little more.

Since this seems like it'd break our steam runtime linux target, I think we need a plan going forward.

There's definitely more high prio stuff we could be doing as well to be fair.

Thoughts are appreciated.