bug ItemStackHolder crashes on private field (ObjectHolder doesn't)

[cad97]

24 minutes ago, cad97 said:

TL;DR reproduction case:

 

import net.minecraft.item.Item;
import net.minecraft.item.ItemStack;
import net.minecraftforge.fml.common.Mod;
import net.minecraftforge.fml.common.event.FMLPostInitializationEvent;
import net.minecraftforge.fml.common.event.FMLPreInitializationEvent;
import net.minecraftforge.fml.common.registry.GameRegistry;
import org.apache.logging.log4j.Logger;

@Mod(modid = "itemstackholder")
public class ISHMod {
    private Logger logger;

    @GameRegistry.ObjectHolder("minecraft:iron_bars")
    private static Item itemIronBars;

    @GameRegistry.ItemStackHolder("minecraft:iron_bars")
    private static ItemStack stackIronBars;

    public Logger getLogger() {
        return logger;
    }

    @Mod.EventHandler
    public void preInit(FMLPreInitializationEvent event) {
        logger = event.getModLog();
    }

    @Mod.EventHandler
    public void postInit(FMLPostInitializationEvent event) {
        logger.info(itemIronBars.toString());
        logger.info(stackIronBars.toString());
    }
}

 

This crashes with:

 

[20:24:57] [main/INFO] [FML]: Processing ObjectHolder annotations
[20:24:57] [main/INFO] [FML]: Found 1169 ObjectHolder annotations
[20:24:57] [main/INFO] [FML]: Identifying ItemStackHolder annotations
[20:24:57] [main/INFO] [STDOUT]: [net.minecraft.init.Bootstrap:printToSYSOUT:629]: ---- Minecraft Crash Report ----
// Everything's going to plan. No, really, that was supposed to happen.

Time: 7/6/17 8:24 PM
Description: Initializing game

java.lang.RuntimeException: java.lang.NoSuchFieldException: stackIronBars
	at com.google.common.base.Throwables.propagate(Throwables.java:240)
	at net.minecraftforge.fml.common.registry.ItemStackHolderInjector.addHolder(ItemStackHolderInjector.java:97)
	at net.minecraftforge.fml.common.registry.ItemStackHolderInjector.findHolders(ItemStackHolderInjector.java:63)
	at net.minecraftforge.fml.common.Loader.preinitializeMods(Loader.java:602)
	at net.minecraftforge.fml.client.FMLClientHandler.beginMinecraftLoading(FMLClientHandler.java:266)
	at net.minecraft.client.Minecraft.init(Minecraft.java:508)
	at net.minecraft.client.Minecraft.run(Minecraft.java:416)
	at net.minecraft.client.main.Main.main(Main.java:118)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at net.minecraft.launchwrapper.Launch.launch(Launch.java:135)
	at net.minecraft.launchwrapper.Launch.main(Launch.java:28)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97)
	at GradleStart.main(GradleStart.java:26)
Caused by: java.lang.NoSuchFieldException: stackIronBars
	at java.lang.Class.getField(Class.java:1703)
	at net.minecraftforge.fml.common.registry.ItemStackHolderInjector.addHolder(ItemStackHolderInjector.java:91)
	... 18 more


A detailed walkthrough of the error, its code path and all known details is as follows:
---------------------------------------------------------------------------------------

-- Head --
Thread: Client thread
Stacktrace:
	at com.google.common.base.Throwables.propagate(Throwables.java:240)
	at net.minecraftforge.fml.common.registry.ItemStackHolderInjector.addHolder(ItemStackHolderInjector.java:97)
	at net.minecraftforge.fml.common.registry.ItemStackHolderInjector.findHolders(ItemStackHolderInjector.java:63)
	at net.minecraftforge.fml.common.Loader.preinitializeMods(Loader.java:602)
	at net.minecraftforge.fml.client.FMLClientHandler.beginMinecraftLoading(FMLClientHandler.java:266)
	at net.minecraft.client.Minecraft.init(Minecraft.java:508)

-- Initialization --
Details:
Stacktrace:
	at net.minecraft.client.Minecraft.run(Minecraft.java:416)
	at net.minecraft.client.main.Main.main(Main.java:118)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at net.minecraft.launchwrapper.Launch.launch(Launch.java:135)
	at net.minecraft.launchwrapper.Launch.main(Launch.java:28)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at net.minecraftforge.gradle.GradleStartCommon.launch(GradleStartCommon.java:97)
	at GradleStart.main(GradleStart.java:26)

-- System Details --
Details:
	Minecraft Version: 1.12
	Operating System: Windows 10 (amd64) version 10.0
	Java Version: 1.8.0_121, Oracle Corporation
	Java VM Version: Java HotSpot(TM) 64-Bit Server VM (mixed mode), Oracle Corporation
	Memory: 653341240 bytes (623 MB) / 789053440 bytes (752 MB) up to 3801088000 bytes (3625 MB)
	JVM Flags: 0 total; 
	IntCache: cache: 0, tcache: 0, allocated: 0, tallocated: 0
	FML: MCP 9.40 Powered by Forge 14.21.1.2387 5 mods loaded, 5 mods active
	States: 'U' = Unloaded 'L' = Loaded 'C' = Constructed 'H' = Pre-initialized 'I' = Initialized 'J' = Post-initialized 'A' = Available 'D' = Disabled 'E' = Errored
	UC	minecraft{1.12} [Minecraft] (minecraft.jar) 
	UC	mcp{9.19} [Minecraft Coder Pack] (minecraft.jar) 
	UC	FML{8.0.99.99} [Forge Mod Loader] (forgeSrc-1.12-14.21.1.2387.jar) 
	UC	forge{14.21.1.2387} [Minecraft Forge] (forgeSrc-1.12-14.21.1.2387.jar) 
	UC	itemstackholder{1.0} [itemstackholder] (itemstackholder_main) 
	Loaded coremods (and transformers): 
	GL info: ' Vendor: 'NVIDIA Corporation' Version: '4.5.0 NVIDIA 382.05' Renderer: 'GeForce GTX 860M/PCIe/SSE2'
	Launched Version: 1.12
	LWJGL: 2.9.4
	OpenGL: GeForce GTX 860M/PCIe/SSE2 GL version 4.5.0 NVIDIA 382.05, NVIDIA Corporation
	GL Caps: Using GL 1.3 multitexturing.
Using GL 1.3 texture combiners.
Using framebuffer objects because OpenGL 3.0 is supported and separate blending is supported.
Shaders are available because OpenGL 2.1 is supported.
VBOs are available because OpenGL 1.5 is supported.

	Using VBOs: Yes
	Is Modded: Definitely; Client brand changed to 'fml,forge'
	Type: Client (map_client.txt)
	Resource Packs: 
	Current Language: English (US)
	Profiler Position: N/A (disabled)
	CPU: 8x Intel(R) Core(TM) i7-4810MQ CPU @ 2.80GHz
[20:24:57] [main/INFO] [STDOUT]: [net.minecraft.init.Bootstrap:printToSYSOUT:629]: #@!@# Game crashed! Crash report saved to: #@!@# D:\...\itemstackholder\run\.\crash-reports\crash-2017-07-06_20.24.57-client.txt

 

The crash comes from a catch/throw in Minecraft Forge with a comment // unpossible?, so should be worth looking into. The behaviors are different, so one is (probably) more wrong.

 

The difference looks to be this: ItemStackHolderInjector uses Field f = clazz.getField(annotationTarget); and ObjectHolderRegistry uses Field f = clazz.getDeclaredField(annotationTarget);.

 

Switching ItemStackHolderInjector to use Class::getDeclaredField like ObjectHolderRegistry would, at the very least, save future modders the confusion I felt when java told me that the field that is right there doesn't exist.

 

This would also enable the use case where I discovered this: private fields populated with ItemStacks for use in recipe registration. Because I've caught Kotlin fever, the linked code there is in Kotlin, but compiles to the equivalent private static member (with a private set method and a public get method, as of the linked commit; I plan to make it fully private, this was just the work commit where I caught this issue).

 

I can make this an issue and/or PR against the MinecraftForge GitHub, but I didn't want to needlessly clutter the repository if this is considered a non-issue. Consider my vote to apply this one-line patch, though. I haven't tested this, because I don't currently have the setup required to build a custom version of forge and test it.

(Cross-post from accidental placement in Modder Support)

[LexManos]

Yes this is kinda intentional.

Honestly I think ItemStackHolder shouldn't need to exist anymore. So when I was doing the rewrite I didn't touch it.

Private field support was a ADDED functionality I added when I re-worked the registry stuff. 

So ya, no real change here, just don't use ItemStackHolder on private fields.