Machine Block HELP [1.12]

[gameristic34]

So I have been having a series of issues when 'learning' to make a machine block, started just trying to look at the furnace block but it wasn't what i wanted to look for as I wanted extra slots and turned to the internet for a tutorial, followed it and then took things apart and made it 'work' the way I wanted it to and added and some extra. I have already taken my jank code from then and made a new project (this one) and worked it in only using the former as reference in most cases. It 'works' but has several issues I've spent a month on now trying to resolve with out a new issue popping up and sadly as smart as I am I used the same repository as the last one so I lost my reference.

Just to go over of how the block is supposed to function:

• Consume any Iron or Item crafted with Iron (slot[0])
• Damage an item in the second slot (slot[1])
• and use redstone dust and blocks as fuel
• fill the side bar before spitting out the output

I have succeeded in this but the main issues i can't figure out are:

• First slot stack resets when you open the container after closing
• NBT data of tile entity isn't saving as it should

I'm ignoring the transfer stack issues as it's not relevant to this (shouldn't be) and believe this is NBT related and I'm not calling markDirty() where I should.

 

Apart from the two bigger issues above I have a mess of typos and no item textures but I just want to figure out how to solve these two issues first and i'll likely for the sanity of cleaner code rewrite this from the ground up as not much has been done yet and I'll have a clear direction.

 

GitHub

[V0idWa1k3r]

Why did you commit compiled class files to github? You do realize we can't open them and look at your code right? Git is where you upload your sources, not your compiled application.

As a side note - that is not how you setup a minecraft mod repository. The root directory must be your workspace directory.

[gameristic34]

  On 10/23/2018 at 6:16 PM, V0idWa1k3r said:

Why did you commit compiled class files to github? You do realize we can't open them and look at your code right? Git is where you upload your sources, not your compiled application.

As a side note - that is not how you setup a minecraft mod repository. The root directory must be your workspace directory.

Expand  

fixed it, used the bin folder and not the src

[V0idWa1k3r]

There are a lot of isRemote checks in your code like this one

if (this.world.isRemote)

However this check ensures that whatever happens afterwards happens on the client and not on the server. As the server is the driving logic this makes sure nothing actually happens, only the client sees something changed. Untill the server is forced to sync the items to the client that is. This is the reason for

  On 10/23/2018 at 6:07 PM, gameristic34 said:

• First slot stack resets when you open the container after closing

Expand  

 

In your deserialization method:

this.currentBurnTime = getItemBurnTime((ItemStack)this.handler.getStackInSlot(2));

What if the fuel is consumed completely and the itemstack left in the slot 2 is an empty itemstack? Then when deserializing the fuel will be set to 0, even though there might have been some left.

 

compound.setInteger("BurnTime", (short)this.burnTime);
compound.setInteger("CookTime", (short)this.cookTime);
compound.setInteger("CookTimeTotal", (short)this.totalCookTime);

Any reason you are storing these as integers(4 bytes) yet casting them to shorts beforehand(2 bytes)?

 

  On 10/23/2018 at 6:07 PM, gameristic34 said:

•  NBT data of tile entity isn't saving as it should

Expand  

Define "isn't saving as it should". What is wrong?

 

Why are you using get/setField methods? They are a horrible abomination of IInventory. Don't have them, access the fields directly.

 

implements ITileEntityProvider

Don't. Just override Block#hasTileEntity and Block#createTileEntity.

 

this.setRegistryName("blockmetallicsyphon");

Your registry name should be prefixed with your modid.

 

@SideOnly(Side.CLIENT)
    public void initModel() 

IHasModel and it's substitutes are stupid. All items need models and there is nothing about model registration that requires access to something private/protected in your block/item classes. Register your models in the model registry event, not in your block/item classes.

 

Instead of setting the state for rotation in Block#onBlockAdded or Block#onBlockPlacedBy just return the state you want in Block#getStateForPlacement.

 

Your getMetaFromState and getStateFromMeta are not compatible. The data that is read isn't the one that is being written.

 

Instead of the weird TE manipulations done in your setState method just override TileEntity#shouldRefresh to not change the TE when the blockstate property changes.

 

ItemBase/BlockBase is an antipattern. You do not need it.

 

static BlockMetallicSyphon blockmetallicsyphon = new BlockMetallicSyphon();
static BlockTestContainer blocktestcontainer = new BlockTestContainer();

Don't ever use static initializers for IForgeRegistryEntry. Instantinate your stuff directly in the appropriate RegistryEvent. 

 

You never actually sync the data in your container implementation. You just tell the server that it needs to be synced yet you never do anything when the data arrives on the client. You are missing the Container#updateProgressBar method.

 

IItemHandler itemHandler = this.te.getCapability(CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, null);

Any reason you are doing a capability lookup instead of just accessing your ItemStackHandler field?

 

  Quote

CommonProxy

Expand  

CommonProxy makes no sense. Proxies are classes that are supposed to separate sided code. If your code is common it goes into your main mod class, not into a proxy.

 

  Quote

class IGuiRegister

Expand  

This is me nitpicking but in java you usually prefix something with an I if it is an interface, not a class.

 

GameRegistry.registerTileEntity(TileEntityMetallicSyphon.class, Index.MODID + "_metallicsyphoncontainerblock");

Don't use methods that are ~1.5 years outdated. Use the version that takes a ResourceLocation as it's second argument.

[gameristic34]

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

There are a lot of isRemote checks in your code like this one

if (this.world.isRemote)

However this check ensures that whatever happens afterwards happens on the client and not on the server. As the server is the driving logic this makes sure nothing actually happens, only the client sees something changed. Untill the server is forced to sync the items to the client that is. This is the reason for

Expand  

Taken out the isRemote, that was my figurative ductape method to try and make it work just lacked the experience.

 

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

In your deserialization method:

this.currentBurnTime = getItemBurnTime((ItemStack)this.handler.getStackInSlot(2));

What if the fuel is consumed completely and the itemstack left in the slot 2 is an empty itemstack? Then when deserializing the fuel will be set to 0, even though there might have been some left.

Expand  

I have now changed back, that was a vestige of the tutorial i seemed to have missed and changed it back as I get and set the burn time in the update method.

 

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

  On 10/23/2018 at 6:07 PM, gameristic34 said:

•  NBT data of tile entity isn't saving as it should

Expand  

Define "isn't saving as it should". What is wrong?

Expand  

it was how I interpeted the error showing in the game with my fields resetting back after saving and reloading the world but this was likely related to the isRemote calls I had

 

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

Why are you using get/setField methods? They are a horrible abomination of IInventory. Don't have them, access the fields directly.

 

implements ITileEntityProvider

Don't. Just override Block#hasTileEntity and Block#createTileEntity.

Expand  

I kept the get/setField methods for the same reason as the messy transferStack method, just not important to fix (also i think I have some variables mixed up in it)

and the hasTileEntity & createTielEntity where what I had originally but when I tried to use it it failed and I choose to use ITileEntityProvider as when I went by reference it had creatNewTileEntity instead

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

this.setRegistryName("blockmetallicsyphon");

Your registry name should be prefixed with your modid.

 

@SideOnly(Side.CLIENT)
    public void initModel() 

IHasModel and it's substitutes are stupid. All items need models and there is nothing about model registration that requires access to something private/protected in your block/item classes. Register your models in the model registry event, not in your block/item classes.

 

Instead of setting the state for rotation in Block#onBlockAdded or Block#onBlockPlacedBy just return the state you want in Block#getStateForPlacement.

 

Your getMetaFromState and getStateFromMeta are not compatible. The data that is read isn't the one that is being written.

 

Instead of the weird TE manipulations done in your setState method just override TileEntity#shouldRefresh to not change the TE when the blockstate property changes.

 

ItemBase/BlockBase is an antipattern. You do not need it.

Expand  

okay big one, the state and properties is something I'm barely grasping especially the directional but i'll look more at it based on what you've told me and getStateForPlacement sounds cleaner. As for my meta to state methods, I'm at a loss of what it means, my get meta is getting the index of Facing with a math + to the result of an if ACTIVE is true or false and giving 8 as a byte for true and 0 for false. The state from meta is just something I can't get, I HAVE to find something good as a resource for learning this. setState was just to update the ACTIVE property to change the front facing texture while it was active, the reference I had included having it play sound while ACTIVE was true in there. itemBase and blockBase didn't seem to be a problem I just wanted to try and make new classes for blocks and item creation have less mistakes just because I did a typo but I guess I shouldn't be that lazy while still trying to get grounded with my modding knowledge.

 

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

static BlockMetallicSyphon blockmetallicsyphon = new BlockMetallicSyphon();
static BlockTestContainer blocktestcontainer = new BlockTestContainer();

Don't ever use static initializers for IForgeRegistryEntry. Instantinate your stuff directly in the appropriate RegistryEvent. 

Expand  

Yes Sir, just me being lazy again

 

  On 10/23/2018 at 8:36 PM, V0idWa1k3r said:

You never actually sync the data in your container implementation. You just tell the server that it needs to be synced yet you never do anything when the data arrives on the client. You are missing the Container#updateProgressBar method.

 

IItemHandler itemHandler = this.te.getCapability(CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, null);

Any reason you are doing a capability lookup instead of just accessing your ItemStackHandler field?

 

  Quote

CommonProxy

Expand  

CommonProxy makes no sense. Proxies are classes that are supposed to separate sided code. If your code is common it goes into your main mod class, not into a proxy.

 

  Quote

class IGuiRegister

Expand  

This is me nitpicking but in java you usually prefix something with an I if it is an interface, not a class.

 

GameRegistry.registerTileEntity(TileEntityMetallicSyphon.class, Index.MODID + "_metallicsyphoncontainerblock");

Don't use methods that are ~1.5 years outdated. Use the version that takes a ResourceLocation as it's second argument.

Expand  

Cool last chunk, So I had assumed that the detectAndSendChanges method kept it in sync but thinking over it it never says it does and I added the updateProgressBar method but when I looked at the Container the method has no code in it to do anything. Common proxy has been cleaned up, as has the initModel in my block class. Now although old when I put in ResourceLocation as a field I can't find the registry method for this in GameRegistry, ResourceLocation is only used in registering recipes

 

*IGuiRegister and IRegistryHandler where Interfaces i just didn't change the names when i got fed up with it and made them classes.

 

*updated git

Edited October 23, 2018 by gameristic34

[V0idWa1k3r]

  On 10/23/2018 at 9:45 PM, gameristic34 said:

As for my meta to state methods, I'm at a loss of what it means, my get meta is getting the index of Facing with a math + to the result of an if ACTIVE is true or false and giving 8 as a byte for true and 0 for false. The state from meta is just something I can't get, I HAVE to find something good as a resource for learning this.

Expand  

These methods are a way to serialize/deserialize blockstate properties to a metadata. Metadata can only go as high as 15(4 bits). So if you needed to store the facing + whether the block is "active" you would store the facing in the first 3 bits(6 possible variants) and use the last bit as a boolean for the active property.

   1          111
   ^           ^
isActive     facing

Then you can read the facing as EnumFacing.values[meta & 7(0111)] and the active as meta & 8 == 8 and write them as facing.ordinal | (isActive ? 8 : 0);

The issue with your current setus is that while you are reading the facing value you are blindly setting the active value to false, this is what I mean by incompatible - you do not restore to the same state you saved as, you loose the active property.

 

  On 10/23/2018 at 9:45 PM, gameristic34 said:

setState was just to update the ACTIVE property to change the front facing texture while it was active, the reference I had included having it play sound while ACTIVE was true in there

Expand  

Well first of all you are setting the state twice for no apparent reason considering that it is the same in both cases. Why do the same operation twice in a row like that? And secondly I mostly meant the whole tileentity nonesense. You do not need to validate and set the tile entity. Just override the method in TileEntity I told you about and you are good.

 

  On 10/23/2018 at 9:45 PM, gameristic34 said:

when I looked at the Container the method has no code in it to do anything

Expand  

Because the Container doesn't need to sync anything, it is a base class for things. Look at ContainerFurnace for example of usage.

 

  On 10/23/2018 at 9:45 PM, gameristic34 said:

Common proxy has been cleaned up

Expand  

Unless you haven't updated your git yet no, it is very much still there. And it needs to be gone completely.

 

  On 10/23/2018 at 9:45 PM, gameristic34 said:

Now although old when I put in ResourceLocation as a field I can't find the registry method for this in GameRegistry, ResourceLocation is only used in registering recipes

Expand  

Unless you are using extremely outdated forge or minecraft the resourcelocation method is there and the other one is explicitly marked as deprecated and not to be used.

[gameristic34]

thanks for pointing out that....I am apperently using 14.21.12443 and thats for 1.12 not 1.12.2 or .1, going to update now and hope nothing goes wrong

and the git did update just the root is different from the last one as this time I got GitKraken to work and last time i manually did it

Edited October 23, 2018 by gameristic34

[gameristic34]

Okay, Thank you for the break down the meta data, I recall now being told that it just slipped my mind since then, I haven't done the changes for the state yet just commented out the duplicates as i don't remember why there are duplicates and totally didn't almost lose my project trying to update forge.....

  On 10/23/2018 at 10:04 PM, V0idWa1k3r said:

Because the Container doesn't need to sync anything, it is a base class for things. Look at ContainerFurnace for example of usage.

Expand  

did and found it setting field in an instance of iInventory so I mirrored with my instance of iInventory

  On 10/23/2018 at 10:04 PM, V0idWa1k3r said:

Unless you haven't updated your git yet no, it is very much still there. And it needs to be gone completely.

Expand  

updated git went weird I think I fixed it?

  On 10/23/2018 at 10:04 PM, V0idWa1k3r said:

  On 10/23/2018 at 9:45 PM, gameristic34 said:

Now although old when I put in ResourceLocation as a field I can't find the registry method for this in GameRegistry, ResourceLocation is only used in registering recipes

Expand  

Unless you are using extremely outdated forge or minecraft the resourcelocation method is there and the other one is explicitly marked as deprecated and not to be used.

Expand  

done and works, no glaring issues still stand, just gonna change my states and post back with an updated git

[V0idWa1k3r]

  On 10/23/2018 at 10:53 PM, gameristic34 said:

updated git went weird I think I fixed it?

Expand  

You can check your git repository yourself, there is really no need to be asking us whether everything is okay. CommonProxy is still there. And blocks/items are still instantinated in static initializers.

 

  On 10/23/2018 at 10:53 PM, gameristic34 said:

did and found it setting field in an instance of iInventory so I mirrored with my instance of iInventory

Expand  

But you don't have to. set/getField are just getting/setting values of various fields. You can instead simply access the fields directly, without these getters. You are not implementing IInventory anyway, why mirror it's concepts?

[gameristic34]

  On 10/23/2018 at 11:03 PM, V0idWa1k3r said:

updated git went weird I think I fixed it?

Expand  

was supposed to be a period I don't know why it was a question mark wasn't meant to be a question.

 

Now Common proxy from what I'm aware is necessary in the main implementation like this

@SidedProxy(serverSide = "com.armalas.ee.proxy.ServerProxy", clientSide = "com.armalas.ee.proxy.ClientProxy")
	public static CommonProxy proxy;

and removing it removes this implementation too? leaving me with directly calling the client and server proxy in the three init methods individually I havn't come across this being the case before so pardon my confusion.

 

Also my block registry use

public static void blockReg(Register<Block> event) {
		event.getRegistry().registerAll(blocks);
		GameRegistry.registerTileEntity(TileEntityMetallicSyphon.class, new ResourceLocation("_metallicsyphoncontainerblock"));
		GameRegistry.registerTileEntity(TileEntityTestContainer.class, new ResourceLocation("_testcontainerblock"));
	}

should I not use the registerAll?

  On 10/23/2018 at 11:03 PM, V0idWa1k3r said:

But you don't have to. set/getField are just getting/setting values of various fields. You can instead simply access the fields directly, without these getters. You are not implementing IInventory anyway, why mirror it's concepts?

Expand  

you are correct I seem to have misunderstood something here, updateProgressbar is for what purpose then? it sounds like that is what detectAndSendChanges is doing

[V0idWa1k3r]

  On 10/23/2018 at 11:32 PM, gameristic34 said:

Common proxy from what I'm aware is necessary in the main implementation like this

Expand  

Use an interface instead. Like IProxy or something.

 

  On 10/23/2018 at 11:32 PM, gameristic34 said:

should I not use the registerAll?

Expand  

You can use registerAll if you want. Just don't have your blocks/items/whatever instantinated in a static initializers

static BlockMetallicSyphon blockmetallicsyphon = new BlockMetallicSyphon();
static BlockTestContainer blocktestcontainer = new BlockTestContainer();

^

This is wrong. Instantinate your registry entries directly in the registry event.

 

  On 10/23/2018 at 11:32 PM, gameristic34 said:

updateProgressbar is for what purpose then? it sounds like that is what detectAndSendChanges is doing

Expand  

detectAndSendChanges is a server-side method that compares the fields in the TE to the fields in the Container checking whether the fields in the TE were changed. If they were it sends the changed data to all listeners together with an index for that data.

updateProgressBar is the client-side method that receives the data and the index and is responsible for setting the fields in the client TE with the new server data.

[gameristic34]

  On 10/23/2018 at 11:38 PM, V0idWa1k3r said:

detectAndSendChanges is a server-side method that compares the fields in the TE to the fields in the Container checking whether the fields in the TE were changed. If they were it sends the changed data to all listeners together with an index for that data.

updateProgressBar is the client-side method that receives the data and the index and is responsible for setting the fields in the client TE with the new server data.

Expand  

How did you learn this difference it really isn't something I can see being learned from looking at the ContainerFurnace method but thanks none the less.

 

static BlockMetallicSyphon blockmetallicsyphon = new BlockMetallicSyphon();
static BlockTestContainer blocktestcontainer = new BlockTestContainer();

This was because I had calls like this

	@Override
	public Item getItemDropped(IBlockState state, Random rand, int fortune) {
		// TODO Auto-generated method stub
		return Item.getItemFromBlock(BlockIndex.blockmetallicsyphon);
	}

and I'll change this out soon I have plenty of changes to work on for now and have made a list from whats in this thread to go through it all. Also the Interface suggestion of IProxy or something similar, I looked more through the files to see the call hierarchy and see how you implemented it I'll also try to find a reliable source to read from so I don't make mistakes again.