Skip to content

Conversation

@CiiLu
Copy link
Contributor

@CiiLu CiiLu commented Dec 28, 2025

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds automatic installation support for Legacy Fabric, a mod loader for older Minecraft versions (1.13.2 and below). The implementation follows the same patterns used for Fabric and Quilt mod loaders.

Key Changes

  • Added LEGACY_FABRIC enum value to ModLoaderType and corresponding library types in LibraryAnalyzer
  • Created new package org.jackhuang.hmcl.download.legacyfabric with version lists, install tasks, and API support
  • Updated download providers (MojangDownloadProvider and BMCLAPIDownloadProvider) to include Legacy Fabric support
  • Added localization strings and icons for Legacy Fabric across multiple languages
  • Updated UI components to display and handle Legacy Fabric installations
  • Modified switch statements from old-style to switch expressions while adding Legacy Fabric cases

Reviewed changes

Copilot reviewed 29 out of 31 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
ModLoaderType.java Added LEGACY_FABRIC enum value
LibraryAnalyzer.java Added LEGACY_FABRIC and LEGACY_FABRIC_API library types with detection logic
LegacyFabricVersionList.java New file implementing version list for Legacy Fabric loader
LegacyFabricRemoteVersion.java New file defining remote version for Legacy Fabric
LegacyFabricInstallTask.java New file implementing installation task for Legacy Fabric
LegacyFabricAPIVersionList.java New file implementing version list for Legacy Fabric API
LegacyFabricAPIRemoteVersion.java New file defining remote version for Legacy Fabric API
LegacyFabricAPIInstallTask.java New file implementing installation task for Legacy Fabric API
MojangDownloadProvider.java Added Legacy Fabric version lists and updated switch to expression
BMCLAPIDownloadProvider.java Added Legacy Fabric version lists and updated switch to expression
DefaultLauncher.java Added INST_LEGACYFABRIC environment variable
McbbsModpackExportTask.java Added Legacy Fabric support for modpack export
FabricInstallTask.java Added @JsonSerializable annotations for Gson classes
QuiltInstallTask.java Added @JsonSerializable annotations for Gson classes
FabricAPIInstallTask.java Refactored to use getModsDirectory() method
QuiltAPIInstallTask.java Refactored to use getModsDirectory() method
I18N*.properties Added localization strings for Legacy Fabric in English, Chinese, and Traditional Chinese
legacyfabric.png / [email protected] Added icon images for Legacy Fabric
VersionIconType.java Added LEGACY_FABRIC icon type
VersionIconDialog.java Added Legacy Fabric icon to version icon selection
ModListPageSkin.java Updated switch statements and added Legacy Fabric case
ModListPage.java Added Legacy Fabric to supported loaders (supports Fabric mods)
InstallerListPage.java Updated filter logic to exclude all API installers using endsWith
VersionsPage.java Added icon mapping for Legacy Fabric versions
InstallersPage.java Added Legacy Fabric case with underscore replacement
TaskListPane.java Added Legacy Fabric install task naming and stage handling
InstallerItem.java Added Legacy Fabric items and incompatibility rules
HMCLGameRepository.java Added Legacy Fabric icon detection for version display

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mutualIncompatible(incompatibleMap, forge, fabric, quilt, neoForge, cleanroom, legacyfabric);
addIncompatibles(incompatibleMap, liteLoader, fabric, quilt, neoForge, cleanroom, legacyfabric);
addIncompatibles(incompatibleMap, optiFine, fabric, quilt, neoForge, cleanroom, liteLoader, legacyfabric);
addIncompatibles(incompatibleMap, fabricApi, forge, quiltApi, neoForge, liteLoader, optiFine, cleanroom, legacyfabricApi, legacyfabricApi);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fabricApi incompatibility list has 'legacyfabricApi' duplicated. The second occurrence should be 'legacyfabric' instead, so that fabricApi is marked as incompatible with both the Legacy Fabric loader and Legacy Fabric API.

Suggested change
addIncompatibles(incompatibleMap, fabricApi, forge, quiltApi, neoForge, liteLoader, optiFine, cleanroom, legacyfabricApi, legacyfabricApi);
addIncompatibles(incompatibleMap, fabricApi, forge, quiltApi, neoForge, liteLoader, optiFine, cleanroom, legacyfabricApi, legacyfabric);

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,61 @@
/*
* Hello Minecraft! Launcher
* Copyright (C) 2021 huangyuhui <[email protected]> and contributors
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent copyright year. The copyright year is '2021' but other files in the same package use '2022'. For consistency, this should be updated to match.

Copilot uses AI. Check for mistakes.
case FORGE -> loaderName = i18n("install.installer.forge");
case NEO_FORGE -> loaderName = i18n("install.installer.neoforge");
case CLEANROOM -> loaderName = i18n("install.installer.cleanroom");
case LEGACY_FABRIC -> loaderName = i18n("install.installer.legacyfabric").replace(" ", "_");
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The replacement of spaces with underscores only for Legacy Fabric seems inconsistent with how other loaders are named. This special handling should be documented or the naming convention should be consistent across all loaders.

Copilot uses AI. Check for mistakes.
addons.add(new McbbsModpackManifest.Addon(FABRIC.getPatchId(), fabricVersion)));
analyzer.getVersion(QUILT).ifPresent(quiltVersion ->
addons.add(new McbbsModpackManifest.Addon(QUILT.getPatchId(), quiltVersion)));
analyzer.getVersion(LEGACY_FABRIC).ifPresent(quiltVersion ->
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable name should be 'legacyFabricVersion' instead of 'quiltVersion'. This is a copy-paste error from the Quilt code above.

Copilot uses AI. Check for mistakes.
LITE_LOADER,
PACK;
LEGACY_FABRIC,
PACK
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon at the end of the enum. The semicolon should be placed after 'PACK' on line 29.

Suggested change
PACK
PACK;

Copilot uses AI. Check for mistakes.
return true;
}
},
FABRIC_API(true, "fabric-api", "net.fabricmc", "fabric-api", null),FORGE(true, "forge", "net\\.minecraftforge", "(forge|fmlloader)", ModLoaderType.FORGE) {
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma. There should be a space between 'FABRIC_API(true, "fabric-api", "net.fabricmc", "fabric-api", null),' and 'FORGE(...)' to maintain consistent formatting.

Suggested change
FABRIC_API(true, "fabric-api", "net.fabricmc", "fabric-api", null),FORGE(true, "forge", "net\\.minecraftforge", "(forge|fmlloader)", ModLoaderType.FORGE) {
FABRIC_API(true, "fabric-api", "net.fabricmc", "fabric-api", null), FORGE(true, "forge", "net\\.minecraftforge", "(forge|fmlloader)", ModLoaderType.FORGE) {

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants