Optimization - how much is too much?

[Ernio]

I'd like to know when I should consider caching stuff instead of counting it every tick for player.

 

I am mostly worried about ExtendedInventory items. Every tick, there is a loop through all ItemStacks, their NBT being read and the value inside ExtendedPlayer is being updated, most of the time it's literally not changed (same as in tick before)

There are also a lot more values applied to main value, player can have effects (buffs/defuffs).

 

Finally if I'd have to count there would be about 100 if's, few thousands iterations (sum/multiply), and some string-scanning (worst case I would compare it to few houndred nbt.getInteger("something") operations).

That's not huge amount for computer, but when you get 50 players+ it might mean SOMETHING.

 

Should I be worried about anything here?

 

Anyway, I've been considering reading NBT of ItemStacks inside inventory ONLY when you place/remove them and move that NBT to object values and store them ready-to-use in ExtendedPlayer, but again - I am worried that there might be some errors?

Is there any possibility that SOMEHOW game will remove ItemStack from slot without slot knowing it (so the ready-to-use list wont get updated).

This is how I sync it: this is ofc IInventory of player (size of slot is always 1)

@Override
public ItemStack decrStackSize(int index, int quantity)
{
	if (this.stackList[index] != null)
	{
		ItemStack itemstack;

		if (this.stackList[index].stackSize <= quantity)
		{
			itemstack = this.stackList[index];				
			this.stackList[index] = null;

			if (itemstack != null && itemstack.getItem() instanceof IItemCore)
			{
				((IItemCore) itemstack.getItem()).onUnequipped(itemstack, player.get());
			}
		}
		else
		{
			itemstack = this.stackList[index].splitStack(quantity);

			if (this.stackList[index].stackSize == 0)
			{
				this.stackList[index] = null;
			}

			if (itemstack != null && itemstack.getItem() instanceof IItemCore)
			{
				((IItemCore) itemstack.getItem()).onUnequipped(itemstack, player.get());
			}
		}
		if (inventoryHandler != null) this.inventoryHandler.onCraftMatrixChanged(this);
		syncSlotToClients(index);
		return itemstack;
	}
	else
	{
		return null;
	}
}

@Override
public void setInventorySlotContents(int index, ItemStack stack)
{
	this.stackList[index] = stack;
	if (stack != null && stack.getItem() instanceof IItemCore)
	{
		((IItemCore) stack.getItem()).onEquipped(stack, player.get());
	}
	if (inventoryHandler != null) this.inventoryHandler.onCraftMatrixChanged(this);
	syncSlotToClients(index);
}

 

P.S - yeah, I have those weird habits to optimize everything everywhere, so I might be overreacting.

 

Edit: As for bolded question - if you know something (situation it might occur) please write so I can handle it.

[coolAlias]

It really depends. If you have your own IInventory implementation like you showed below, then the case of items changing is already handled and you shouldn't need to loop at all.

 

Each time a slot changes, you call equip or unequip for the item which modifies values in the player's extended properties (much better than writing to NBT each time), then you just get the value from the extended properties rather than using getEntityData().getWhateverValue("something").

 

That would be about as efficient as you could get, not involving any loops or disk I/O, at least for managing the player's current buffs/debuffs.

 

Btw, in your setInventorySlotContents implementation, you should check first if the current stack in the slot is not null and call onUnequipped for it, otherwise you could easily end up with permanent buffs/debuffs.

[Ernio]

"Btw, in your setInventorySlotContents implementation, you should check first if the current stack in the slot is not null and call onUnequipped for it, otherwise you could easily end up with permanent buffs/debuffs."

-coolAlias

 

I'd eventually notice it, but nice catch thx