Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libfmt patch #8

Closed
azihassan opened this issue Oct 2, 2024 · 7 comments
Closed

libfmt patch #8

azihassan opened this issue Oct 2, 2024 · 7 comments

Comments

@azihassan
Copy link
Owner

azihassan commented Oct 2, 2024

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
Copy link

glebm commented Oct 3, 2024

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
Copy link
Owner Author

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
Copy link
Owner Author

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
Copy link

glebm commented Oct 4, 2024

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

@azihassan
Copy link
Owner Author

azihassan commented Oct 4, 2024

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
Copy link

glebm commented Oct 4, 2024

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
Copy link
Owner Author

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

@azihassan azihassan moved this from In progress to Done in Devilutionx Dreamcast port Oct 7, 2024
@azihassan azihassan closed this as completed by moving to Done in Devilutionx Dreamcast port Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants