loot / libloot

A C++ library for accessing LOOT's metadata and sorting functionality.
GNU General Public License v3.0
32 stars 12 forks source link

Represent optional scalar metadata fields with std::optional #30

Closed Ortham closed 5 years ago

Ortham commented 6 years ago

This is a more general take on #14. There are a number of scalar metadata fields that are optional, and it can currently be tricky to distinguish unspecified values from specified defaults. For example, PluginMetadata's group field defaults to default, so it's impossible to tell if user metadata that sets a value of default is overriding whatever the masterlist metadata is or has omitted the field, without comparing against the masterlist, which shouldn't be necessary. Diffing metadata is also more complicated.

One solution would be to use the C++17 std::optional<T> type to represent optional scalar fields, more accurately reflecting the metadata format in code. This does introduce large breaking API changes and requires a C++ compiler and runtime update, but there are other C++17 features that make updating attractive, and the result would be a more semantic API.

The metadata structure fields that are optional scalars are:

Ortham commented 6 years ago

After experimenting with using optionals, I've decided to only use them for PluginMetadata groups. For all other optional scalars they make the code more complex and don't actually bring any benefit: group values are the only ones involved in metadata merging, so there's never any need to distinguish between unspecified and specified default for the other fields' values.

Ortham commented 6 years ago

This is done in the breaking-changes branch.

Ortham commented 5 years ago

Done as of 403bf3467e58ad246fcc0f6b5b43ca4bf782cf0f.