jabelar

So I have a handful of mods I like to publish and I also like to publish them for a few versions of Minecraft. I also have created a "starter" mod that is already set up with a lot of the tricks and utility classes in the way I like to organize my mod. Until now, when I wanted to start a new mod I wouldn't do any git branching but would rather do file copy of my starter mod and make it a new repository. For different versions of the mod I would do the same (file copy and new repository). This works pretty well but doesn't take advantage of ability to merge and cherry pick fixes and features across mods and across versions of mods. If I want to update something in one of my mods I have to cut and paste. So I feel that there is some benefit in doing proper branching and release management. In other words, I think what I should do is instead of copying my starter mod I should create a branch for each mod and then create a branch from that for each version of Minecraft. Is that recommended? My hesitation is that I just tried doing that and found that the amount of modification between versions of Minecraft is often quite large. Like if 1.13 comes along with "the flattening" I'll probably have so much recoding to do that I'm not sure managing it as a branch gives me much advantage. In the past things like the addition of blockstates, or changes to capabilities, are similarly pretty big changes. And frankly managing branches and merges can be a bit of a hassle. So any opinions on this? Is it better to simply hand-craft a repository for each mod/version or do you guys find proper code branching and release management the way to go? (Of course even if I do each mod separately I still use git to manage the development along the way, so I'm not debating the overall use of git.)

The event handler class is your own class. So you can just make a class called EventHandler. Then in that method you can put all the different methods you might have over time to handle events. Also, there is a newer way of registering the class so you should use that (I'll update the tutorial). Instead of explicitly registering it to the bus, you can now just put an annotation on your class: @EventBusSubscriber(modid = "your mod id here") Of course for put in your mod id string there. Also, for this annotation to work you need to make the method static. So just add the static keyword to my tutorial code. I'll update the tutorial with this method over next couple days, but you should be able to follow my instruction above to get it working.

the event parameter has a getItem() method. Call it and check if it is equal to the iron ingot.

I wrote a tutorial on this: http://jabelarminecraft.blogspot.com/p/minecraft-forge-1721710-keybinding.html

What's in the WorldGenStructure class? Is that your own class -- I can't find that class in the vanilla or forge source... Anyway, the basic structure template format doesn't directly allow loot tables to get into the generated chests. But you can mark the chests by adding data blocks to your structure, then when you generate the structure you can loop through the data blocks and use them for additional processing. For example, you could put a data block right on top of the chest block to mark the position and then use that to find the tileentity for the chest when it generates and add the loot table directly at that point. There might be a simpler way, but that seems to be the way some other people have solved this problem.

Yeah, I've looked at this before and for some reason (mainly because vanilla Minecraft didn't need it), the packets that are sent for damage don't fully reflect damage source (unless it is a couple specific types that have a visual effect). One way to work around this is to send your own custom packet. Handle the LivingHurtEvent or similar on server side and send a packet to the client with the information you need to help you display.

This is loot for your own structure, right? And is your structure coded using classes that extend the StructureComponent class? If so, there is a call for generateChest() that you can override. Also, generally if you've coded your own structure you are free to place blocks however you want, so you can just place a chest however you want. Anyway, it is easier to help if you post the code for your structure that you have already.

can you post your code again? the gist link is giving me a 404 error.

You're right, it should be previous tick position. I'll fo back edit the equation to be correct.

EDIT: Made a correction based on diesieben07's comment. The reason you're confused is that your code is called every render frame, not every tick. Instead you must handle both ticks and renders. The partial ticks helps you convert and synchronize between them. This is how I would name the fields in my equation to make it clearer: currentFramePos = previousTickPos + (currentTickPos - previousTickPos) * partialTicks Then the steps would be as follows: 1) Start by initializing all of the position fields to the starting position. Don't intialize partialTicks as that is a parameter you'll get from the render event. 2) In a tick-handling (not a render!) event you would set the previousTickPos to currentTickPos, then add whatever motion you want to currentTickPos. 3) In a render-handling event you would set the currentFramePos using the formula: currentFramePos = previousTickPos + (currentTickPos - previousTickPos) * partialTicks The reason you do all this is that you need to draw in render events but you need to set the motion speed in a tick event, so you need to handle both ticks and renders and use partialTicks to convert.

One thing to keep in mind is that iterating through a large area will likely cause performance issues. You have to realize that the number of blocks grows rapidly. For example, in one chunk there is 65k block positions to scan! If you scan for two chunks in each direction you'll be at over 1 million block positions. Due to this performance hit, there are not really any vanilla mechanisms that scan blocks for a distance. There are a lot of "neighbor" checks (like redstone connections) but they only check the next block. Thinks like bookshelves have a dedicated scan when the enchanting table is opened and it only checks a short distance for specific placement. What exactly are you trying to achieve?

You can still do it but would require doing something a bit intrusive. For example, one approach is to replace all EntityArrow with your own version that extends EntityArrow and overrides the collision method. Another approach would be to set the public pickupStatus field in every EntityArrow to EntityArrow.PickupStatus.DISALLOWED and then handle the entity update event and process the collision yourself.

What do you mean by "export? How did you build the JAR file? Did you use the gradlew build command?

Actually the more important question is whether this value needs to persist between saves. If it does not, then you are free to create public static fields that store the values, and then regenerate them each time the game starts. But if you need to save them then generally you would add them either to the player or to the world in a saveable way, such as using the WorldSaveData mechanism. You can also pretty easily just create your own file format for saving the information -- like on client side you could store it along with config info.

Yeah, but there is RegistryEvent.NewRegistry for this and it is actually fired during pre-init. But yeah, instead of doing it directly in a pre-init handler we should use the NewRegistry event. It will happen before the block and item registries.

Also the documentation says:

Is that true? In my console output I get: [15:29:43] [main/INFO] [GradleStart]: username: MistMaestro [15:29:43] [main/INFO] [GradleStart]: Extra: [] [15:29:43] [main/INFO] [GradleStart]: Running with arguments: [--userProperties, {}, --assetsDir, C:/Users/agilroy/.gradle/caches/minecraft/assets, --assetIndex, 1.12, --accessToken{REDACTED}, --version, 1.12.2, --username, MistMaestro, --tweakClass, net.minecraftforge.fml.common.launcher.FMLTweaker, --tweakClass, net.minecraftforge.gradle.tweakers.CoremodTweaker] [15:29:43] [main/INFO] [LaunchWrapper]: Loading tweak class name net.minecraftforge.fml.common.launcher.FMLTweaker [15:29:43] [main/INFO] [LaunchWrapper]: Using primary tweak class name net.minecraftforge.fml.common.launcher.FMLTweaker [15:29:43] [main/INFO] [LaunchWrapper]: Loading tweak class name net.minecraftforge.gradle.tweakers.CoremodTweaker [15:29:43] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.common.launcher.FMLTweaker [15:29:43] [main/INFO] [FML]: Forge Mod Loader version 14.23.4.2732 for Minecraft 1.12.2 loading [15:29:43] [main/INFO] [FML]: Java is Java HotSpot(TM) 64-Bit Server VM, version 1.8.0_101, running on Windows 10:amd64:10.0, installed at C:\Program Files\Java\jdk1.8.0_101\jre [15:29:43] [main/ERROR] [FML]: Apache Maven library folder was not in the format expected. Using default libraries directory. [15:29:43] [main/ERROR] [FML]: Full: C:\Users\agilroy\.gradle\caches\modules-2\files-2.1\org.apache.maven\maven-artifact\3.5.3\7dc72b6d6d8a6dced3d294ed54c2cc3515ade9f4\maven-artifact-3.5.3.jar [15:29:43] [main/ERROR] [FML]: Trimmed: c:/users/agilroy/.gradle/caches/modules-2/files-2.1/org.apache.maven/maven-artifact/3.5.3/ [15:29:43] [main/INFO] [FML]: Managed to load a deobfuscated Minecraft name- we are in a deobfuscated environment. Skipping runtime deobfuscation [15:29:43] [main/INFO] [FML]: Detected deobfuscated environment, loading log configs for colored console logs. [15:29:45] [main/INFO] [FML]: Ignoring missing certificate for coremod FMLCorePlugin (net.minecraftforge.fml.relauncher.FMLCorePlugin), we are in deobf and it's a forge core plugin [15:29:45] [main/INFO] [FML]: Ignoring missing certificate for coremod FMLForgePlugin (net.minecraftforge.classloading.FMLForgePlugin), we are in deobf and it's a forge core plugin [15:29:45] [main/INFO] [FML]: Searching C:\ModdingWorkspace\run\assets\.\mods for mods [15:29:45] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.gradle.tweakers.CoremodTweaker [15:29:45] [main/INFO] [GradleStart]: Injecting location in coremod net.minecraftforge.fml.relauncher.FMLCorePlugin [15:29:45] [main/INFO] [GradleStart]: Injecting location in coremod net.minecraftforge.classloading.FMLForgePlugin [15:29:45] [main/INFO] [LaunchWrapper]: Loading tweak class name net.minecraftforge.fml.common.launcher.FMLInjectionAndSortingTweaker [15:29:45] [main/INFO] [LaunchWrapper]: Loading tweak class name net.minecraftforge.fml.common.launcher.FMLDeobfTweaker [15:29:45] [main/INFO] [LaunchWrapper]: Loading tweak class name net.minecraftforge.gradle.tweakers.AccessTransformerTweaker [15:29:45] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.common.launcher.FMLInjectionAndSortingTweaker [15:29:45] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.common.launcher.FMLInjectionAndSortingTweaker [15:29:45] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.relauncher.CoreModManager$FMLPluginWrapper [15:29:48] [main/ERROR] [FML]: FML appears to be missing any signature data. This is not a good thing [15:29:48] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.relauncher.CoreModManager$FMLPluginWrapper [15:29:48] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.common.launcher.FMLDeobfTweaker [15:29:49] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.gradle.tweakers.AccessTransformerTweaker [15:29:49] [main/INFO] [LaunchWrapper]: Loading tweak class name net.minecraftforge.fml.common.launcher.TerminalTweaker [15:29:49] [main/INFO] [LaunchWrapper]: Calling tweak class net.minecraftforge.fml.common.launcher.TerminalTweaker [15:29:49] [main/INFO] [LaunchWrapper]: Launching wrapped minecraft {net.minecraft.client.main.Main} [15:29:50] [main/INFO] [minecraft/Minecraft]: Setting user: MistMaestro [15:29:56] [main/WARN] [minecraft/GameSettings]: Skipping bad option: lastServer: [15:29:56] [main/INFO] [minecraft/Minecraft]: LWJGL Version: 2.9.4 [15:29:57] [main/INFO] [FML]: -- System Details -- Details: Minecraft Version: 1.12.2 Operating System: Windows 10 (amd64) version 10.0 Java Version: 1.8.0_101, Oracle Corporation Java VM Version: Java HotSpot(TM) 64-Bit Server VM (mixed mode), Oracle Corporation Memory: 445135024 bytes (424 MB) / 568852480 bytes (542 MB) up to 3786407936 bytes (3611 MB) JVM Flags: 0 total; IntCache: cache: 0, tcache: 0, allocated: 0, tallocated: 0 FML: Loaded coremods (and transformers): GL info: ' Vendor: 'Intel' Version: '4.5.0 - Build 23.20.16.4973' Renderer: 'Intel(R) HD Graphics 530' [15:29:57] [main/INFO] [FML]: MinecraftForge v14.23.4.2732 Initialized [15:29:58] [main/INFO] [FML]: Starts to replace vanilla recipe ingredients with ore ingredients. [15:29:58] [main/INFO] [FML]: Replaced 1036 ore ingredients [15:29:58] [main/INFO] [FML]: Searching C:\ModdingWorkspace\run\assets\.\mods for mods [15:29:59] [main/INFO] [FML]: Forge Mod Loader has identified 5 mods to load [15:30:00] [main/INFO] [FML]: Attempting connection with missing mods [minecraft, mcp, FML, forge, examplemod] at CLIENT [15:30:00] [main/INFO] [FML]: Attempting connection with missing mods [minecraft, mcp, FML, forge, examplemod] at SERVER [15:30:00] [Thread-3/INFO] [FML]: Using sync timing. 200 frames of Display.update took 78672809 nanos [15:30:02] [main/INFO] [minecraft/SimpleReloadableResourceManager]: Reloading ResourceManager: Default, FMLFileResourcePack:Forge Mod Loader, FMLFileResourcePack:Minecraft Forge, FMLFileResourcePack:Example Mod, jabelar_resource_pack.zip [15:30:02] [main/INFO] [FML]: Processing ObjectHolder annotations [15:30:02] [main/INFO] [FML]: Found 1197 ObjectHolder annotations [15:30:02] [main/INFO] [FML]: Identifying ItemStackHolder annotations [15:30:02] [main/INFO] [FML]: Found 0 ItemStackHolder annotations [15:30:03] [main/INFO] [FML]: Configured a dormant chunk cache size of 0 [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.MainMod:preInit:101]: preInit() Example Mod [15:30:03] [Forge Version Check/INFO] [forge.VersionCheck]: [forge] Starting version check at http://files.minecraftforge.net/maven/net/minecraftforge/forge/promotions_slim.json [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModConfig:initConfig:47]: Example Mod config path = C:\ModdingWorkspace\run\assets\config\examplemod.cfg [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModConfig:initConfig:48]: Config file exists = true [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModConfig:syncConfig:67]: Allow unrealistic deconstruction = false [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModConfig:syncConfig:71]: Allow horse armor crafting = true [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModConfig:syncConfig:76]: Allow enchanted book deconstruction = true [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModConfig:syncConfig:81]: Allow partial deconstruction = true [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModTileEntities:registerTileEntities:30]: Registering tile entities [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModFluids:registerFluids:54]: Registering fluids [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModFluids:registerFluids:63]: Registering fluid: slime with bucket = true [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModNetworking:registerSimpleNetworking:42]: Registering simple networking [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModWorldGen:findFreeDimensionID:56]: Found free dimension ID = 2 [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.worldgen.WorldTypeCloud:<init>:44]: Constructing WorldTypeCloud [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.proxy.ClientProxy:preInit:144]: on Client side [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.proxy.ClientProxy$MouseHelperAI:<init>:462]: Constructing MouseHelper for AI bots [15:30:03] [Forge Version Check/INFO] [forge.VersionCheck]: [forge] Found status: OUTDATED Target: 14.23.4.2739 [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.proxy.ClientProxy:fixLocaleClass:163]: Swapping in custom locale class [15:30:03] [Forge Version Check/INFO] [forge.VersionCheck]: [examplemod] Starting version check at https://raw.githubusercontent.com/jabelar/ExampleMod-1.12/master/src/main/resources/versionChecker.json [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModBlocks$RegistrationHandler:onEvent:107]: Registering Blocks [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.blocks.BlockCompactor:<init>:61]: Constructing BlockCompactor instance [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.blocks.BlockCloud:<init>:47]: BlockCloud constructor [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.blocks.BlockCloudBedrock:<init>:42]: BlockCloud constructor [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.blocks.BlockParticleEmitter:<init>:47]: BlockParticleEmitter constructor [15:30:03] [main/INFO] [FML]: Applying holder lookups [15:30:03] [main/INFO] [FML]: Holder lookups applied [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModItems$RegistrationHandler:onEvent:74]: Registering items [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModBlocks$RegistrationHandler:registerItemBlocks:132]: Registering ItemBlocks [15:30:03] [main/INFO] [FML]: Applying holder lookups [15:30:03] [main/INFO] [FML]: Holder lookups applied [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModBiomes$RegistrationHandler:onEvent:52]: Registering biomes [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModEnchantments$RegistrationHandler:onEvent:46]: Registering Enchantments [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModEntities$RegistrationHandler:onEvent:63]: Registering entities [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModPotions$RegistrationHandler:onEvent:65]: Registering potions [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModPotions$RegistrationHandler:onTypeEvent:86]: Registering potion types [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModSounds$RegistrationHandler:onEvent:80]: Registering sound events [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModProfessions$RegistrationHandler:onEvent:63]: Registering villager professions [15:30:03] [main/INFO] [FML]: Applying holder lookups [15:30:03] [main/INFO] [FML]: Holder lookups applied [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModItems$RegistrationHandler:onModelEvent:94]: Registering item models [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModItems$RegistrationHandler:onModelEvent:109]: Registering custom item models [15:30:03] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModBlocks$RegistrationHandler:onModelEvent:170]: Registering block models [15:30:03] [main/INFO] [FML]: Applying holder lookups [15:30:03] [main/INFO] [FML]: Holder lookups applied [15:30:03] [main/INFO] [FML]: Injecting itemstacks [15:30:03] [main/INFO] [FML]: Itemstack injection complete [15:30:05] [Forge Version Check/INFO] [forge.VersionCheck]: [examplemod] Found status: UP_TO_DATE Target: null [15:30:08] [Sound Library Loader/INFO] [minecraft/SoundManager]: Starting up SoundSystem... [15:30:09] [Thread-5/INFO] [minecraft/SoundManager]: Initializing LWJGL OpenAL [15:30:09] [Thread-5/INFO] [minecraft/SoundManager]: (The LWJGL binding of OpenAL. For more information, see http://www.lwjgl.org) [15:30:09] [Thread-5/INFO] [minecraft/SoundManager]: OpenAL initialized. [15:30:09] [Sound Library Loader/INFO] [minecraft/SoundManager]: Sound engine started [15:30:17] [main/INFO] [FML]: Max texture size: 16384 [15:30:17] [main/INFO] [minecraft/TextureMap]: Created: 512x512 textures-atlas [15:30:18] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.client.models.ModelSlimeBag:bake:143]: Baking custom model [15:30:18] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModItems$RegistrationHandler:onModelEvent:125]: Models have been baked [15:30:19] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.init.ModRecipes$RegistrationHandler:onEvent:42]: Registering recipes [15:30:19] [main/INFO] [FML]: Applying holder lookups [15:30:19] [main/INFO] [FML]: Holder lookups applied [15:30:19] [main/INFO] [STDOUT]: [com.blogspot.jabelarminecraft.examplemod.MainMod:init:129]: init() Basically, I see it start pre-init, then do block registration (and objectholder injection), then item registration (and objectholder injection), then other registrations (like biomes) (and objectholder injection), then init. It seems that all the registration happens between pre-init and init.

In the generalized case, there is definitely a lot of possible situations of "circular" dependencies. But in the specific case of your own mod registry it is somewhat more manageable. For example if you need your registry to occur before blocks and items, you can do it in pre-init. If some other mod wants to use your registry they could access it in the block registry handling method -- I think you should be able to access other, previously populated registries during the vanilla events, right? So isn't the solution to the original poster is to make his registry in pre-init and instruct other mods that want to use it to interact with it at the beginning of their block registry event handling method?

I'm not sure exactly what you mean, but I'm guessing you want to be able to have separate chat with specific people? The methods would be first of all you would make another GUI like the current chat one. Then you would make a custom packet to communicate the chat messages. Lastly you would add a command to start the chat.

Yep, there's a lot of that in Minecraft modding...

The reality is a lot of modders are going to copy vanilla blocks or extend them. They will see that the getDrops() methods call the damageDropped() method. I feel that people will continue to use damageDropped() because the same logic (of converting select properties to metadata) is needed in two places - for getDrops() and for getPickBlock(). If you have code/logic needed in two places isn't it good practice to have a common method to contain it? You're right though that the recommended way should be to use getDrops() and getPickBlock(). But I don't think damage dropped is legacy crap -- it is a legitimate place to put the state conversion.

It is not BS at all. The getPickBlock() relies on damage dropped method. The code I posted is from that actual method. I didn't make it up. If you don't override the damage dropped method in your modded block, getPickBlock() will not work for you. Here's the code calls directly from source: public ItemStack getPickBlock(IBlockState state, RayTraceResult target, World world, BlockPos pos, EntityPlayer player) { return getItem(world, pos, state); } public ItemStack getItem(World worldIn, BlockPos pos, IBlockState state) { return new ItemStack(Item.getItemFromBlock(this), 1, this.damageDropped(state)); }

Modded blocks should always override the damaged block return value. If your modded blocks have properties but don't do that then they won't work right as item blocks.

Sorry haven't had time to write it up yet. I should say that it turned out to be a bigger effort than I thought because Minecraft code is kinda messy and it copies some of the renderers to other classes before you get an opportunity to replace it with reflection. So, for example, you have to then modify every biped renderer for entities that can wear armor (EntityZombie, EntityWitherSkeleton, EntityGiantZombie, and so forth). But I finally got it all working I think (could use more testing). Actually I think the Elytra effect is also separate so I better check if that is working. Overall it is fairly simple approach but sort of difficult to get the details right.

getPickBlock() uses the same code I posted in my corrected suggestion. That is the function that sorts out which metadata should be passed to item. For some blocks like flowers it might be the same for both damage dropped and metadata, but for others like logs the orientation doesn't need to pass into the item.