Mod Menu

Mod Menu

Mods
37M Downloads

Modmenu produces a NPE when having an unknown Badge entry

zeichenreihe opened this issue ยท 0 comments

commented

Having a mod that has for example the following entry in the fabric.mod.json lets modmenu produce a NPE while rendering badges.

"custom": {
    "modmenu": {
      "badges": [
        "liteloader"
      ]
    }
  }

This occours, because the badge variable in the second method here is null:

ModMenu/src/main/java/com/terraformersmc/modmenu/util/mod/ModBadgeRenderer.java

Lines 31 to 40 in 5d640d6

public void draw(DrawContext DrawContext, int mouseX, int mouseY) {
this.badgeX = startX;
this.badgeY = startY;
Set<Mod.Badge> badges = mod.getBadges();
badges.forEach(badge -> drawBadge(DrawContext, badge, mouseX, mouseY));
}
public void drawBadge(DrawContext DrawContext, Mod.Badge badge, int mouseX, int mouseY) {
this.drawBadge(DrawContext, badge.getText().asOrderedText(), badge.getOutlineColor(), badge.getFillColor(), mouseX, mouseY);
}

This happens because the Set<Badge> contains null. After looking from where this set comes, you get to here:

ModMenu/src/main/java/com/terraformersmc/modmenu/util/mod/fabric/FabricMod.java

Lines 92 to 131 in 5d640d6

this.modMenuData = new ModMenuData(
badgeNames,
parentId,
parentData
);
/* Hardcode parents and badges for Fabric API & Fabric Loader */
String id = metadata.getId();
if (id.startsWith("fabric") && metadata.containsCustomValue("fabric-api:module-lifecycle")) {
if (FabricLoader.getInstance().isModLoaded("fabric-api") || !FabricLoader.getInstance().isModLoaded("fabric")) {
modMenuData.fillParentIfEmpty("fabric-api");
} else {
modMenuData.fillParentIfEmpty("fabric");
}
modMenuData.badges.add(Badge.LIBRARY);
}
if (id.startsWith("fabric") && (id.equals("fabricloader") || metadata.getProvides().contains("fabricloader") || id.equals("fabric") || id.equals("fabric-api") || metadata.getProvides().contains("fabric") || metadata.getProvides().contains("fabric-api") || id.equals("fabric-language-kotlin"))) {
modMenuData.badges.add(Badge.LIBRARY);
}
/* Add additional badges */
this.badges = modMenuData.badges;
if (this.metadata.getEnvironment() == ModEnvironment.CLIENT) {
badges.add(Badge.CLIENT);
}
if (OptionalUtil.isPresentAndTrue(CustomValueUtil.getBoolean("fabric-loom:generated", metadata)) || "java".equals(id)) {
badges.add(Badge.LIBRARY);
}
if ("deprecated".equals(CustomValueUtil.getString("fabric-api:module-lifecycle", metadata).orElse(null))) {
badges.add(Badge.DEPRECATED);
}
if (metadata.containsCustomValue("patchwork:patcherMeta")) {
badges.add(Badge.PATCHWORK_FORGE);
}
if (modpackMods.contains(getId()) && !"builtin".equals(this.metadata.getType())) {
badges.add(Badge.MODPACK);
}
if ("minecraft".equals(getId())) {
badges.add(Badge.MINECRAFT);
}

As you can see, in Line 113 it sets the field this.badges, which is returned by the getBadges method. As you can see, this code doesn't directly add null as a value. Therefore, the value null must come from the field modMenuData.badges. This field is set in the constructor of ModMenuData:

ModMenu/src/main/java/com/terraformersmc/modmenu/util/mod/fabric/FabricMod.java

Lines 337 to 341 in 5d640d6

public ModMenuData(Set<String> badges, Optional<String> parent, DummyParentData dummyParentData) {
this.badges = Badge.convert(badges);
this.parent = parent;
this.dummyParentData = dummyParentData;
}

This just calls Badges.convert(), which takes a Set<String> and gives back a Set<Badge>:

ModMenu/src/main/java/com/terraformersmc/modmenu/util/mod/Mod.java

Lines 151 to 153 in 5d640d6

public static Set<Badge> convert(Set<String> badgeKeys) {
return badgeKeys.stream().map(KEY_MAP::get).collect(Collectors.toSet());
}

And this also shows us where the null comes from: KEY_MAP::get can return null if there's no key-value pair. And that's percisely what happens in my case.

Note that the usage of liteloader here is just because it happened in a modmenu port to Ornithe, a modding toolchain for old minecraft versions. The relevant code path was not changed in the code. Therefore it could also happen in modern modmenu.
(I also fist asked there, OrnitheMC/modmenu#4, and we determined that upstream also has the issue, and therefore I think that it should be fixed in upstream)

A crashlog from Ornithe:

Caused by: java.lang.NullPointerException
	at com.terraformersmc.modmenu.util.mod.ModBadgeRenderer.drawBadge(ModBadgeRenderer.java:33) ~[modmenu-0.2.0+mc1.12.2.jar:?]
	at com.terraformersmc.modmenu.util.mod.ModBadgeRenderer.lambda$draw$0(ModBadgeRenderer.java:29) ~[modmenu-0.2.0+mc1.12.2.jar:?]
	at java.lang.Iterable.forEach(Iterable.java:75) ~[?:1.8.0_392]
	at com.terraformersmc.modmenu.util.mod.ModBadgeRenderer.draw(ModBadgeRenderer.java:29) ~[modmenu-0.2.0+mc1.12.2.jar:?]
	at com.terraformersmc.modmenu.gui.widget.entries.ModListEntry.render(ModListEntry.java:75) ~[modmenu-0.2.0+mc1.12.2.jar:?]
	at net.minecraft.client.gui.widget.EntryListWidget.renderEntry(EntryListWidget.java:28) ~[1.12.2-TRv0.8.2-named-711666161528F3329D7A8C8C611232C03E2D58B845ABA1B8F40897F5800EAE50.jar:?]
	at com.terraformersmc.modmenu.gui.widget.ModListWidget.renderList(ModListWidget.java:246) ~[modmenu-0.2.0+mc1.12.2.jar:?]
	at net.minecraft.client.gui.widget.ListWidget.render(ListWidget.java:194) ~[1.12.2-TRv0.8.2-named-711666161528F3329D7A8C8C611232C03E2D58B845ABA1B8F40897F5800EAE50.jar:?]
	at com.terraformersmc.modmenu.gui.ModsScreen.render(ModsScreen.java:321) ~[modmenu-0.2.0+mc1.12.2.jar:?]
	at net.minecraft.client.render.GameRenderer.render(GameRenderer.java:947) ~[1.12.2-TRv0.8.2-named-711666161528F3329D7A8C8C611232C03E2D58B845ABA1B8F40897F5800EAE50.jar:?]
	... 6 more

This issue also comes with further questions:

  • Should there be some kind of API to register badge? This would, at least partially, remove the issue, as then one mod counld register the badge liteloader and define it's colors and text (see what skyrising used for an example here: skyrising@385c15f)
  • This would however still not 100% fix the issue, Badges.convert() should at least either throw an exception if there's any null in the set (that could happen if a mod forgot to register a badge), or silently filter out nulls in the set. This could be done by filtering the stream: .filter(Objects::nonNull).