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

[ByThePowerOfScience]

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

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:

https://github.com/Traben-0/Entity_Texture_Features/blob/2441486fc1ae8bc71038707f16239fd9b3fc50d7/common/src/main/java/traben/entity_texture_features/features/property_reading/TrueRandomProvider.java#L43-L44

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:

https://github.com/Traben-0/Entity_Texture_Features/blob/2441486fc1ae8bc71038707f16239fd9b3fc50d7/common/src/main/java/traben/entity_texture_features/ETFApi.java#L438-L473

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) ```

[Traben-0]

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

[ByThePowerOfScience]

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.