Jump to content
Search In
  • More options...
Find results that contain...
Find results in...

[1.12.2] Help with a "mobile furnace"


LoganDark
 Share

Recommended Posts

So I'm trying to make a silly "mobile furnace".

 

I have it mostly functional, but (inventory) items don't update on the client unless you reopen the GUI, and I feel like I'm doing something horribly, deeply wrong, as this should probably not be the case for any use of Container/IInventory (because detectAndSendChanges). I've been studying the TileEntityFurnace/TileEntityChest classes a LOT trying to figure out what to do here. But it's really really hard to do things, since there are no instances of items, so I can't use class variables or whatever. That's fine, I'll just load from NBT and then save it again in onUpdate. Which is bad, I hate it.

 

So, I'm really, badly needing a code review. I need someone to look at my code, figure out what's wrong with it, and please please teach me how to do it right. I never thought something as simple as an item furnace could be so complicated...

 

The simplest way for you to see how things are, my code, and etc. is to download the project and run it for yourself. ./gradlew setupDecompWorkspace idea genIntellijRuns and so on. The project's on GitHub.

 

Here are some quick links to the Kotlin files, if you want to take a look without downloading it:

ItemPotatoFurnace (the Item)

ContainerPotatoFurnace (the ContainerFurnace)

GuiPotatoFurnace (the GuiFurnace)

InventoryPotatoFurnace (the IInventory)

 

If you can teach me how to use capabilities for items, that would be great too. The forge docs are SO confusing on capabilities, I wouldn't even know how to use them for blocks.

 

Basically, I'm just really looking for help on anything. If you can, teach me how to keep the items synchronized on client and server. Anything else would definitely be appreciated. I started modding about 2 days ago, never touched the MDK ever before that. This is also my first time with Kotlin, hehe.

 

Notes for each thingy:

  • The item: This isn't listed in a creative tab like the other items so you'll have to use /give @p diamond2potato:potato_furnace. Right click to open the furnace, obviously. Use it like a normal furnace. It'll keep smelting as long as it's in your inventory, I think. I haven't checked if onUpdate gets called in a chest.
  • The container: It has a logger left over from when I made sure this was being called, it's weird. You can ignore it, or remove it (which would be nice for keeping your logs clean)
  • The GUI: Just a GuiFurnace. I made it its own class in case I need to change anything, but I don't think I'll need to.
  • The inventory: This is where I think my error is. There's, of course, the item implementation which mandates some sort of bad practice due to the way it handles things (advice would be appreciated by the way), but this inventory is probably the source of all my problems. I would love to use capabilities but I'm just not sure how. Please advise...
Edited by LoganDark
Added 'Notes for each thingy'
Link to comment
Share on other sites

  • Problematic code, issue 5, 10, 14 (yes, Kotlin's object is effectively using a static initializer. Your items must be regular classes and you must instantiate them during a forge-controlled event).
  • Assigning values to fields annotated with @ObjectHolder makes no sense.
  • Why are you injecting vanilla Minecraft items using @ObjectHolder? Just use constants in Minecraft's Items class. Same for your use of Block.getBlockFromName, just use the constants in Blocks.
  • Please use JSON recipes, do not register them in code.
  • Just use an if statement...
  • Please do not create unregistered item instances (in ItemHoeFood and friends).
Link to comment
Share on other sites

  1. Issue 5 was detailed in my post, read it. If you want me to use capabilities you're going to have to tell me how they work because I don't know how to put capabilities in an item. Issue 10, point taken, I'm in the process of fixing it and pushing it to GitHub. Issue 14, at first I didn't understand what you meant, but then I realized you didn't mean registering them, but actually instantiating them (like ItemPotatoPickaxe and such).
  2. I only do that so I don't have to add ? to all the types. I know Forge replaces them at runtime using reflection, which is why I assign dumb values to them, knowing they'll change. Kotlin has no "guaranteed to be injected at runtime" annotation, unfortunately. And there's a difference between nullable and non-nullable types.
  3. I wasn't actually aware that Items existed until a while ago.
  4. Point taken
  5. I do that so I can use val rather than var, but still, point taken. Kotlin has no ternary which is what I would've used.
  6. These are not item instances, they're classes, like ItemHoe or ItemFood. ItemHoeFood is an ItemHoe that can be eaten. It's meant to be subclassed, not registered. Then the subclasses registered. See ItemPotatoHoe.
Edited by LoganDark
Link to comment
Share on other sites

Just now, LoganDark said:

Kotlin has no "guaranteed to be injected at runtime" annotation, unfortunately

Doesn’t it exist and isn’t it called “lateinit” or something?

About Me

Spoiler

My Discord - Cadiboo#8887

My WebsiteCadiboo.github.io

My ModsCadiboo.github.io/projects

My TutorialsCadiboo.github.io/tutorials

Versions below 1.14.4 are no longer supported on this forum. Use the latest version to receive support.

When asking support remember to include all relevant log files (logs are found in .minecraft/logs/), code if applicable and screenshots if possible.

Only download mods from trusted sites like CurseForge (minecraft.curseforge.com). A list of bad sites can be found here, with more information available at stopmodreposts.org

Edit your own signature at www.minecraftforge.net/forum/settings/signature/ (Make sure to check its compatibility with the Dark Theme)

Link to comment
Share on other sites

1 minute ago, Cadiboo said:

Doesn’t it exist and isn’t it called “lateinit” or something?

Only works on vars (which are then forever cursed with the absence of a non-null guarantee, requiring me to use !! everywhere I use it), and I strongly prefer using vals. I'll look into it.

Edited by LoganDark
Link to comment
Share on other sites

31 minutes ago, LoganDark said:

Only works on vars (which are then forever cursed with the absence of a non-null guarantee, requiring me to use !! everywhere I use it), and I strongly prefer using vals. I'll look into it.

lateinit is exactly the thing to do here. No, it does not require you to make your field nullable.

 

35 minutes ago, LoganDark said:

Kotlin has no ternary which is what I would've used. 

if-else is an expression in Kotlin, just like when:

val foo = if (a == b) "true" else "false"

Or even more complex:

val foo = if (a == b) {

    // do stuff here

    "foo's value here"

} else {

    // do more other stuff

    "foo's more different value here"

}

 

Capabilities on items work by overriding initCapabilities on your Item class.

 

36 minutes ago, LoganDark said:

These are not item instances, they're classes, like ItemHoe or ItemFood. ItemHoeFood is an ItemHoe that can be eaten. It's meant to be subclassed, not registered. Then the subclasses registered. See ItemPotatoHoe.

They instantiate ItemFood inside them, which is then used (you are calling methods on it) without it being registered. This is a bad idea.

Link to comment
Share on other sites

Just now, diesieben07 said:

lateinit is exactly the thing to do here. No, it does not require you to make your field nullable.

 

if-else is an expression in Kotlin, just like when:

val foo = if (a == b) "true" else "false"

Or even more complex:

val foo = if (a == b) {

    // do stuff here

    "foo's value here"

} else {

    // do more other stuff

    "foo's more different value here"

}

 

Capabilities on items work by overriding initCapabilities on your Item class.

 

They instantiate ItemFood inside them, which is then used (you are calling methods on it) without it being registered. This is a bad idea.

Sorry, didn't know if-else could be that small, heh...

 

And, well, I instantiated ItemFood because I didn't want to reimplement everything (even if it is just copy and paste).

 

Also, where would I put the lateinit? Would I store all items in their own variables under my mod class and register them all individually? Put them in companion objects and use reflection to set it during the registration step?

 

Also, sure I can override initCapabilities but what do I put in it? The docs are completely unclear on this.

Link to comment
Share on other sites

Just now, LoganDark said:

And, well, I instantiated ItemFood because I didn't want to reimplement everything (even if it is just copy and paste).

Unfortunately that's what you will have to do. In 1.14.3 this changes.

 

Just now, LoganDark said:

Also, where would I put the lateinit? Would I store all items in their own variables under my mod class and register them all individually? Put them in companion objects and use reflection to set it during the registration step?

On your @ObjectHolder fields. Forge will then automatically inject them after registration. The documentation has more information on this.

 

1 minute ago, LoganDark said:

Also, sure I can override initCapabilities but what do I put in it? The docs are completely unclear on this.

You need to return an ICapabilityProvider, which then exposes one or more capability, just like described for tile entities, etc. in the documentation.

Link to comment
Share on other sites

11 minutes ago, diesieben07 said:

Unfortunately that's what you will have to do. In 1.14.3 this changes.

 

On your @ObjectHolder fields. Forge will then automatically inject them after registration. The documentation has more information on this.

 

You need to return an ICapabilityProvider, which then exposes one or more capability, just like described for tile entities, etc. in the documentation.

  1. Isn't an item just a data structure, with methods and such? I don't see anything wrong with instantiating one without registering it, but I'll see if I can remove that.
  2. So I need an ObjectHolder for every item in my mod rather than doing it the way I currently do? It probably doesn't make any difference if I instantiate my singleton items before or during registration. Items don't do anything on their own when instantiated, right? Please do clarify on this though, what could go wrong if I keep doing it the way I currently do?
  3. How would I keep a separate instance of the Capability for each item? I have no idea how to do this other than instantiating it every tick in onUpdate. Unless something else handles what I return from initCapabilities? I'm confused, you need to provide more info than "read the docs" because the docs don't actually detail this. They just say "make an instance of the capability and override get/hasCapability". Read it yourself and then tell me how useful that is!
Edited by LoganDark
Link to comment
Share on other sites

10 minutes ago, LoganDark said:

So I need an ObjectHolder for every item in my mod rather than doing it the way I currently do?

No. You can assign the fields yourself if you want to.

 

10 minutes ago, LoganDark said:

It probably doesn't make any difference if I instantiate my singleton items before or during registration. Items don't do anything on their own when instantiated, right? Please do clarify on this though, what could go wrong if I keep doing it the way I currently do?

setRegistryName for example does not work correctly when called outside of "mod context". A static initializer has no set time when it will run, so there is no guarantee that Forge can determine its your mod that is executing code. setRegistryName however needs to know which mod is using it.

 

10 minutes ago, LoganDark said:

How would I keep a separate instance of the Capability for each item? I have no idea how to do this other than instantiating it every tick in onUpdate. Unless something else handles what I return from initCapabilities? I'm confused, you need to provide more info than "read the docs" because the docs don't actually detail this. They just say "make an instance of the capability and override get/hasCapability". Read it yourself and then tell me how useful that is!

Did you read the javadoc on initCapabilities? It explains that the resulting ICapabilityProvider is tied to the given ItemStack. Your item is queried again for every ItemStack that's created of it.

Link to comment
Share on other sites

5 minutes ago, diesieben07 said:

No. You can assign the fields yourself if you want to.

So I need a field for every item in my mod? I have no issue with ObjectHolder. I have an issue with having to manually create variables for each item and assign an instance to it manually for every single item in the mod. I'm not saying I WOULDN'T do it, I'm just trying to figure out why exactly I SHOULD do it, rather than what I'm doing right now, which is adding an entry to an array and calling it good.

 

5 minutes ago, diesieben07 said:

setRegistryName for example does not work correctly when called outside of "mod context". A static initializer has no set time when it will run, so there is no guarantee that Forge can determine it's your mod that is executing code. setRegistryName however needs to know which mod is using it.

Could you give me an example of it not "working correctly"?

 

5 minutes ago, diesieben07 said:

Did you read the javadoc on initCapabilities? It explains that the resulting ICapabilityProvider is tied to the given ItemStack. Your item is queried again for every ItemStack that's created of it.

Okay, thank you. Sorry I haven't read the javadoc...

 

I'm going to go try things now. Now that I have the mod in version control, I can go crazy. I'll provide an update if things change.

Edited by LoganDark
Link to comment
Share on other sites

1 minute ago, LoganDark said:

Could you give me an example of it not "working correctly"?

If Forge cannot determine it's your mod that's executing code your Item's registry name will now be "minecraft:myawesomeblock" and it will be tracked as being a vanilla Minecraft item.

 

2 minutes ago, LoganDark said:

So I need a field for every item in my mod?

No. If you do not need a reference to your items it is fine to just create an instance, register them and then move on. You do not need to keep the instance around if you do not need it. It will still be in Forge's registry.

Link to comment
Share on other sites

3 minutes ago, diesieben07 said:

If Forge cannot determine it's your mod that's executing code your Item's registry name will now be "minecraft:myawesomeblock" and it will be tracked as being a vanilla Minecraft item.

ResourceLocations have a namespace argument that's always set to my mod ID, that's what Forge uses for the namespace, right?

 

9 minutes ago, diesieben07 said:

No. If you do not need a reference to your items it is fine to just create an instance, register them and then move on. You do not need to keep the instance around if you do not need it. It will still be in Forge's registry.

Oh, wait, so I can use ObjectHolders in each class that uses them...? Or Item.getByNameOrId?

Link to comment
Share on other sites

1 minute ago, LoganDark said:

ResourceLocations have a namespace argument that's always set to my mod ID, that's what Forge uses for the namespace, right?

In this case, yes. However you should not rely on this working properly without the mod context being set. It may or may not break things in the future. To be honest, I have not explored all the ways in which it can possibly break things. Please just don't do it and follow the proper procedures for registering game elements.

 

2 minutes ago, LoganDark said:

Oh, wait, so I can use ObjectHolders in each class that uses them...? Or Item.getByNameOrId?

You can use as many object holders as you want, yes. You can also use the registries in ForgeRegistries, don't use the legacy things like Item.getByNameOrId. Also note that a lookup by name is always less clean: It requires hardcoded strings everywhere and is also slower than just using a field.

Link to comment
Share on other sites

Okay, so I did some updates.

 

First, I'm now using Items and Blocks for almost all vanilla items/blocks (minus potatoFood and bakedPotatoFood in the Diamond2Potato class, to not have to cast them to ItemFood every time I use their ItemFood methods). Second, I've moved the Potato Furnace over to Capabilities.

 

bc93b1659a.gif

 

Now, the new Capability-based furnace seems to work okay, except for one problem. It doesn't seem to save, and items disappear randomly if you drop the item, move it around, etc. Now, I've defined the Capabilities.IStorage methods in my capability, but it doesn't seem to do anything on its own. So, my question now is, how do I get it to save? Should I just save it manually using the stack's tag compound using a 'mark dirty' method of some sort?

 

Additionally, how do I keep this in sync between client and server? Does it just happen automatically, or is there something specific I should do?

 

Capabilities commit

Edited by LoganDark
Link to comment
Share on other sites

Okay, so after some more work I've made the potato furnace almost fully functional in singleplayer. You can move the item around in your inventory, drop it on the ground, etc. and pick it back up and the state persists.

 

GitHub commit

 

It still doesn't work in multiplayer. Even in singleplayer, sometimes when, for example, there's 16 Potatoes in the item slot and you try to drop 16 on top, the 16 extra disappears.

 

Any more advice on how to make this work?

Edited by LoganDark
Link to comment
Share on other sites

2 hours ago, diesieben07 said:

Why do you have an adapter to IInventory? You don't need this.

What do I use instead?

2 hours ago, diesieben07 said:

on your TE

What TE? Is an ItemStack a TileEntity? No, so why would you mark it dirty? I'm pretty sure it saves on its own...

 

--------------------------------------------------------------------

 

Please see my new post here:

 

This one is obsolete.

 

Edited by LoganDark
Link to comment
Share on other sites

24 minutes ago, LoganDark said:

What do I use instead?

IItemHandler. You already have it.

 

25 minutes ago, LoganDark said:

What TE? Is an ItemStack a TileEntity? No, so why would you mark it dirty? I'm pretty sure it saves on its own...

Yeah I derped and forgot this is ItemStack based.

Link to comment
Share on other sites

2 minutes ago, diesieben07 said:

IItemHandler. You already have it.

How do I use that for the GUI then? Is there a type of GUI slot that accepts IItemHandlers? Because the regular GUI Slot only accepts IInventories. I need the adapter, even though I use IItemHandler behind the scenes.

Edited by LoganDark
Link to comment
Share on other sites

43 minutes ago, diesieben07 said:

SlotItemHandler.

Even that uses IInventory LOL

 

Besides, I need get/setField. Thanks for the recommendation, though, I'll be sure to use that next time if I need something like a chest.

 

Edit: I'm dumb, I can make the container take FurnaceCapability instead of IItemHandler. Working on it...

Edited by LoganDark
Link to comment
Share on other sites

4 hours ago, LoganDark said:

Even that uses IInventory LOL

You mean this line?

private static IInventory emptyInventory = new InventoryBasic("[Null]", true, 0);

Yeah, that's so it can call super without passing null, which would crash. It doesn't actually USE it, its literally a dummy variable.

Apparently I'm a complete and utter jerk and come to this forum just like to make fun of people, be confrontational, and make your personal life miserable.  If you think this is the case, JUST REPORT ME.  Otherwise you're just going to get reported when you reply to my posts and point it out, because odds are, I was trying to be nice.

 

Exception: If you do not understand Java, I WILL NOT HELP YOU and your thread will get locked.

 

DO NOT PM ME WITH PROBLEMS. No help will be given.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share



×
×
  • Create New...

Important Information

By using this site, you agree to our Privacy Policy.