Prefer game versions according to acceptable versions list (fixes #181)

The acceptable versions list should now be specified in order of preference, where the last version is the most preferable Minecraft version

cmd/update.go

@@ -30,11 +30,6 @@ var updateCmd = &cobra.Command{
 			fmt.Println(err)
 			os.Exit(1)
 		}
-		mcVersion, err := pack.GetMCVersion()
-		if err != nil {
-			fmt.Println(err)
-			os.Exit(1)
-		}
 
 		var singleUpdatedName string
 		if viper.GetBool("update.all") {
@@ -70,7 +65,7 @@ var updateCmd = &cobra.Command{
 			updaterPointerMap := make(map[string][]*core.Mod)
 			updaterCachedStateMap := make(map[string][]interface{})
 			for k, v := range updaterMap {
-				checks, err := core.Updaters[k].CheckUpdate(v, mcVersion, pack)
+				checks, err := core.Updaters[k].CheckUpdate(v, pack)
 				if err != nil {
 					// TODO: do we return err code 1?
 					fmt.Printf("Failed to check updates for %s: %s\n", k, err.Error())
@@ -148,7 +143,7 @@ var updateCmd = &cobra.Command{
 				}
 				updaterFound = true
 
-				check, err := updater.CheckUpdate([]core.Mod{modData}, mcVersion, pack)
+				check, err := updater.CheckUpdate([]core.Mod{modData}, pack)
 				if err != nil {
 					fmt.Println(err)
 					os.Exit(1)

core/interfaces.go

@@ -10,9 +10,9 @@ type Updater interface {
 	// ParseUpdate takes an unparsed interface{} (as a map[string]interface{}), and returns an Updater for a mod file.
 	// This can be done using the mapstructure library or your own parsing methods.
 	ParseUpdate(map[string]interface{}) (interface{}, error)
-	// CheckUpdate checks whether there is an update for each of the mods in the given slice, for the given MC version,
+	// CheckUpdate checks whether there is an update for each of the mods in the given slice,
 	// called for all of the mods that this updater handles
-	CheckUpdate([]Mod, string, Pack) ([]UpdateCheck, error)
+	CheckUpdate([]Mod, Pack) ([]UpdateCheck, error)
 	// DoUpdate carries out the update previously queried in CheckUpdate, on each Mod's metadata,
 	// given pointers to Mods and the value of CachedState for each mod
 	DoUpdate([]*Mod, []interface{}) error

core/pack.go

@@ -164,6 +164,16 @@ func (pack Pack) GetMCVersion() (string, error) {
 	return mcVersion, nil
 }
 
+// GetSupportedMCVersions gets the versions of Minecraft this pack allows in downloaded mods, ordered by preference (highest = most desirable)
+func (pack Pack) GetSupportedMCVersions() ([]string, error) {
+	mcVersion, ok := pack.Versions["minecraft"]
+	if !ok {
+		return nil, errors.New("no minecraft version specified in modpack")
+	}
+	allVersions := append(append([]string(nil), viper.GetStringSlice("acceptable-game-versions")...), mcVersion)
+	return allVersions, nil
+}
+
 func (pack Pack) GetPackName() string {
 	if pack.Name == "" {
 		return "export"

core/versionutil.go

@@ -145,3 +145,16 @@ func ComponentToFriendlyName(component string) string {
 		return component
 	}
 }
+
+// HighestSliceIndex returns the highest index of the given values in the slice (-1 if no value is found in the slice)
+func HighestSliceIndex(slice []string, values []string) int {
+	highest := -1
+	for _, val := range values {
+		for i, v := range slice {
+			if v == val && i > highest {
+				highest = i
+			}
+		}
+	}
+	return highest
+}

curseforge/curseforge.go

@@ -110,6 +110,14 @@ func getCurseforgeVersion(mcVersion string) string {
 	return mcVersion
 }
 
+func getCurseforgeVersions(mcVersions []string) []string {
+	out := make([]string, len(mcVersions))
+	for i, v := range mcVersions {
+		out[i] = getCurseforgeVersion(v)
+	}
+	return out
+}
+
 var urlRegexes = [...]*regexp.Regexp{
 	regexp.MustCompile("^https?://(?P<game>minecraft)\\.curseforge\\.com/projects/(?P<slug>[^/]+)(?:/(?:files|download)/(?P<fileID>\\d+))?"),
 	regexp.MustCompile("^https?://(?:www\\.|beta\\.)?curseforge\\.com/(?P<game>[^/]+)/(?P<category>[^/]+)/(?P<slug>[^/]+)(?:/(?:files|download)/(?P<fileID>\\d+))?"),
@@ -262,32 +270,37 @@ func matchLoaderTypeFileInfo(packLoaders []string, fileInfoData modFileInfo) boo
 	}
 }
 
-func matchGameVersion(mcVersion string, modMcVersion string) bool {
+// findLatestFile looks at mod info, and finds the latest file ID (and potentially the file info for it - may be null)
-	if getCurseforgeVersion(mcVersion) == modMcVersion {
+func findLatestFile(modInfoData modInfo, mcVersions []string, packLoaders []string) (fileID uint32, fileInfoData *modFileInfo, fileName string) {
-		return true
+	cfMcVersions := getCurseforgeVersions(mcVersions)
-	} else {
+	bestMcVer := -1
-		for _, v := range viper.GetStringSlice("acceptable-game-versions") {
-			if getCurseforgeVersion(v) == modMcVersion {
-				return true
-			}
-		}
-		return false
-	}
-}
 
-func matchGameVersions(mcVersion string, modMcVersions []string) bool {
+	// For snapshots, curseforge doesn't put them in GameVersionLatestFiles
-	for _, modMcVersion := range modMcVersions {
+	for _, v := range modInfoData.LatestFiles {
-		if getCurseforgeVersion(mcVersion) == modMcVersion {
+		mcVerIdx := core.HighestSliceIndex(mcVersions, v.GameVersions)
-			return true
+		// Choose "newest" version by largest ID
-		} else {
+		// Prefer higher indexes of mcVersions
-			for _, v := range viper.GetStringSlice("acceptable-game-versions") {
+		if mcVerIdx > -1 && matchLoaderTypeFileInfo(packLoaders, v) && (mcVerIdx >= bestMcVer || v.ID > fileID) {
-				if getCurseforgeVersion(v) == modMcVersion {
+			fileID = v.ID
-					return true
+			fileInfoData = &v
+			fileName = v.FileName
+			bestMcVer = mcVerIdx
 		}
 	}
+	// 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)
+		// Choose "newest" version by largest ID
+		// Prefer higher indexes of mcVersions
+		if mcVerIdx > -1 && matchLoaderType(packLoaders, v.Modloader) && (mcVerIdx >= bestMcVer || v.ID > fileID) {
+			fileID = v.ID
+			fileInfoData = nil // (no file info in GameVersionLatestFiles)
+			fileName = v.Name
+			bestMcVer = mcVerIdx
 		}
 	}
-	return false
+	return
 }
 
 type cfUpdateData struct {
@@ -311,16 +324,20 @@ func (u cfUpdater) ParseUpdate(updateUnparsed map[string]interface{}) (interface
 
 type cachedStateStore struct {
 	modInfo
-	hasFileInfo bool
 	fileID   uint32
-	fileInfo    modFileInfo
+	fileInfo *modFileInfo
 }
 
-func (u cfUpdater) CheckUpdate(mods []core.Mod, mcVersion string, pack core.Pack) ([]core.UpdateCheck, error) {
+func (u cfUpdater) CheckUpdate(mods []core.Mod, pack core.Pack) ([]core.UpdateCheck, error) {
 	results := make([]core.UpdateCheck, len(mods))
 	modIDs := make([]uint32, len(mods))
 	modInfos := make([]modInfo, len(mods))
 
+	mcVersions, err := pack.GetSupportedMCVersions()
+	if err != nil {
+		return nil, err
+	}
+
 	for i, v := range mods {
 		projectRaw, ok := v.GetParsedUpdateData("curseforge")
 		if !ok {
@@ -354,55 +371,18 @@ func (u cfUpdater) CheckUpdate(mods []core.Mod, mcVersion string, pack core.Pack
 		}
 		project := projectRaw.(cfUpdateData)
 
-		updateAvailable := false
+		fileID, fileInfoData, fileName := findLatestFile(modInfos[i], mcVersions, packLoaders)
-		fileID := project.FileID
+		if fileID > project.FileID && fileID != 0 {
-		fileInfoObtained := false
+			// Update available!
-		var fileInfoData modFileInfo
-		var fileName string
-
-		// For snapshots, curseforge doesn't put them in GameVersionLatestFiles
-		for _, v := range modInfos[i].LatestFiles {
-			// Choose "newest" version by largest ID
-			if matchGameVersions(mcVersion, v.GameVersions) && v.ID > fileID && matchLoaderTypeFileInfo(packLoaders, v) {
-				updateAvailable = true
-				fileID = v.ID
-				fileInfoData = v
-				fileInfoObtained = true
-				fileName = v.FileName
-			}
-		}
-
-		for _, file := range modInfos[i].GameVersionLatestFiles {
-			// TODO: change to timestamp-based comparison??
-			// TODO: manage alpha/beta/release correctly, check update channel?
-			// Choose "newest" version by largest ID
-			if matchGameVersion(mcVersion, file.GameVersion) && file.ID > fileID && matchLoaderType(packLoaders, file.Modloader) {
-				updateAvailable = true
-				fileID = file.ID
-				fileName = file.Name
-				fileInfoObtained = false // Make sure we get the file info again
-			}
-		}
-
-		if !updateAvailable {
-			results[i] = core.UpdateCheck{UpdateAvailable: false}
-			continue
-		}
-
-		// The API also provides some files inline, because that's efficient!
-		if !fileInfoObtained {
-			for _, file := range modInfos[i].LatestFiles {
-				if file.ID == fileID {
-					fileInfoObtained = true
-					fileInfoData = file
-				}
-			}
-		}
-
 			results[i] = core.UpdateCheck{
 				UpdateAvailable: true,
 				UpdateString:    v.FileName + " -> " + fileName,
-			CachedState:     cachedStateStore{modInfos[i], fileInfoObtained, fileID, fileInfoData},
+				CachedState:     cachedStateStore{modInfos[i], fileID, fileInfoData},
+			}
+		} else {
+			// Could not find a file, too old, or up to date: no update available
+			results[i] = core.UpdateCheck{UpdateAvailable: false}
+			continue
 		}
 	}
 	return results, nil
@@ -413,8 +393,10 @@ func (u cfUpdater) DoUpdate(mods []*core.Mod, cachedState []interface{}) error {
 	for i, v := range mods {
 		modState := cachedState[i].(cachedStateStore)
 
-		fileInfoData := modState.fileInfo
+		var fileInfoData modFileInfo
-		if !modState.hasFileInfo {
+		if modState.fileInfo != nil {
+			fileInfoData = *modState.fileInfo
+		} else {
 			var err error
 			fileInfoData, err = cfDefaultClient.getFileInfo(modState.ID, modState.fileID)
 			if err != nil {

curseforge/install.go

@@ -39,7 +39,7 @@ var installCmd = &cobra.Command{
 			fmt.Println(err)
 			os.Exit(1)
 		}
-		mcVersion, err := pack.GetMCVersion()
+		mcVersions, err := pack.GetSupportedMCVersions()
 		if err != nil {
 			fmt.Println(err)
 			os.Exit(1)
@@ -90,9 +90,9 @@ var installCmd = &cobra.Command{
 			var cancelled bool
 			if slug == "" {
 				searchTerm := strings.Join(args, " ")
-				cancelled, modInfoData = searchCurseforgeInternal(searchTerm, false, game, category, mcVersion, getSearchLoaderType(pack))
+				cancelled, modInfoData = searchCurseforgeInternal(searchTerm, false, game, category, mcVersions, getSearchLoaderType(pack))
 			} else {
-				cancelled, modInfoData = searchCurseforgeInternal(slug, true, game, category, mcVersion, getSearchLoaderType(pack))
+				cancelled, modInfoData = searchCurseforgeInternal(slug, true, game, category, mcVersions, getSearchLoaderType(pack))
 			}
 			if cancelled {
 				return
@@ -115,7 +115,7 @@ var installCmd = &cobra.Command{
 		}
 
 		var fileInfoData modFileInfo
-		fileInfoData, err = getLatestFile(modInfoData, mcVersion, fileID, pack.GetLoaders())
+		fileInfoData, err = getLatestFile(modInfoData, mcVersions, fileID, pack.GetLoaders())
 		if err != nil {
 			fmt.Printf("Failed to get file for project: %v\n", err)
 			os.Exit(1)
@@ -182,7 +182,7 @@ var installCmd = &cobra.Command{
 					depIDPendingQueue = depIDPendingQueue[:0]
 
 					for _, currData := range depInfoData {
-						depFileInfo, err := getLatestFile(currData, mcVersion, 0, pack.GetLoaders())
+						depFileInfo, err := getLatestFile(currData, mcVersions, 0, pack.GetLoaders())
 						if err != nil {
 							fmt.Printf("Error retrieving dependency data: %s\n", err.Error())
 							continue
@@ -265,7 +265,7 @@ func (r modResultsList) Len() int {
 	return len(r)
 }
 
-func searchCurseforgeInternal(searchTerm string, isSlug bool, game string, category string, mcVersion string, searchLoaderType modloaderType) (bool, modInfo) {
+func searchCurseforgeInternal(searchTerm string, isSlug bool, game string, category string, mcVersions []string, searchLoaderType modloaderType) (bool, modInfo) {
 	if isSlug {
 		fmt.Println("Looking up CurseForge slug...")
 	} else {
@@ -328,9 +328,9 @@ func searchCurseforgeInternal(searchTerm string, isSlug bool, game string, categ
 	}
 
 	// If there are more than one acceptable version, we shouldn't filter by game version at all (as we can't filter by multiple)
-	filterGameVersion := getCurseforgeVersion(mcVersion)
+	filterGameVersion := ""
-	if len(viper.GetStringSlice("acceptable-game-versions")) > 0 {
+	if len(mcVersions) == 1 {
-		filterGameVersion = ""
+		filterGameVersion = getCurseforgeVersion(mcVersions[0])
 	}
 	var search, slug string
 	if isSlug {
@@ -403,39 +403,18 @@ func searchCurseforgeInternal(searchTerm string, isSlug bool, game string, categ
 	}
 }
 
-func getLatestFile(modInfoData modInfo, mcVersion string, fileID uint32, packLoaders []string) (modFileInfo, error) {
+func getLatestFile(modInfoData modInfo, mcVersions []string, fileID uint32, packLoaders []string) (modFileInfo, error) {
 	if fileID == 0 {
-		var fileInfoData modFileInfo
+		if len(modInfoData.LatestFiles) == 0 && len(modInfoData.GameVersionLatestFiles) == 0 {
-		fileInfoObtained := false
-		anyFileObtained := false
-
-		// For snapshots, curseforge doesn't put them in GameVersionLatestFiles
-		for _, v := range modInfoData.LatestFiles {
-			anyFileObtained = true
-			// Choose "newest" version by largest ID
-			if matchGameVersions(mcVersion, v.GameVersions) && v.ID > fileID && matchLoaderTypeFileInfo(packLoaders, v) {
-				fileID = v.ID
-				fileInfoData = v
-				fileInfoObtained = true
-			}
-		}
-		// TODO: change to timestamp-based comparison??
-		for _, v := range modInfoData.GameVersionLatestFiles {
-			anyFileObtained = true
-			// Choose "newest" version by largest ID
-			if matchGameVersion(mcVersion, v.GameVersion) && v.ID > fileID && matchLoaderType(packLoaders, v.Modloader) {
-				fileID = v.ID
-				fileInfoObtained = false // Make sure we get the file info
-			}
-		}
-		if fileInfoObtained {
-			return fileInfoData, nil
-		}
-
-		if !anyFileObtained {
 			return modFileInfo{}, fmt.Errorf("addon %d has no files", modInfoData.ID)
 		}
 
+		var fileInfoData *modFileInfo
+		fileID, fileInfoData, _ = findLatestFile(modInfoData, mcVersions, packLoaders)
+		if fileInfoData != nil {
+			return *fileInfoData, nil
+		}
+
 		// Possible to reach this point without obtaining file info; particularly from GameVersionLatestFiles
 		if fileID == 0 {
 			return modFileInfo{}, errors.New("mod not available for the configured Minecraft version(s) (use the acceptable-game-versions option to accept more) or loader")

modrinth/install.go

@@ -142,14 +142,14 @@ func installVersionById(versionId string, versionFilename string, pack core.Pack
 }
 
 func installViaSearch(query string, versionFilename string, autoAcceptFirst bool, pack core.Pack, index *core.Index) error {
-	mcVersion, err := pack.GetMCVersion()
+	mcVersions, err := pack.GetSupportedMCVersions()
 	if err != nil {
 		return err
 	}
 
 	fmt.Println("Searching Modrinth...")
 
-	results, err := getProjectIdsViaSearch(query, append([]string{mcVersion}, viper.GetStringSlice("acceptable-game-versions")...))
+	results, err := getProjectIdsViaSearch(query, mcVersions)
 	if err != nil {
 		return err
 	}

modrinth/modrinth.go

@@ -227,10 +227,14 @@ func parseSlugOrUrl(input string, slug *string, version *string, versionID *stri
 	return
 }
 
-func findLatestVersion(versions []*modrinthApi.Version, useFlexVer bool) *modrinthApi.Version {
+func findLatestVersion(versions []*modrinthApi.Version, gameVersions []string, useFlexVer bool) *modrinthApi.Version {
 	latestValidVersion := versions[0]
 	latestValidLoaderIdx := getMinLoaderIdx(versions[0].Loaders)
+	bestGameVersion := core.HighestSliceIndex(gameVersions, versions[0].GameVersions)
 	for _, v := range versions[1:] {
+		loaderIdx := getMinLoaderIdx(v.Loaders)
+		gameVersionIdx := core.HighestSliceIndex(gameVersions, v.GameVersions)
+
 		var compare int32
 		if useFlexVer {
 			// Use FlexVer to compare versions
@@ -238,22 +242,18 @@ func findLatestVersion(versions []*modrinthApi.Version, useFlexVer bool) *modrin
 		}
 
 		if compare == 0 {
-			loaderIdx := getMinLoaderIdx(v.Loaders)
+			if loaderIdx < latestValidLoaderIdx { // Prefer loaders; principally Quilt over Fabric, mods over datapacks (Modrinth backend handles filtering)
-			// Prefer loaders; principally Quilt over Fabric, mods over datapacks (Modrinth backend handles filtering)
+				compare = 1
-			if loaderIdx < latestValidLoaderIdx {
+			} 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
+				compare = 1
+			}
+		}
+		if compare > 0 {
 			latestValidVersion = v
 			latestValidLoaderIdx = loaderIdx
-				continue
+			bestGameVersion = gameVersionIdx
-			}
-
-			// FlexVer comparison is equal, compare date instead
-			if v.DatePublished.After(*latestValidVersion.DatePublished) {
-				latestValidVersion = v
-				latestValidLoaderIdx = getMinLoaderIdx(v.Loaders)
-			}
-		} else if compare > 0 {
-			latestValidVersion = v
-			latestValidLoaderIdx = getMinLoaderIdx(v.Loaders)
 		}
 	}
 
@@ -261,11 +261,10 @@ func findLatestVersion(versions []*modrinthApi.Version, useFlexVer bool) *modrin
 }
 
 func getLatestVersion(projectID string, name string, pack core.Pack) (*modrinthApi.Version, error) {
-	mcVersion, err := pack.GetMCVersion()
+	gameVersions, err := pack.GetSupportedMCVersions()
 	if err != nil {
 		return nil, err
 	}
-	gameVersions := append([]string{mcVersion}, viper.GetStringSlice("acceptable-game-versions")...)
 	var loaders []string
 	if viper.GetString("datapack-folder") != "" {
 		loaders = append(pack.GetLoaders(), withDatapackPathMRLoaders...)
@@ -285,10 +284,10 @@ func getLatestVersion(projectID string, name string, pack core.Pack) (*modrinthA
 
 	// TODO: option to always compare using flexver?
 	// TODO: ask user which one to use?
-	flexverLatest := findLatestVersion(result, true)
+	flexverLatest := findLatestVersion(result, gameVersions, true)
-	releaseDateLatest := findLatestVersion(result, false)
+	releaseDateLatest := findLatestVersion(result, gameVersions, false)
 	if flexverLatest != releaseDateLatest && releaseDateLatest.VersionNumber != nil && flexverLatest.VersionNumber != nil {
-		fmt.Printf("Warning: Modrinth versions for %s inconsistent between version numbers and release dates (%s newer than %s)\n", name, *releaseDateLatest.VersionNumber, *flexverLatest.VersionNumber)
+		fmt.Printf("Warning: Modrinth versions for %s inconsistent between latest version number and newest release date (%s vs %s)\n", name, *flexverLatest.VersionNumber, *releaseDateLatest.VersionNumber)
 	}
 
 	return releaseDateLatest, nil

modrinth/updater.go

@@ -35,7 +35,7 @@ type cachedStateStore struct {
 	Version   *modrinthApi.Version
 }
 
-func (u mrUpdater) CheckUpdate(mods []core.Mod, mcVersion string, pack core.Pack) ([]core.UpdateCheck, error) {
+func (u mrUpdater) CheckUpdate(mods []core.Mod, pack core.Pack) ([]core.UpdateCheck, error) {
 	results := make([]core.UpdateCheck, len(mods))
 
 	for i, mod := range mods {