Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public HMCLModpackInstallTask(HMCLGameRepository repository, Path zipFile, Modpa
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(Modpack.class));

if (!HMCLModpackProvider.INSTANCE.getName().equals(config.getType()))
if (config.getType() != null && !HMCLModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a HMCL modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
Expand Down
20 changes: 12 additions & 8 deletions HMCL/src/main/java/org/jackhuang/hmcl/game/ModpackHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -236,19 +236,23 @@ else if (modpack.getManifest() instanceof McbbsModpackManifest)
}

public static Task<Void> getUpdateTask(HMCLGameRepository repository, ServerModpackManifest manifest, Charset charset, GameInstanceID instanceId, ModpackConfiguration<?> configuration) throws UnsupportedModpackException {
switch (configuration.getType()) {
case ServerModpackRemoteInstallTask.MODPACK_TYPE:
return new ModpackUpdateTask(repository, instanceId, new ServerModpackRemoteInstallTask(repository.getDependency(), manifest, instanceId))
.thenComposeAsync(repository.refreshAsync())
.withStagesHints(new Task.StagesHint("hmcl.modpack"), new Task.StagesHint("hmcl.modpack.download", List.of("hmcl.install.assets", "hmcl.install.libraries")));
default:
throw new UnsupportedModpackException();
String type = configuration.getType();
if (type == null || ServerModpackRemoteInstallTask.MODPACK_TYPE.equals(type)) {
return new ModpackUpdateTask(repository, instanceId, new ServerModpackRemoteInstallTask(repository.getDependency(), manifest, instanceId))
.thenComposeAsync(repository.refreshAsync())
.withStagesHints(new Task.StagesHint("hmcl.modpack"), new Task.StagesHint("hmcl.modpack.download", List.of("hmcl.install.assets", "hmcl.install.libraries")));
} else {
throw new UnsupportedModpackException();
}
}

public static Task<?> getUpdateTask(HMCLGameRepository repository, Path zipFile, Charset charset, GameInstanceID instanceId, ModpackConfiguration<?> configuration) throws UnsupportedModpackException, ManuallyCreatedModpackException, MismatchedModpackTypeException {
Modpack modpack = ModpackHelper.readModpackManifest(zipFile, charset);
ModpackProvider provider = getProviderByType(configuration.getType());
String type = configuration.getType();
if (type == null) {
type = modpack.getManifest().getProvider().getName();
}
ModpackProvider provider = getProviderByType(type);
if (provider == null) {
throw new UnsupportedModpackException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;

@Immutable
public final class ModpackConfiguration<T> implements Validation {
Expand All @@ -36,7 +37,7 @@ public static <T> TypeToken<ModpackConfiguration<T>> typeOf(Class<T> clazz) {
}

private final T manifest;
private final String type;
private final @Nullable String type;
private final String name;
private final String version;
private final List<FileInformation> overrides;
Expand All @@ -45,7 +46,7 @@ public ModpackConfiguration() {
this(null, null, "", null, Collections.emptyList());
}

public ModpackConfiguration(T manifest, String type, String name, String version, List<FileInformation> overrides) {
public ModpackConfiguration(T manifest, @Nullable String type, String name, String version, List<FileInformation> overrides) {
this.manifest = manifest;
this.type = type;
this.name = name;
Expand All @@ -57,8 +58,35 @@ public T getManifest() {
return manifest;
}

@Nullable
public String getType() {
return type;
if (type != null) {
return type;
}
if (manifest instanceof ModpackManifest modpackManifest) {
return modpackManifest.getProvider().getName();
}
if (manifest instanceof Modpack modpack && modpack.getManifest() != null) {
return modpack.getManifest().getProvider().getName();
}
if (manifest instanceof Map<?, ?> map) {
if (map.containsKey("instanceType") || map.containsKey("components") || map.containsKey("mmcPack")) {
return "MultiMC";
}
if (map.containsKey("formatVersion") && map.containsKey("game")) {
return "Modrinth";
}
if (map.containsKey("files") && map.containsKey("minecraft")) {
return "Curse";
}
if (map.containsKey("fileApi") && !map.containsKey("manifestType")) {
return "Server";
}
if (map.containsKey("addons") || map.containsKey("manifestType")) {
return "Mcbbs";
}
}
return null;
}

public String getName() {
Expand Down Expand Up @@ -90,8 +118,6 @@ public List<FileInformation> getOverrides() {
public void validate() throws JsonParseException {
if (manifest == null)
throw new JsonParseException("MinecraftInstanceConfiguration missing `manifest`");
if (type == null)
throw new JsonParseException("MinecraftInstanceConfiguration missing `type`");
}
Comment on lines 118 to 121

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Infer the provider before accepting a null type

When an existing modpack.cfg omits type, this relaxed validation lets finishModpackInstallingAsync proceed, but it then calls ModpackHelper.getUpdateTask before any of the modified installer checks run. The local-file path passes null to getProviderByType and reports the pack as unsupported, while the server-manifest path switches on null and throws an uncaught NullPointerException; consequently, the legacy configurations targeted by this change still cannot be updated. The provider must be inferred or normalized before these dispatch points rather than merely allowing type to remain null.

Useful? React with 👍 / 👎.


@Immutable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ public CurseInstallTask(DefaultDependencyManager dependencyManager, Path zipFile
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(CurseManifest.class));

if (!CurseModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a Curse modpack. Cannot update this instance.");
if (config.getType() != null && !CurseModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a CurseForge modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ public McbbsModpackLocalInstallTask(DefaultDependencyManager dependencyManager,
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(McbbsModpackManifest.class));

if (!McbbsModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a Mcbbs modpack. Cannot update this instance.");
if (config.getType() != null && !McbbsModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a MCBBS modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ public McbbsModpackRemoteInstallTask(DefaultDependencyManager dependencyManager,
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(McbbsModpackManifest.class));

if (!MODPACK_TYPE.equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a Mcbbs modpack. Cannot update this instance.");
if (config.getType() != null && !McbbsModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a MCBBS modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public ModrinthInstallTask(DefaultDependencyManager dependencyManager, Path zipF
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(ModrinthManifest.class));

if (!ModrinthModpackProvider.INSTANCE.getName().equals(config.getType()))
if (config.getType() != null && !ModrinthModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a Modrinth modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public void preExecute() throws Exception {
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(MultiMCInstanceConfiguration.class));

if (!MultiMCModpackProvider.INSTANCE.getName().equals(config.getType()))
if (config.getType() != null && !MultiMCModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a MultiMC modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public ServerModpackLocalInstallTask(DefaultDependencyManager dependencyManager,
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(ServerModpackManifest.class));

if (!ServerModpackProvider.INSTANCE.getName().equals(config.getType()))
if (config.getType() != null && !ServerModpackProvider.INSTANCE.getName().equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a Server modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public ServerModpackRemoteInstallTask(DefaultDependencyManager dependencyManager
if (Files.exists(json)) {
config = JsonUtils.fromJsonFile(json, ModpackConfiguration.typeOf(ServerModpackManifest.class));

if (!MODPACK_TYPE.equals(config.getType()))
if (config.getType() != null && !MODPACK_TYPE.equals(config.getType()))
throw new IllegalArgumentException("Instance " + instanceId + " is not a Server modpack. Cannot update this instance.");
}
} catch (JsonParseException | IOException ignore) {
Expand Down