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
Fix #9386: use variant instead of unique_ptr to prevent compilers failing on the code generation #9391
Conversation
Don't forget to update |
Rename the zero-parameter NetworkValidateClientName to NetworkValidateOurClientName to make it clearer it is performed on our client name, and to make it a non-overloaded function to aid with the variant being added a few commits later
One UpdateServiceInterval has two parameters to update the service interval for a vehicle type, the other for all vehicle types at once. Rename the latter to help with function resolution for the introduction of variants.
… the saveload macros This to help variant's constructor to be able to resolve the constructor of the setting
… actual SettingDesc
…gDesc constructor This as using std::string causes much more variables to be tracked, potentially causing problemes for certain compilers in certain situations
…ants instead of new + unique_ptr With std::variant all memory can be figured out at compile time, so the compiler needs to keep track of fewer elements. It also saves out a unique_ptr and its memory management, over a slight impact for resolving a setting.
Reminder we shouldn't forget to update https://github.com/OpenTTD/website/blob/master/_data/download-descriptions.yml if this lands. |
I couldn't tell you how or why your CI builds work, however on my local machine running OSX 10.14 this breaks the build as
|
Minimum target has been changed back to 10.13 in #10253. |
Right, but builds fail with |
Maybe it depends on Xcode version, CI currently uses Xcode 14.2 and MacOSX13.1 SDK |
Yeah I'm building (trying to) on OSX 10.14.6 with whatever the newest XCode is that I could get away with (11.3.1). |
Motivation / Problem
In the words of TB:
Description
Fixes #9386 by using
std::variant
to forego the need to generate code for anew
and as such needing to bother withdelete
at a later stage.The forwarding constructor of
std::variant
is quite picky (at least on MSVC). It does not like to resolve overloaded functions, so some functions needed to be renamed to be resolvable. Furthermore the 'naked' initializer forSaveLoad
was causing issues at some places, so effectively 'name' it by addingSaveLoad
to the SLE* macros.Finally resolving from a variant to a type is not super trivial when you do not know the exact type, for this
std::visit
is used which helps but including that in a simpleiterator
is beyond my C++-foo, so fell back to a helper function to convert the type of the 'auto loop' to the needed/wantedconst SettingDesc *
.Limitations
std::visit
requires Mac OS 10.14 or higher which has been out-of-support for half a year... and that causes deprecations warnings that I can't solve.Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.