Search the Community

So, you kw that function call you put in the method created by IHasModel? The one to your proxy. Copy that line. Go to where you have your model registry event handler method where you cast items in a list to IHasModel. Take that line with the cast and delete it. You can also remove the instanceof check. Paste the line you copied. Replace this with the loop iterator (probably item). Delete IHasModel and all references to it. Tada, you're done.

Stop using IHasModel. Code Style Issue #3 (and #1) See also Problematic Code #7 and possibly #14 Want to read more about IHasModel? I've posted the same thing like 30 times elsewhere. I'm getting sick of it. http://www.minecraftforge.net/forum/search/?&q=IHasModel&type=forums_topic&author=Draco18s&search_and_or=or&sortby=relevancy

Also problematic code #7. Additionally, IHasModel is stupid. ALL items need models. The IHasModel interface does fuckall but make you write the same line of code over and over again when you could instead do this: @SubscribeEvent public static void onModelRegister(ModelRegistryEvent event) { for(Item item : ModItems.ITEMS) { Main.proxy.registerItemRenderer(item, 0, "inventory"); } } And I'll also point out that the ModelRegistryEvent is client side only and so this event should already be in your Client Proxy, which means you can do this: @SubscribeEvent public static void onModelRegister(ModelRegistryEvent event) { for(Item item : ModItems.ITEMS) { ModelLoader.setCustomModelResourceLocation(item, 0, new ModelResourceLocation(item.getRegistryName(), "inventory") } } Hey @diesieben07 can we add the stupidness of IHasModel to the list of Code Style issues? I've probably said "this is dumb" like 14 times now.

No one cares, this isn't a chat room. Anyway, ok, that looks less awful than I'd originally thought. ModelRegistryEvent is client-side only. It can't live here. IHasModel is a fucking stupid convention. Seriously. ALL ITEMS HAVE MODELS Your IHasModel does nothing to insure that your block class registers an ItemBlock for itself, leading to a possible crash if you forget. The method your IHasModel interface supplies just redirects to your client proxy with no additional information from the block/item. proxy.registerModel(item, item.getRegistryName(), "inventory") will work just fine (or if the code is already in the client proxy--see #1--then you don't need the proxy. part...or really, you just just call ModelLoader directly).

You've done something horribly wrong if you're attempting to register a model for Air. (By the way, hint: you do not need an IHasModel interface: all items need a model and they're all done the exact same* way: proxy.registerRenderer(item,0,new ResourceLocation(item.getReigstryName().toString(), "inventory") and any block that has an item renderer will already register its item renderer due to the loop over the ModItems.ITEMS list, which contains the item form of the block already, if you don't cast to IHasModel). https://github.com/Draco18s/ReasonableRealism/blob/1.12.1/src/main/java/com/draco18s/hardlib/client/ClientEasyRegistry.java#L197-L199 The array there is just a custom object to hold an Item, Metadata, and String resource location, created during preInit and held until the ModelRegistryEvent. There's nothing special there. *Except when they don't, in which case your IHasModel hasn't really saved you anything.

1) You don't need IHasModel at all. All items need models. ALL OF THEM. ALL OF THEM NO EXCEPTIONS and the data required to register one is public. Blocks get their models registered automatically (this is true of items as well come 1.13). Its an antipattern because you will never have an item that does not have a model, so utilizing an interface to supply it is worse than useless (your blocks that require IHasModel? They have ItemBlocks and ItemBlocks are items and require a model). You're actually writing more code than not using the interface. 2) block instanceof BakedModelProvider is bad. BakedModelProvider is almost certainly client side only (or references something that's client side only) and will crash the dedicated server

1) You do not need a class called BlockBase or ItemBase because "base" block and item classes already exist. They're called Block and Item. A "base" class that saves you from having to write a few lines of code over and over again is an anti-pattern. What happens if you want to write your own Fence block? Well, now you can't inherit from BlockBase because BlockBase isn't a Fence and you'd have to copy all of that code. But you can't inherit from FenceBlock because now you have to write all your BlockBase code again. 2) You do not need IHasModel. Two very very simple reasons. One: all items need models, no exceptions. Blocks that need item models also (gasp!) have items and all items need models. Two, the information that you expose in the methods specified by IHasModel are already public. So you perform a completely pointless instanceof and class cast to call a method that does nothing useful. See this method: https://github.com/LotteWiltshy/lottycraft-mod/blob/master/LottyCraft/src/main/java/com/lotty/lottycraft/util/handler/RegistryHandler.java#L21-L30 Watch this: @SubscribeEvent public static void onModelRegister(ModelRegistryEvent event) { for(Item item : ModItems.ITEMS) { Main.proxy.registerItemRenderer(item, 0, "inventory"); } } HOLY SHIT BALLS BABY JESUS, IT FUCKING WORKS Except, for you know, the ModelRegistryEvent is SideOnly(CLIENT) so you can't actually have this here.... Oh, also: https://github.com/LotteWiltshy/lottycraft-mod/blob/master/LottyCraft/src/main/java/com/lotty/lottycraft/init/ModItems.java#L13-L14 Problematic Code #14

I'm almost certain that Loremaster didn't start the IHasModel fiasco. Either way, I've commented on his video about IHasModel. He's literally the last person that should be teaching others how to mod because he, himself, is still learning. Blind leading the blind, much?

Stop using IHasModel. All items have models. All of them. Yes, ALL of them. And they all need the same registration code for the most part and none of the information required to register a model is private. Stop uisng BlockBase. There's already a "base" block class, its called Block. Using a super class to avoid repeating yourself is not what the class hierarchy is for. You want to stay DRY? Do things right and put the code that registers your models inside the ModelRegistryEvent and stop using IHasModel. Get rid of the need for IMetaName. With 1.13 around the corner you shouldn't be using metadata to define completely different blocks (aka The Flattening), so there's no reason to go around doing that now, there's no longer a (functionally relevant) block limit any more, as everything is referenced by the registry name (a string). Anyway, this is how I registered blocks that had variant states: https://github.com/Draco18s/ReasonableRealism/blob/1.12.1/src/main/java/com/draco18s/hardlib/client/ClientEasyRegistry.java#L63 (or the version on L75 if you have a custom state mapper) I didn't bother constructing the variant string manually (like you're doing), but used the exact same method calls that the game uses.

Hmm. "The IHasModel is not necessary. All items require models that must be registered and the registration can be done without any non-public information from the Item instance. The same goes for blocks: not all blocks have items but the ones that do, follow the same rule. IHasModel was created and perpetuated through a series of uninformed tutorials and has become an example of Cargo Cult Programming." May be worth noting that ModelRegistryEvent is client-side-only as a Problematic Code separately. "The ModelRegistryEvent is in the Forge client namespace. This means that it may not be present on the dedicated server. As such, it should only be used inside a Client Proxy class to avoid issues."

Your Durmstrium ladder does not implement IHasModel, so you never register a model for it. (And this is why I hate the IHasModel method of supplying models. There are so much better ways. Hint: fucking everything needs a model registered... except when it doesn't)

IHasModel is completely and utterly pointless. All it does is make you write the same 4 lines of code all over again for every item so that you can pass public values to a public function.

Code Style #3, I'd be willing to bet you didn't tag your item class with IHasModel (and this is one of the reasons its stupid).

Is that IHasModel? Stop using it. It does nothing except make you retype the same lines over and over. All items need models, yes, ALL ITEMS which means that the interface is not special. Now if the interface is because the item has a custom mesh definition, that's fine (that's special). This means that the model you want isn't being registered and baked. This is common for custom mesh definition models. This is the chunk I use: https://github.com/Draco18s/ReasonableRealism/blob/1.12.1/src/main/java/com/draco18s/hardlib/client/ClientEasyRegistry.java#L184-L195 (and a mesh definition: https://github.com/Draco18s/ReasonableRealism/blob/1.12.1/src/main/java/com/draco18s/industry/item/ItemCastingMold.java#L89 ) It looks like the code you posted should perform similar operations on the right data.

Less "bad" and more "completely pointless." The only bad thing about it is that it violates the DRY principle ("don't repeat yourself") making you type the same 3 lines over and over again, as well as obfuscating how/if your models are being registered. Its not that it doesn't work, but that when it fails, its harder to figure out why (as well as serving no purpose: all items need models and none of the information needed to register one is private (but this CAN change in some circumstances--eg custom mesh definitions--and THAT'S what an interface is good for)). (Note that I'm the one that started the crusade against IHasModel and I'm pretty sure you're echoing what someone else said about it, and not what I've said. ) Other than that, pretty accurate post

Also, scrap IHasModel. All items need models. ALL OF THEM. ALL OF THEM. And none of the data needed to register one is private.

for now. That said, the "code style" type issues are ones that are just bad practice, not that they cause the game to break. In some cases it makes things more complicated than they need to be (IHasModel) and in some cases simplifies something that shouldn't be (Proxies).

Well... 1) Your blockstate file does not contain an inventory variant 2) You didn't show your ModelRegistryEvent Related: IHasModel is stupid and pointless (I will beat this out of you cargo cult programmers eventually). ALL items need models and all the information necessary to register a model is public. Code Style #3. Problematic Code #10.

You're current registering the item with a "" variant. You should use " inventory" Also IHasModel is stupid. All items need models and none of the information necessary to register a model is private. ItemBase is an antipattern:

You don't need this. IHasModel is stupid. All items need models and all the information needed to register a model is public. Your log is useless, there are no errors. Not sure why the cart won't pass.

Your blockstatecontainer only allows for 2 variants. If we look at your enum class, there are 9 total variants. This can't fit in 4 bits of metadata along with 2 bits used for log axis. Which we see: https://github.com/TheSlayerMC/Journey-1.12/blob/master/main/java/net/slayer/api/block/BlockModLog.java#L55 https://github.com/TheSlayerMC/Journey-1.12/blob/master/main/java/net/slayer/api/block/BlockModLog.java#L96 BTW, your API has references to your non-API. This means your API cannot be packaged separately. https://github.com/TheSlayerMC/Journey-1.12/blob/master/main/java/net/slayer/api/block/BlockModLog.java#L7-L15 Also, IHasModel is stupid. All items need models and none of the information necessary to register a model is protected or private.

Not sure what's going on there. But. IHasModel is stupid. ALL items need models and the information required to register one is all public.

You do not need it. Anything ItemBase does you can do without ItemBase. setRegistryName? Done: Item someItem = new Item().setRegistryName("some_item") Creative tab too? Item someItem = new Item().setRegistryName("some_item").setCreativeTab(...) Adding it to your ModItems.ITEMS list? ModItems.ITEMS.add(someItem) registerModels() from IHasModel? Another anti-pattern. Just do this in your client proxy's ModelRegistryEvent: for(Item item : ModItems.ITEMS) { ModelLoader.setCustomModelResourceLocation(item, o, new ModelResourceLocation(item.getRegistryName(), "inventory")); }

The only other thing I suggest you do is get rid of the IHasModel interface, if you have one per Code Style #3. Your ModelRegistryEvent handler can do all of the work just fine without having to cast and request.

Also: IHasModel is stupid. All items need models. ALL ITEMS. ModelRegistryEvent is client side only. This will crash the dedicated server. This can be done externally (i.e. not inside the class) just fine: Technium.proxy.registerItemRender(item, 0, "inventory"); If your item classes look like this, you don't need them at all. The fuck? You used the IProxy interface (good) method and a common proxy (bad)?