[1.7.10] Calculations Server Side Only?

[HalestormXV]

Here is the KeyHandler code. It is relatively simple. I just followed a YouTube tutorial on it:

http://pastebin.com/JMEcau60

 

With respect to the code optimization, yes I know for sure I need to work on it. I plan to work on that significantly. I have prior coding experience in C++, Lua, and PhP but for some ridiculous reason Java and I struggle. But it is for sure something I know I need to work on and I do plan on addressing it. I do appreciate the tutorial links and will look into them for sure.

[coolAlias]

Personally, I would prefer to compare the event key to keys[bACKPACK_KEY].keyCode rather than your keyValues array - that is only for the default key binding value i.e. before the player has a chance to change it in their settings. Basically, once the game starts, you have no idea what the actual key value of the keybinding will be, so you need to use the KeyBinding instance you created.

 

Anyway, the problem would seem to be in your packet, since when you open your GUI without sending a packet it works fine, and as long as your packet is actually being sent and your network code is all correct, that is the only place left that could be broken.

 

Do the System.outs in your packet's #handleServerMessage method ever print to the console? It looks like it should print out 36 messages, since you never break out of your for loop. Might want to do that, as I doubt you want to open the GUI 36 times.

[HalestormXV]

Actually, yes. They do print out lol (and it is quite funny to see my own code message insult myself). And yes they print out what appears to be 36 times on point.  Oopsy lol Could that actually be why? Because it is being sent so many times that a number of GUIs are "opening on top" of each other on the code side of things?

 

Here is the dispatcher I created from your tutorial: http://pastebin.com/UR8HafKd

[coolAlias]

That could very well have something to do with it, yeah

[HalestormXV]

Alright, I optimized the packet according to your prior post. It works fine:

public static class Handler extends AbstractServerMessageHandler<OpenGuiPacketAlt> 
{
	@Override
	public IMessage handleServerMessage(EntityPlayer player, OpenGuiPacketAlt message, MessageContext ctx)
	{
		for (int i = 0; i < player.inventory.getSizeInventory(); i++)
		{
			if (player.inventory.getStackInSlot(i) != null)
			{
				if (i != player.inventory.currentItem)
				{
					System.out.println("Congratulations you finally created a frigen packet that works!");
					player.openGui(MainRegistry.modInstance, message.id, player.worldObj, i, 0, 0);
				}else if (i == player.inventory.currentItem)
				{
					player.openGui(MainRegistry.modInstance, message.id, player.worldObj, player.inventory.currentItem, 0, 0);
				}
			}
		}
		return null;
	}
}

 

I know I am going to break the loop once we get to the slot that contains the item? So I know HOW to break the loop with the "break;" and I know the break will be in my for loop so that we don't cycle. But to only send the packet "when its needed" is where I am stuck. How would I go about doing that? Would I need an addition nested if in that for loop somewhere?

 

EDIT: Ignore that else if - I didn't recognize it until I posted it here. I changed it to a regular else. ><

[coolAlias]

What do you mean? You've already sent the packet, that's why you are in the message handler code.

 

And it looks like you got my advice backwards somehow... I wasn't recommending you remove the check on the Item contained in the ItemStack, but removing the unuseful check if i is equal to the currentItem. I mean, if i == currentItem, you're still sending 'i' because they are equal, right? So that's a pointless check that makes your code more complicated looking than it should:

@Override
public IMessage handleServerMessage(EntityPlayer player, OpenGuiPacketAlt message, MessageContext ctx) {
for (int i = 0; i < player.inventory.getSizeInventory(); i++) {
	ItemStack stack = player.inventory.getStackInSlot(i);
	if (stack != null && stack.getItem() == YourMod.backpackItem) {
		player.openGui(MainRegistry.modInstance, message.id, player.worldObj, i, 0, 0);
		break;
	}
}
return null;
}

[HalestormXV]

Yes I misunderstood and honestly looking at it, that was a totally amateur mistake on my part which I should have realized with my prior experience even though it isn't in Java. I blame the fact that I have been trying to get this thing working since I learned how to make my first packet lol since all my users have been bugging me about it and I want to get it for them lol. But nevertheless still a careless mistake that I really shouldn't have made, especially since I have other checks just like that in other parts of my code. So that is resolved, and the packet only gets sent once now and the loop properly breaks.

 

However here is a clip of the still existing original error:

https://dl.dropboxusercontent.com/u/5221303/Error.mp4

 

The GUI isn't opening a ridiculous amount of times anymore, so that has been ruled out.

Here is the code:

http://pastebin.com/h404yYjv (Inventory)

http://pastebin.com/bJzCS88i (Container)

http://pastebin.com/wyGwaZem (Slot)

 

I am currently using Jabelar's tutorial to rework the keybinding issue that you mentioned. But it appears that is not the source of this error as it is just a keypress that sends the packet.

[coolAlias]

Only other issue that sticks out is you didn't choose between the 2 different transferStackInSlot options - you can't use both.

 

In other words, use ONLY ONE of those two if statements, keeping the 'else if' after whichever one you choose.

[HalestormXV]

How did I miss that one. I swear I didn't realize I missed it. I go back to me looking at this code too much and not giving my eyes a rest from it. Alright, so it appears the shift click issue is resolved....The only one that remains is the issue in which when you open the inventory with the Keybind and the item is not selected. You "pick up" the item and can't place it in any other slot and have to drop it on the floor and re-pick it up. However, if you open the inventory with the Y key while you are holding the item it properly locks the item in place and any other items you are dragging around in your inventory can be placed elsewhere in the inventory with the exception of the Container for the keyPouch.

 

Now in an effort to try and figure this out on my own I though that maybe it has to do with this line in the Container class.

@Override
public ItemStack slotClick(int slot, int button, int flag, EntityPlayer player) {
	// this will prevent the player from interacting with the item that opened the inventory:
	if (slot >= 0 && getSlot(slot) != null && getSlot(slot).getStack() == player.getHeldItem()) {
		return null;
	}
	return super.slotClick(slot, button, flag, player);
}

 

Is that the correct hunch and is the issue that it is checking to see player.getHeldItem() hence why it works properly when you are "holding the item." And would I be correct in assuming I have to do the same think i did in the GUIHandler and get the slotID that has been passed to X to correct that? Or something to that effect? Or at the very least if not anything to do with getting the slot ID change that code somehow.

[coolAlias]

Yep, that's the right place to look, and yes, you could pass in 'x' to your Container constructor so it knows which slot to lock down.

[HalestormXV]

Alright so I think i fixed it, however I don't know if this method is SMP compatible.

 

I pass the "X" via the packet to the Container Constructor

 

Portion of PacketCode

System.out.println("Congratulations you finally created a frigen packet that works!");
ContainerCelestialKeypouch.lockDown = i;
player.openGui(MainRegistry.modInstance, message.id, player.worldObj, i, 0, 0);
break;

 

That packet sends the slotID to the Container and stores it in the container as: public static int lockDown;

 

Then I changed the lockDown code to this:

@Override
public ItemStack slotClick(int slot, int button, int flag, EntityPlayer player) {
	// this will prevent the player from interacting with the item that opened the inventory:
	System.out.println("The lockdown is currently: " +lockDown);
	if (slot >= 0 && getSlot(slot) != null && getSlot(slot).getSlotIndex() == lockDown)
	{
		return null;
	}
	return super.slotClick(slot, button, flag, player);

 

This method seems to work however I can only test it on single player. It also prints out the correct index of the slot to indicate that it is correct. What I am going to do is add the same code to the itemClass itself so that the itemClass also pass the "x" to the container constructor. That will take care of the issue that will pop up if the user happens to OpenGUI with Y -> Proceed to Move Bag to Hotbar -> RightClick to open the bag. This will not "lockdown" the bag because packet which updates the "slotID" has not been sent as they are bypassing the packet process. Granted, next time they open the bag wit hteh hotkey it will update itself, but rather than do that I figured it be better just to pass the value via the itemclass onRightClick function.

 

Here is my concern. Is this an efficient way to do this? In other words I know the lockDown int is static and as static this would make it the same across all instances, will that cause issues on SMP? My gut tells me it will especially since the int can and will change between each player. If that is the case then I could resolve that by simply creating a method within the container class that "fetches" the lockDown value each time, correct?

 

Or hell, might it be even easier to simply store the slotID in the NBT tag and just pull from that, since it write NBT everytime you open it since it stores all its contents in the NBT data, why not store its own slotINdex? Or even better add in the item's onUpdate just write the item's current slotID to the NBT if it has changed.

[coolAlias]

That's the wrong way to do it. First of all, that looks like you used a static class member - no no no no. NO. Please Google 'Java static class member' and read about what it means.

 

You need to pass the slot index value you already have in your IGuiHandler to your Container's constructor when you instantiate it. Do you know what that means in terms of code? It means this:

return new ContainerCelestialKeypouch(player, player.inventory, new InventoryKeyPouch(stack), x);

Then in your Container class:

private final slotIndex;

public ContainerCelestialKeypouch(EntityPlayer player, IInventory inv, InventoryKeyPouch itemInv, int slotIndex) {
this.slotIndex = slotIndex;
// rest of constructor code
}

[HalestormXV]

Thanks for all that. I knew what doing the static meant. It makes it the same across all instances, that's why I said my gut tells me it isn't going to work. I was wrapped in the thinking that I couldn't just pass my variable into the ContainerKeypouch constructor for some reason even though I have done it with some of my other classes. Everything works as it should now after making the changes.

 

No more ghost items, and no more moving of the pouch while it is opened. (Either by rightclick or by Hotkey) All that is left is to adjust my keyHandler which I am sure using Jabelar's tutorial (http://jabelarminecraft.blogspot.com/p/minecraft-forge-1721710-keybinding.html) will be easily applied.

 

Thank you again for all the help and making me realize my silly mistakes lol. I am going to have to review over some of those oracle tuts in more detail. At least I know and admit I have a weakness lol