[ETF] Entity Texture Features - [Fabric & Forge]

[ETF] Entity Texture Features - [Fabric & Forge]

Mods
40M Downloads

[BUG] Infinite hang: ETF scans every single texture when ResourceLocation is "minecraft:<empty string>"

ByThePowerOfScience opened this issue ยท 2 comments

commented

Modmaker here: I tracked down the cause in a debugger. Skip to the end for the problematic area.

Describe the issue

  • What happened? [e.g. crashed, texture is broken]
    • Turning a villager from "Minecraft Comes Alive Reborn" into a Zombie Villager causes an infinite hang.
  • What were you doing when it happened? [e.g. loading the game, playing multiplayer, single-player]
    • Playing multiplayer in a modpack containing about 300 mods. However, the debugger shows that the problem is in an interaction with MCA giving a dummy ResourceLocation and ETF not knowing what to do with that.

Screenshots
dwm_2024-06-03_21-37-52

Your setup: (please complete the following information):

  • Minecraft version: 1.20.2
  • ModLoader: Forge
  • ETF Version: 5.2.3
  • Does the issue persist with only ETF installed?
    • Due to the interactions at play here, it is not easy to reproduce with only ETF installed.
      [x] Have you checked the FAQ in the Readme, or Discord, to see if it is a known issue?

The Problem Area:

Entity_Texture_Features/common/src/main/java/traben/entity_texture_features/features/property_reading/TrueRandomProvider.java

Lines 43 to 44 in 2441486

while (ETFDirectory.getDirectoryVersionOf(ETFUtils.addVariantNumberSuffix(vanillaIdentifier, totalTextureCount + 1))
!= null) {

From what I can tell, MCA Reborn gives a dummy resource location ("minecraft" : "") to Minecraft because it wants to use its own dynamic texture based on the villager's pre-existing custom skin.

The problem arises because ETFDirectory.getDirectoryVersionOf returns non-null forever when given a resource location like this, even with the suffix added. When I got to it in IDEA's JVM-attach debugger, totalTextureCount was 563133802.

The Fix:

Entity_Texture_Features/common/src/main/java/traben/entity_texture_features/ETFApi.java

Lines 438 to 473 in 2441486

static ETFApi.ETFVariantSuffixProvider getVariantProviderOrNull(ResourceLocation propertiesFileIdentifier, ResourceLocation vanillaIdentifier, String... suffixKeyName) {
//get optifine property provider or null
PropertiesRandomProvider optifine = PropertiesRandomProvider.of(propertiesFileIdentifier, vanillaIdentifier, suffixKeyName);
//get true random provider or null
TrueRandomProvider random = TrueRandomProvider.of(vanillaIdentifier);
//try fallback property if null
if (optifine == null
&& vanillaIdentifier.getPath().endsWith(".png")
&& "minecraft".equals(vanillaIdentifier.getNamespace())
&& vanillaIdentifier.getPath().contains("_")) {
String vanId = vanillaIdentifier.getPath().replaceAll("_(tame|angry|nectar|shooting|cold)", "");
optifine = PropertiesRandomProvider.of(ETFUtils.res(vanId.replace(".png", ".properties")), ETFUtils.res(vanId), suffixKeyName);
}
if (random == null && optifine == null) {
//no variation at all
return null;
} else if (/*only*/ optifine == null) {
//todo why was this there if (source != ETFManager.TextureSource.ENTITY_FEATURE) {
return random;
} else if (/*only*/ random == null) {
//optifine random confirmed
return optifine;
} else {
//if 2.png is higher it MUST be treated as true random confirmed
if (optifine.getPackName() != null
&& optifine.getPackName().equals(ETFUtils.returnNameOfHighestPackFromTheseTwo(
random.getPackName(), optifine.getPackName()))) {
return optifine;
} else {
//todo why was this there if (source != ETFManager.TextureSource.ENTITY_FEATURE) {
return random;
}
}
}

It's pretty easy: at the top of ETFApi.ETFVariantSuffixProvider#getVariantProviderOrNull, check if vanillaIdentifier is a valid identifier to begin with before matching textures for it. That or just check if it's empty. Should fix the issue right up.

Stacktrace from IDEA Debugger 
lambda$negate$3:5613, Pattern$CharPredicate (java.util.regex)
is:-1, Pattern$CharPredicate$$Lambda$30+0x0000000800081a80 (java.util.regex)
match:4281, Pattern$CharPropertyGreedy (java.util.regex)
match:1755, Matcher (java.util.regex)
matches:712, Matcher (java.util.regex)
matches:1176, Pattern (java.util.regex)
matches:2844, String (java.lang)
addVariantNumberSuffix:112, ETFUtils2 (traben.entity_texture_features.utils)
addVariantNumberSuffix:98, ETFUtils2 (traben.entity_texture_features.utils)
of:41, TrueRandomProvider (traben.entity_texture_features.features.property_reading)
getVariantProviderOrNull:446, ETFApi$ETFVariantSuffixProvider (traben.entity_texture_features)
of:24, ETFTextureVariator (traben.entity_texture_features.features.texture_handlers)
getETFTextureVariant:263, ETFManager (traben.entity_texture_features.features)
getETFVariantNotNullForInjector:45, ETFUtils2 (traben.entity_texture_features.utils)
localvar$gaf000$entity_texture_features$etf$mixinAllEntityLayers:3270, RenderType (net.minecraft.client.renderer)
m_110443_:201, RenderType (net.minecraft.client.renderer)
m_110458_:205, RenderType (net.minecraft.client.renderer)
renderModel:109, VillagerLayer (forge.net.mca.client.render.layer)
renderFinal:99, VillagerLayer (forge.net.mca.client.render.layer)
render:90, VillagerLayer (forge.net.mca.client.render.layer)
m_6494_:29, VillagerLayer (forge.net.mca.client.render.layer)
m_7392_:131, LivingEntityRenderer (net.minecraft.client.renderer.entity)
m_7392_:45, MobRenderer (net.minecraft.client.renderer.entity)
m_7392_:18, MobRenderer (net.minecraft.client.renderer.entity)
m_114384_:140, EntityRenderDispatcher (net.minecraft.client.renderer.entity)
m_109517_:1440, LevelRenderer (net.minecraft.client.renderer)
m_109599_:1223, LevelRenderer (net.minecraft.client.renderer)
m_109089_:1126, GameRenderer (net.minecraft.client.renderer)
m_109093_:909, GameRenderer (net.minecraft.client.renderer)
m_91383_:1146, Minecraft (net.minecraft.client)
m_91374_:718, Minecraft (net.minecraft.client)
handleClientCrash:37, InGameCatcher (fudge.notenoughcrashes.mixinhandlers)
modify$ejd000$notenoughcrashes$atTheEndOfFirstCatchBeforePrintingCrashReport:12990, Minecraft (net.minecraft.client)
m_91374_:738, Minecraft (net.minecraft.client)
main:218, Main (net.minecraft.client.main)
invoke0:-1, NativeMethodAccessorImpl (jdk.internal.reflect)
invoke:77, NativeMethodAccessorImpl (jdk.internal.reflect)
invoke:43, DelegatingMethodAccessorImpl (jdk.internal.reflect)
invoke:568, Method (java.lang.reflect)
runTarget:111, CommonLaunchHandler (net.minecraftforge.fml.loading.targets)
clientService:99, CommonLaunchHandler (net.minecraftforge.fml.loading.targets)
lambda$makeService$0:25, CommonClientLaunchHandler (net.minecraftforge.fml.loading.targets)
run:-1, CommonClientLaunchHandler$$Lambda$1315+0x000000080076a6c8 (net.minecraftforge.fml.loading.targets)
launch:30, LaunchServiceHandlerDecorator (cpw.mods.modlauncher)
launch:53, LaunchServiceHandler (cpw.mods.modlauncher)
launch:71, LaunchServiceHandler (cpw.mods.modlauncher)
run:108, Launcher (cpw.mods.modlauncher)
main:78, Launcher (cpw.mods.modlauncher)
accept:26, BootstrapLaunchConsumer (cpw.mods.modlauncher)
accept:23, BootstrapLaunchConsumer (cpw.mods.modlauncher)
main:141, BootstrapLauncher (cpw.mods.bootstraplauncher)
commented

An interested edge case I hadn't expected, does the illegal path setting in etf>misc settings change this behaviour at all?

commented

An interested edge case I hadn't expected, does the illegal path setting in etf>misc settings change this behaviour at all?

Sorry for never replying, I was waiting for when I could get around to checking and never did. Regardless, the "empty" check should probably be in base code anyway to prevent the bug, since ResLoc("minecraft", "") is the trivially-invalid path.