loot / loot.github.io

The website and meta issue tracker.
https://loot.github.io
GNU General Public License v3.0
12 stars 9 forks source link

Adjust or clarify how LOOT handles versions #97

Closed sibir-ine closed 2 years ago

sibir-ine commented 2 years ago

LOOT's extended Semantic Versioning scheme for version parsing & comparison isn't completely backward-compatible with SemVer v2.0.0. Most ways are documented under the version() condition function in the Functions section of the libloot docs (i.e., case-insensitivity & a wider range of separators). However, we haven't explicitly documented the ways these extensions break compatibility. For one, case-insensitivity breaks precedence rules as non-numeric characters follow ASCII sort order in SemVer. The parsing of hyphens as pre-release separators also breaks precedence rules as authors can use them in pre-release identifiers.

LOOT also breaks from SemVer in at least one way that we haven't documented. Specifically, LOOT treats pre-release versions as having higher precedence than their non–pre-release counterparts (e.g., 1.2.3 < 1.2.3-alpha). I agree that this should be the case for non–SemVer-compatible versions (e.g., 2.1.3alpha, 2.1.3.alpha) as I've often seen a pre-release version indicating higher precedence than an equivalent version without a pre-release identifier in these cases. However, if a mod author is adhering to SemVer (i.e., X.Y.Z-pre-release), this could confuse them.

Therefore, I think we should either adjust LOOT's version handling to be compatible with SemVer or clarify that we use a modified version of SemVer that isn't backward-compatible & exactly how.

For adjusting LOOT's version handling, we could accomplish this by splitting LOOT's version handling into two regimes: one that strictly adheres to SemVer & one that adheres to LOOT's modified, SemVer-based scheme. If the format of a version is consistent with SemVer, LOOT should parse it as a SemVer version. If it doesn't, LOOT should parse it using our current scheme.

When comparing two SemVer versions, precedence should follow SemVer's precedence rules (including case-sensitivity & hyphens as valid pre-release identifiers & not separators). When comparing two non-SemVer versions with our current scheme, LOOT can continue to evaluate them as it currently does. When comparing a SemVer version & a non-SemVer version, LOOT can do the same. The only non-trivial case I can think of where this could lead to undesirable results is when a mod author releases a pre-release version of their mod, but we use a non–SemVer-compatible version for comparison. I believe we've been careful to match the version format used by the mod author when writing conditions, so I don't think this will be an issue.

Whether we adjust LOOT's version handling or not, we should still explicitly detail how LOOT parses versions & evaluates version comparisons as it would benefit mod authors interested in following a versioning scheme compatible with LOOT. Our current explanation may lead them to believe that they can follow SemVer in ways that LOOT won't handle per SemVer's specs. Besides the previously mentioned differences & exactly which other separator characters LOOT supports, are there other aspects of LOOT's version handling that we can document more explicitly?

Ortham commented 2 years ago

We can't strictly adhere to semver because the existing behaviour was built in response to real-world versions not using semver, and we probably can't switch the comparison behaviour between semver-compatible and non-semver-compatible versions because semver is about conforming semantically as well as syntactically, and we don't know the intended semantics just by looking at the version string. That's why LOOT assumes a versioning scheme that's broader than semver.

The case of 1.2.3 < 1.2.3-alpha looks like an oversight, I don't see a test case comparing versions with and without pre-release IDs.

AFAIK case-insensitivity is the only other case in which LOOT's comparisons are not semver-compatible, in the sense that the behaviour contradicts the spec rather than just being more permissive. Unfortunately it's also one area where the current behaviour does matter, as I've seen cases like where something like 1.2.3-A is expected to come after something like 1.2.3-beta.

If you've got any other examples of comparisons that you think LOOT gets wrong, I can check if they're intended or oversights, but I think improving the docs would be good, especially giving specific examples of when LOOT's behaviour differs from semver.

sibir-ine commented 2 years ago

If you've got any other examples of comparisons that you think LOOT gets wrong, I can check if they're intended or oversights, but I think improving the docs would be good, especially giving specific examples of when LOOT's behaviour differs from semver.

I can't think of anything else.

Ortham commented 2 years ago

The pre-release ID comparison oversight has been fixed and the docs improved as of loot/libloot@3d8a6453a4fcc8f4286bb62a6ac35ea7896d2d8f. The docs can be seen here.

@sibir-ine Do the updated docs cover everything you want?

sibir-ine commented 2 years ago

Yes, they look perfect, thanks! As a side note, it doesn't look like the Discord webhook is configured for loot-condition-interpreter.

Ortham commented 2 years ago

OK, closing this then. I've also added that missing webhook.