[1.12.2] Help with a "mobile furnace"

[LoganDark]

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 July 6, 2019 by LoganDark
Added 'Notes for each thingy'

[LoganDark]

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 July 6, 2019 by LoganDark

[Cadiboo]

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?

[LoganDark]

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 July 6, 2019 by LoganDark

[LoganDark]

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.

[LoganDark]

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 July 6, 2019 by LoganDark

[LoganDark]

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 July 6, 2019 by LoganDark

[LoganDark]

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?

[LoganDark]

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.

 

 

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 July 6, 2019 by LoganDark

[LoganDark]

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 July 7, 2019 by LoganDark

[LoganDark]

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 July 8, 2019 by LoganDark

[LoganDark]

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 July 8, 2019 by LoganDark

[LoganDark]

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 July 8, 2019 by LoganDark

[LoganDark]

37 minutes ago, LoganDark said:

I can make the container take FurnaceCapability instead of IItemHandler. Working on it...

Commit

Edited July 8, 2019 by LoganDark

[Draco18s]

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.