Leaderboard

update forge, it got changed public static void openGui(EntityPlayerMP player, IInteractionObject containerSupplier, Consumer<PacketBuffer> extraDataWriter)

What: Not using an object base class.Why: Using an object base class (commonly called BlockBase or ItemBase) is unnecessary and is an anti-pattern. There is already a BlockBase class, it’s the minecraft Block class. Making a class just to have its children inherit default implementations of methods goes against the OOP principle of Composition over Inheritance.Consequences: Using a class like this stops you from extending other classes and because lots of minecraft code uses instanceof checks to specially handle logic, you are likely to encounter weird and hard-to-fix bugs.How: Instead of putting all your common logic in one class and extending it, extract the logic to utility methods. For example: Instead of calling setRegistryName, setTranslationKey, etc. inside your object base’s constructor, extract it to a helper method and call it on every object when you create it. In this example setup calls the above methods. registry.register(setup(new CustomItem(), "custom_item")); What: Not using static initialisers.Why: Using static initialisers does not allow you to control when your objects are created or the order in which your objects are created.Consequences: Using static initialisers prevents other mods from overriding your objects, prevents forge from being able to dynamically load/unload mods and can cause weird, unreproducible crashes.How: Use the @ObjectHolder annotation if you need static references to your objects and create & register your objects in the appropriate registry event. (Note: before registry events Modders created and registered their items in preInit. This practice became out of date in 1.7 with the introduction of registry events, and is no longer possible in 1.13). What: Not using an interface to register models.Why: This interface (commonly called IHasModel) is unnecessary. All items need models and nothing about model registration requires private or protected data.Consequences: This interface makes you write 4 lines of code in each class and 2+ lines of code in your registry event, when 1 or 2 lines of code could accomplish the exact same thing. It also leads to weird bugs when you forget to make your object implement the interface.How: Simply register each model in the registry event (1 line of code for each model) or write a loop that does it for you (1 or 2 lines depending on the implementation). For example: Write out registerModel(item, meta, variant) for each item and variant or write a loop like this for (Item item : allModItemsAndItemBlocks) registerModel(item, meta, variant); A list of all your items can be acquired in many ways, such as looping over registries and checking domain, keeping your own list, looping over registries and using an instanceof check etc. Read more code common issues & solutions at https://gist.github.com/Cadiboo/fbea89dc95ebbdc58d118f5350b7ba93

Instead of constructing the PacketBuffer, writing to it and then calling openGui, simply call openGui with a lambda function that writes to the supplied PacketBuffer. Forge constructs the PacketBuffer for you.

Your most recent few posts with these pre-written snippets of advice have been poorly formatted on the dark theme: To fix it, you should be able to use the Tx (Remove Format) button and then re-apply the code formatting to the necessary parts. Normally I'd PM you this, but unfortunately the forum wants me to delete a whole lot of PMs in my inbox before I can send another one.

NetworkHooks#openGui() should take three arguments, and you're only passing two. The third argument is a PacketBuffer, where you send any extra data you need on the client side, including in your case the tile entity position. E.g.: NetworkHooks.openGui((EntityPlayerMP) player, yourContainerProvider, buf -> buf.writeBlock(tePos)); Edit: third parameter is now a Consumer<PacketBuffer> as @loordgekpointed out. That changed in the last 24 hours, time to update

public boolean onBlockActivated(IBlockState state, World worldIn, BlockPos pos, EntityPlayer player, EnumHand hand, EnumFacing side, float hitX, float hitY, float hitZ) This is how Block#onBlockActivated is defined. You are not overriding it as your parameters are wrong.

replaced by ForgeConfigSpec

“Patching” as you are calling it is done in memory and as such is not stored in a file, though you would still need to load it back up the next time you started the game anyway, which would still take the same amount of time anyway

IC2 is the name of a mod, its modid is industrialcraft2. TiC is the name of a mod, its modid is tconstruct. Reason being short modids are a bad problem because there may be another mod out there with the id rc, ab, or cd. This would cause forge's mod loader system to crash. You are programming get used to writing a lot.