From 42d8370d0cc749194922e461c145bc2cd1cc0de4 Mon Sep 17 00:00:00 2001
From: comp500 <comp500@users.noreply.github.com>
Date: Fri, 10 Mar 2023 16:35:34 +0000
Subject: [PATCH] Latest version fixes: correctly order preferences (fixes
 #198)

Both CurseForge and Modrinth preferences were not being done in a specific order - so
a file with a newer Minecraft version would not necessarily take priority over a file
with a more preferred loader (e.g. Quilt over Fabric) or a file with a newer release
date / CurseForge ID.
Also fixed a loop variable reference in the CurseForge loop, which caused eagerly resolved
(included in API response) file info to be inconsistent with the chosen version, and
added filtering for duplicate values in the acceptable-game-versions/primary version list
to ensure game versions are always compared properly (so the same index == same version).
---
 core/pack.go             | 12 +++++++++++-
 curseforge/curseforge.go | 40 ++++++++++++++++++++++++++++++++--------
 modrinth/modrinth.go     | 15 ++++++++++-----
 3 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/core/pack.go b/core/pack.go
index aa2047e..c6bffbf 100644
--- a/core/pack.go
+++ b/core/pack.go
@@ -3,6 +3,7 @@ package core
 import (
 	"errors"
 	"fmt"
+	"golang.org/x/exp/slices"
 	"io"
 	"os"
 	"path/filepath"
@@ -171,7 +172,16 @@ func (pack Pack) GetSupportedMCVersions() ([]string, error) {
 		return nil, errors.New("no minecraft version specified in modpack")
 	}
 	allVersions := append(append([]string(nil), viper.GetStringSlice("acceptable-game-versions")...), mcVersion)
-	return allVersions, nil
+	// Deduplicate values
+	allVersionsDeduped := []string(nil)
+	for i, v := range allVersions {
+		// If another copy of this value exists past this point in the array, don't insert
+		// (i.e. prefer a later copy over an earlier copy, so the main version is last)
+		if !slices.Contains(allVersions[i+1:], v) {
+			allVersionsDeduped = append(allVersionsDeduped, v)
+		}
+	}
+	return allVersionsDeduped, nil
 }
 
 func (pack Pack) GetPackName() string {
diff --git a/curseforge/curseforge.go b/curseforge/curseforge.go
index 11eeead..8630e42 100644
--- a/curseforge/curseforge.go
+++ b/curseforge/curseforge.go
@@ -297,24 +297,48 @@ func findLatestFile(modInfoData modInfo, mcVersions []string, packLoaders []stri
 	for _, v := range modInfoData.LatestFiles {
 		mcVerIdx := core.HighestSliceIndex(mcVersions, v.GameVersions)
 		loaderIdx, loaderValid := filterFileInfoLoaderIndex(packLoaders, v)
-		// Choose "newest" version by largest ID
-		// Prefer higher indexes of mcVersions
-		if mcVerIdx > -1 && loaderValid && (mcVerIdx > bestMcVer || loaderIdx > bestLoaderType || v.ID > fileID) {
+
+		if mcVerIdx < 0 || !loaderValid {
+			continue
+		}
+		// Compare first by Minecraft version (prefer higher indexes of mcVersions)
+		compare := int32(mcVerIdx - bestMcVer)
+		if compare == 0 {
+			// Prefer higher loader indexes
+			compare = int32(loaderIdx - bestLoaderType)
+		}
+		if compare == 0 {
+			// Other comparisons are equal, compare by ID instead
+			compare = int32(v.ID - fileID)
+		}
+		if compare > 0 {
 			fileID = v.ID
-			fileInfoData = &v
+			fileInfoDataCopy := v // Fix for loop variable reference (which gets reassigned on every iteration!)
+			fileInfoData = &fileInfoDataCopy
 			fileName = v.FileName
 			bestMcVer = mcVerIdx
 			bestLoaderType = loaderIdx
 		}
 	}
-	// TODO: change to timestamp-based comparison??
 	// TODO: manage alpha/beta/release correctly, check update channel?
 	for _, v := range modInfoData.GameVersionLatestFiles {
 		mcVerIdx := slices.Index(cfMcVersions, v.GameVersion)
 		loaderIdx, loaderValid := filterLoaderTypeIndex(packLoaders, v.Modloader)
-		// Choose "newest" version by largest ID
-		// Prefer higher indexes of mcVersions
-		if mcVerIdx > -1 && loaderValid && (mcVerIdx > bestMcVer || loaderIdx > bestLoaderType || v.ID > fileID) {
+
+		if mcVerIdx < 0 || !loaderValid {
+			continue
+		}
+		// Compare first by Minecraft version (prefer higher indexes of mcVersions)
+		compare := int32(mcVerIdx - bestMcVer)
+		if compare == 0 {
+			// Prefer higher loader indexes
+			compare = int32(loaderIdx - bestLoaderType)
+		}
+		if compare == 0 {
+			// Other comparisons are equal, compare by ID instead
+			compare = int32(v.ID - fileID)
+		}
+		if compare > 0 {
 			fileID = v.ID
 			fileInfoData = nil // (no file info in GameVersionLatestFiles)
 			fileName = v.Name
diff --git a/modrinth/modrinth.go b/modrinth/modrinth.go
index 2806d0e..0e310dd 100644
--- a/modrinth/modrinth.go
+++ b/modrinth/modrinth.go
@@ -242,11 +242,16 @@ func findLatestVersion(versions []*modrinthApi.Version, gameVersions []string, u
 		}
 
 		if compare == 0 {
-			if loaderIdx < latestValidLoaderIdx { // Prefer loaders; principally Quilt over Fabric, mods over datapacks (Modrinth backend handles filtering)
-				compare = 1
-			} else if gameVersionIdx > bestGameVersion { // Prefer later specified game versions (main version specified last)
-				compare = 1
-			} else if v.DatePublished.After(*latestValidVersion.DatePublished) { // FlexVer comparison is equal or disabled, compare date instead
+			// Prefer later specified game versions (main version specified last)
+			compare = int32(gameVersionIdx - bestGameVersion)
+		}
+		if compare == 0 {
+			// Prefer loaders; principally Quilt over Fabric, mods over datapacks (Modrinth backend handles filtering)
+			compare = int32(latestValidLoaderIdx - loaderIdx)
+		}
+		if compare == 0 {
+			// Other comparisons are equal, compare date instead
+			if v.DatePublished.After(*latestValidVersion.DatePublished) {
 				compare = 1
 			}
 		}