search

azihassan / devilutionX

Diablo build for modern operating systems
Other
4 stars 1 forks source link

libfmt patch #8

Closed azihassan closed 4 days ago

azihassan commented 1 week ago

To get the game to compile, I had to patch the libfmt source code to add this line. A cleaner approach should be used for this.

glebm commented 1 week ago

What was the compilation error? That struct in libfmt is for extended precision support, which is optional and shouldn't cause an error even if not supported.

azihassan commented 1 week ago

Thanks! It's been a while so I don't remember the specifics, but here's a reproduction: https://github.com/azihassan/devilutionX/actions/runs/11171993466/job/31057721326

azihassan commented 1 week ago

Looks like it's possible to disable double support in libfmt by setting a FMT_USE_DOUBLE macro to 0. I'll look into how to do it with cmake

glebm commented 1 week ago

Here is an example https://github.com/diasurgical/devilutionX/blob/1a32a705fe230932046e6c1c8058af3121ae3e6f/3rdParty/libfmt/CMakeLists.txt#L25

azihassan commented 6 days ago

That helps thanks! After a lot of trial and error, I discovered that those flags aren't supported anymore. The proposed alternative didn't have a noticeable performance impact on Flycast, but it did require that I bump libfmt to last week's commit which might break compilation for the other platforms. I'll let github actions test that possibility

Edit: yikes. Completely forgot about the linter

glebm commented 6 days ago

Looks like FMT_BUILTIN_TYPES=0 should work instead (with the latest libfmt). By the way we've recently removed all the usages of double from DevilutionX (didn't really need it).

azihassan commented 4 days ago

Agreed, that's the proposed alternative I mentioned having tested on Flycast. Not on console yet though

By the way we've recently removed all the usages of double from DevilutionX (didn't really need it).

Nice, I updated the fork to fetch those changes