[Solved] [1.7.10] Block activation seems to randomly not work?

[IceMetalPunk]

*EDIT* The problem has been updated. See this post below.

 

*Original post below saved for archiving*

 

While adding a new block, I noticed something strange. It seems the z-coordinate parameter for all the built-in methods (onBlockActivated, randomDisplayTick, etc.) are 1 too low. In other words, if the block is at (0,0,0), the parameter values are passed as (0,0,-1) instead. It caused some issues with my code, since I'm checking World.getBlockMetadata(), which requires accurate coordinates.

 

I've fixed it by simply adding 1 to the z-coordinate, but this feels like it shouldn't be happening. Is this a known quirk of these methods and I should continue using z+1 everywhere, or is this unheard of and I should be looking for problems with my code?

[Cerandior]

Are you using some sort of loop before passing these values, or you simply taking the raw ones from methods

[IceMetalPunk]

Just taking the raw values. And there's a new strangeness I just discovered: this only happens in the test environment. Once I build the mod and load it into the normal Forge client, the parameter values are correct (I know this because my z+1 code, which worked in the test environment, is now setting data 1 block too *high* on the z-axis). Could the testing environment have anything to do with it? (I'm using Eclipse, by the way.)

[coolAlias]

What version of Forge are you using? If you have a different one in your test environment vs. installed to play mods, that could explain it.

 

There have been similar bugs in the past, but I'm not aware of any with the not-so-recent recommended 1.7.10 build 10.13.4.1448.

[IceMetalPunk]

I'm using Forge 1.7.10 build 10.13.4.1566, in both the dev environment as well as the installed client.

 

*EDIT* Okay, in testing, I've found that there's something truly buggy in my code, which makes me think this is my fault, not Forge's. Should I ask about my code's issue here, or create a new topic for it?

[coolAlias]

I'm using Forge 1.7.10 build 10.13.4.1566, in both the dev environment as well as the installed client.

 

*EDIT* Okay, in testing, I've found that there's something truly buggy in my code, which makes me think this is my fault, not Forge's. Should I ask about my code's issue here, or create a new topic for it?

May as well post your code here.

[IceMetalPunk]

Fair enough

 

So what I'm trying to do is make a simple solid block (an "Ender Iron Block") that's purely decorative. When you right-click it, it should toggle between emitting particles and not emitting particles. In my first tests, it worked, except the z-coordinate had to be shifted by 1 (as mentioned above). But then, I realized it wasn't even doing that consistently; sometimes, it seems to just not toggle at all. The debug output shows that it seems to need the z+1 still (the coordinates that are output are off by 1), but even with that, it seems to randomly decide when to toggle, and usually decides not to.

 

Here's the EnderIronBlock.java code:

 

 

package enderhoppers;

import java.util.Random;

import cpw.mods.fml.relauncher.Side;
import cpw.mods.fml.relauncher.SideOnly;
import net.minecraft.block.Block;
import net.minecraft.block.material.Material;
import net.minecraft.creativetab.CreativeTabs;
import net.minecraft.entity.Entity;
import net.minecraft.entity.player.EntityPlayer;
//import net.minecraft.entity.item.EntityItem;
import net.minecraft.world.World;




public class EnderIronBlock extends Block {
private byte cooldown=0;
public EnderIronBlock(Material material) {
	super(material);
	setHarvestLevel("pickaxe",2); // 0=wood/gold, 1=stone, 2=iron, 3=diamond
	setBlockName("enderIronBlock");
	setHardness(5.0f);
	setResistance(30.0f);
	setStepSound(Block.soundTypeMetal);
	setCreativeTab(CreativeTabs.tabBlock);
	setBlockTextureName("enderhoppers:ender_iron_block");
}

@SideOnly(Side.CLIENT)
@Override
    public void randomDisplayTick(World world, int x, int y, int z, Random rand) {
        if (world.getBlockMetadata(x, y, z)==1) {
            this.particle(world, x, y, z);
        }
        if (this.cooldown>0) {
        	--this.cooldown;
        }
    }

@SideOnly(Side.CLIENT)
@Override
public boolean onBlockActivated(World world, int x, int y, int z, EntityPlayer player, int pInt, float px, float py, float pz) {
	if (this.cooldown<=0) {

		if (world.getBlockMetadata(x, y, z)==0) {
			System.out.println("Set ("+x+", "+y+", "+z+") to 1!");
			world.setBlockMetadataWithNotify(x, y, z, 1, 3);
		}
		else {
			System.out.println("Set ("+x+", "+y+", "+z+") to 0!");
			world.setBlockMetadataWithNotify(x, y, z, 0, 3);
		}
		this.cooldown=5;
	}
	return super.onBlockActivated(world, x, y, z, player, pInt, px, py, pz);
}

private void particle(World world, int x, int y, int z) {
	if (world.getBlockMetadata(x, y, z)==0) {
		return;
	}
	Random rand=world.rand;
	double d0 = 0.0625D;

        for (int l = 0; l < 6; ++l)
        {
            double d1 = (double)((float)x + rand.nextFloat());
            double d2 = (double)((float)y + rand.nextFloat());
            double d3 = (double)((float)z + rand.nextFloat());

            world.spawnParticle("portal", d1, d2, d3, 0.0D, 0.0D, 0.0D);
        }

}
}

 

 

Any ideas what's going on here?

[coolAlias]

Yep - #onBlockActivated is NOT @SideOnly(Side.CLIENT) - you should NEVER add that unless the vanilla or Forge method you are overriding uses it.

 

In 1.7.10, the client player's y position has their eye height added to it, so that's why you're getting off by 1 issues.

 

Also, you cannot have local mutable fields in Block or Item classes because there is only one instance of each of them - your 'cooldown' field is effectively static and will be the same for every single block of this type in the game. If you don't mind them all 'cooling down' at the same time, that's fine, but otherwise you'll need a TileEntity.

[IceMetalPunk]

The cooldown is fine to be shared by all blocks. It's only there so that holding right click too long doesn't toggle the effect multiple times.

 

I'm not getting off-by-1 issues with the y-coordinate, only the z...also, it's in the block's z-coordinate that's off, not the player's.

 

I added the @SideOnly annotation because when both the client and the server were executing the code, it would toggle twice--once on the client, once on the server--effectively resetting the metadata. If there a better way to make it toggle only one time when activated?

[coolAlias]

Oh yeah, z-coord... lol, dunno about that then.

 

As for the block metadata and code running twice - you should only ever change data on the SERVER side. If it's important for the client to know about it for whatever reason (e.g. rendering) then Minecraft usually syncs that data automatically. This is the case with setting blocks or block metadata. Never set that on the client.

 

Many methods are called on both sides, and that's fine, but the server has the final say. @SideOnly is NOT an appropriate solution to this issue - instead, nest your code inside of an `if (!world.isRemote)` check so it only runs on the server, or without the ! to run it on the client (e.g. spawning particles).

[IceMetalPunk]

Thank you! That worked, and fixed all the bugs I was having! I'm a little curious, though: I thought @SideOnly was basically the same as having a check on the world.isRemote value. What's the difference in implementation between the two methods? Clearly it's something major since it caused this bug, but I'd like to understand that annotation a bit more.

[coolAlias]

diesieben does a much better job of explaining it than I probably would.

[IceMetalPunk]

Awesome, thanks! So in essence, while I thought it was only running the code on the client, it actually wasn't, because @SideOnly doesn't do anything to separate the client and the integrated server, only the client JAR and the dedicated server JAR. That is definitely good information to know going forward Gracias!

[RANKSHANK]

Yep - #onBlockActivated is NOT @SideOnly(Side.CLIENT) - you should NEVER add that unless the vanilla or Forge method you are overriding uses it.

I wouldn't say never.

 

The issue here is it's a massive newbie trap and always has been. Side.Client and Side.server are misnomers:

Side.Client is inclusive of the integrated server, and thus does not prevent double calls between the integrated server thread and the client thread. Which is our pitfall here.

Side.Server does not include the integrated server, it's solely for the dedicated server.

Think of it more as Side.Client&Integrated and Side.Dedicated_Server_Only

 

The reason these exist is that they're a flag for the Classloader. If the given side is not equivalent, the Classloader ignores that method/class/field and it will be missing from the runtime environment.

 

So when you used SideOnly(Side.Client), the dedicated server will be missing that method, defaulting to whichever class in it's heiarchy overwrote it last (unless it's THE override for an abstract type in a non abstract class which will cause {Type}NotFound exceptions up the wazoo when the class is loaded). However the integrated server thread and client thread both have access to that method since they share a Classloader.

 

TLDR: Use ONLY when you know you need the given Type to be missing from a particular runtime(Minecraft client or Dedicated server)

 

 

 

 

As for the cooldown, your best bet is sending a packet if you plan to have this network ready, otherwise you'll hit snags when multiple players try interacting with it

 

[coolAlias]

I wouldn't say never.

"Never" is almost always relative to one's experience - for the @SideOnly annotations, I think telling people to NEVER* use them is quite appropriate. By the time they have enough experience to realize they can and should use it in some very few cases, they will probably know how to do so without causing all sorts of mayhem like the OP did in this thread.

 

* Except when a parent method or class uses the annotation, in which case yours should have it, too, which I noted in my first comment about this.