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

Update libfmt, fix compilation warnings and invalid JSON CustomSystem position #5759

Merged
merged 11 commits into from
Feb 15, 2024

Conversation

Web-eWorks
Copy link
Member

Been sitting on this PR for longer than I intended, life got busy.

Essentially does what it says on the tin, updates libfmt to v10.2.1 (fixes #5693) and fixes a few sets of compilation warnings while in the area.

I've found and fixed a bug that was causing JSON custom systems to lose their position values (more correctly, the position was initialized from the sector index instead, which could cause all kinds of bugs) - it's benign at the moment since we don't ship any JSON custom systems other than Sol with a {0,0,0} sector index and position.

This also bumps our CI build version to 20240203-dev following the schema we've set up - this PR was intended to be opened immediately after the release / that commit pushed directly to master, but my attention has been unfortunately elsewhere.

- No non-void-pointer automatic formatting
- const unsigned char* is not implicitly convertible to const char*
- Enums have to be manually cast to int or a formatter provided for them
- Because UndoClosure overrides the Swap() virtual method, Swap() cannot be called inside the constructor
- Instead, call it in the helper function which pushed the undo step
- Address unused variable, integer signedness comparison, and initialization order issues.
- %b is not a format specifier recognized by fmt
- Use Log::Info instead of the printf overloads
- core.h is the "lightweight" API for libfmt
- Other than printf.h, we don't use any of format.h's features
- Missing transitive includes after fmt upgrade
- GCC 14 doesn't pull in <algorithm> for Input.h anymore
- Validates that the compiler correctly obeys the C++17 standard regarding sequence points in a braced-init-list
- Ensures multiple-push / multiple-pull functionality works correctly
@Web-eWorks
Copy link
Member Author

This PR also introduces a new test case addressing the -Wsequence-point warning GCC provides, validating that all compilers we build and test against properly implement the C++17 standard's sequencing rules for initializer clauses in a braced-init-list (which we depend on for pi_lua_multiple_pull).

@Web-eWorks Web-eWorks merged commit a3339aa into pioneerspacesim:master Feb 15, 2024
5 checks passed
@Web-eWorks Web-eWorks deleted the new-year-new-deps branch February 17, 2024 04:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should we update fmt?
1 participant