[1.8][SOLVED]Issues with item that spawns entities

[The_Fireplace]

I have 2 items(Guns) that use ammo. The ammo is saved with ExtendedPlayer. When the guns are fired now, the ammo usually either a.) doesn't spawn or b.)spawns the wrong type of ammo. The Item of the ammo is consumed as it should be, then, when the player leaves the world and rejoins, it is there again, so it seems that the onItemRightClick is only being called on the client side for these items. Sorry if this explanation of the issue is vague.

Anyways, here is the code:

The 2 guns:

https://bitbucket.org/The_Fireplace/unlogic-ii/src/2763b9479b2c4b875fe506e29ea2977ff68f2880/src/main/java/the_fireplace/unlogicii/items/ItemBlockCoalGun.java?at=master

https://bitbucket.org/The_Fireplace/unlogic-ii/src/2763b9479b2c4b875fe506e29ea2977ff68f2880/src/main/java/the_fireplace/unlogicii/items/ItemBlockSmartCoalGun.java?at=master

 

The ammo files:

https://bitbucket.org/The_Fireplace/unlogic-ii/src/2763b9479b2c4b875fe506e29ea2977ff68f2880/src/main/java/the_fireplace/unlogicii/entity/coal/?at=master

 

And a download of the mod, so you can try it out yourself without having to fork and clone the repository, then set up the workspace. The items are Coal Gun and Smart Coal Gun, and they fire the types of coal in the UnLogic II creative tab, and vanilla coal.

http://ge.tt/api/1/files/5OCIQN32/35/blob?download

[Failender]

I am not sure, I havent messed around with entites that much..

Try creating your entity, then setting its Position and After that Spawn it in world

[The_Fireplace]

  On 5/31/2015 at 5:54 PM, Failender said:

I am not sure, I havent messed around with entites that much..

Try creating your entity, then setting its Position and After that Spawn it in world

The issue is not that they are spawning at 0,0,0, it is that they are not spawning at all.

[Born2Code]

Okay. As for the problem of not properly consuming the item, is that part is outside of the if(!world.isRemote) checker. As for the entity you might not have registered the entity properly (or at all), or maybe the applied the rendering wrong and it is spawning.

[coolAlias]

Have you tried using your IDE's debugger? The problem is likely simply one of logic, i.e. a value is not what you expect it to be.

 

Furthermore, simplifying / rewriting your logic would likely help make things clearer. I don't really see the point of all those conditions in your while loop, for example:

// Is it really necessary to store the ammo type in ExtendedPlayer? Why can't you just fetch it each time you right click?
EnumAmmo ammo = ExtendedPlayer.get(playerIn).getAmmoType(); // can this ever return null?
// This loop already changes the item each time, making all those conditionals redundant and messy
// Also, you have an EnumAmmo instance on hand, so why make static methods when you can use the instance itself?
// E.g. ammo.getItem() instead of EnumAmmo.getItem(ammo)
while (!playerIn.inventory.hasItem(ammo.getItem())) {
// no need to involve player here, just fetch the next ammo type and see if the player has it
ammo = ammo.getNext());
// You will need, however some way to stop the loop, e.g. if ammo loops back around to its original value, set it to null and break
}
if (ammo == null) {
return itemStackIn;
}
// set the ExtendedPlayer ammo value at this point, if you still want to do that
ExtendedPlayer.get(player).setAmmo(ammo);
// You've already checked that the player has the item needed, right? Don't need to check again, then.
YourCoalEntity entity = ammo.getCoalEntity(worldIn, playerIn); // make your Enum do some work for you
if (entity != null && !worldIn.isRemote) {
worldIn.spawnEntityInWorld(entity);
// ammo already knows the item, right? So why hard code them all?
playerIn.inventory.consumeInventoryItem(ammo.getItem());
}
return itemStackIn;

You could probably even consolidate all of those different entities into one class, depending on how different they are, simply by setting different class attributes based on the ammo type: setDamage(based on ammo type), setVelocity(based on ammo type), etc.

 

Sorry I kind of went off on a tangent, there, but I think it would be worth your while to do some refactoring here. In the meantime, you should be able to pinpoint your issue by using the debugger.

[The_Fireplace]

  On 6/1/2015 at 12:38 AM, Born2Code said:

Okay. As for the problem of not properly consuming the item, is that part is outside of the if(!world.isRemote) checker. As for the entity you might not have registered the entity properly (or at all), or maybe the applied the rendering wrong and it is spawning.

I will look again, I am fairly certain the entity is registered properly. As for the fact that it is outside the !world.isRemote, that just means it would be removed on both the server and client, and not only on the server, leaving behind ghost items in the inventory. As for the rendering, I know for sure that that is not the issue.

  Quote

Have you tried using your IDE's debugger? The problem is likely simply one of logic, i.e. a value is not what you expect it to be.

 

Furthermore, simplifying / rewriting your logic would likely help make things clearer. I don't really see the point of all those conditions in your while loop, for example:

// Is it really necessary to store the ammo type in ExtendedPlayer? Why can't you just fetch it each time you right click?
EnumAmmo ammo = ExtendedPlayer.get(playerIn).getAmmoType(); // can this ever return null?
// This loop already changes the item each time, making all those conditionals redundant and messy
// Also, you have an EnumAmmo instance on hand, so why make static methods when you can use the instance itself?
// E.g. ammo.getItem() instead of EnumAmmo.getItem(ammo)
while (!playerIn.inventory.hasItem(ammo.getItem())) {
// no need to involve player here, just fetch the next ammo type and see if the player has it
ammo = ammo.getNext());
// You will need, however some way to stop the loop, e.g. if ammo loops back around to its original value, set it to null and break
}
if (ammo == null) {
return itemStackIn;
}
// set the ExtendedPlayer ammo value at this point, if you still want to do that
ExtendedPlayer.get(player).setAmmo(ammo);
// You've already checked that the player has the item needed, right? Don't need to check again, then.
YourCoalEntity entity = ammo.getCoalEntity(worldIn, playerIn); // make your Enum do some work for you
if (entity != null && !worldIn.isRemote) {
worldIn.spawnEntityInWorld(entity);
// ammo already knows the item, right? So why hard code them all?
playerIn.inventory.consumeInventoryItem(ammo.getItem());
}
return itemStackIn;

You could probably even consolidate all of those different entities into one class, depending on how different they are, simply by setting different class attributes based on the ammo type: setDamage(based on ammo type), setVelocity(based on ammo type), etc.

 

Sorry I kind of went off on a tangent, there, but I think it would be worth your while to do some refactoring here. In the meantime, you should be able to pinpoint your issue by using the debugger.

There is no reason to be sorry, Tbh, I love it when people take the time to explain a better way of doing things than what I have. I will look in to what you have said and see about a redesign. The reason it is stored with the extendedplayer is so it stays consistant. This way, a player can disconnect and join again later, with the coal type they are on staying the same.

[coolAlias]

  On 6/1/2015 at 1:36 AM, The_Fireplace said:

There is no reason to be sorry, Tbh, I love it when people take the time to explain a better way of doing things than what I have. I will look in to what you have said and see about a redesign. The reason it is stored with the extendedplayer is so it stays consistant. This way, a player can disconnect and join again later, with the coal type they are on staying the same.

Isn't the type of ammo the player is using based on what they have in their inventory? Or are they selecting it from somewhere? If the latter, that would make sense to store it like you have.

 

I had a look at your main class' registerEntity method, and there are a few things wrong with it:

1. Don't register your entity with a global entity ID - that is for vanilla only

2. If you don't need a spawn egg, there is even more reason not to use global entity ID, as that is its only benefit (well, and the /summon command)

3. Tracking update frequency and range values should be set based on the type of entity you are registering, not as a blanket value for all of them. Furthermore, NO vanilla entity uses an update frequency of 1, and you should not either. Always choose values closest to the most closely related vanilla entity you can find.

 

You can find tracking values in EntityTracker, or online here, though I don't know when it was last updated. Generally, projectiles (except for arrows, for some reason) use (64, 10) and mobs / animals use (80, 3).

[The_Fireplace]

  On 6/1/2015 at 2:10 AM, coolAlias said:

  Quote

There is no reason to be sorry, Tbh, I love it when people take the time to explain a better way of doing things than what I have. I will look in to what you have said and see about a redesign. The reason it is stored with the extendedplayer is so it stays consistant. This way, a player can disconnect and join again later, with the coal type they are on staying the same.

Isn't the type of ammo the player is using based on what they have in their inventory? Or are they selecting it from somewhere? If the latter, that would make sense to store it like you have.

 

I had a look at your main class' registerEntity method, and there are a few things wrong with it:

1. Don't register your entity with a global entity ID - that is for vanilla only

2. If you don't need a spawn egg, there is even more reason not to use global entity ID, as that is its only benefit (well, and the /summon command)

3. Tracking update frequency and range values should be set based on the type of entity you are registering, not as a blanket value for all of them. Furthermore, NO vanilla entity uses an update frequency of 1, and you should not either. Always choose values closest to the most closely related vanilla entity you can find.

 

You can find tracking values in EntityTracker, or online https://docs.google.com/spreadsheets/d/1nqqieNkw9r4NfI-WypIvgeuZnYB4_QU7BcDXptgzqnM/pub?single=true&gid=0&output=htmlhere, though I don't know when it was last updated. Generally, projectiles (except for arrows, for some reason) use (64, 10) and mobs / animals use (80, 3).

Thanks for the information about registering entities, I will fix that. And the ammo is based on what the player has it set to, if they haven't set it yet, it is regular coal, they can press R at any time to change to the next type. The order is Coal, Charged Coal, Destabilized Coal, Restabilized Coal, Refined Coal, and then it goes back to Coal whewhen pressed again. The smart coal gun, when fired, automatically detects if you don't have any coal of the type of ammo you are on and, if you have any valid coal types in the inventory, it switches the player to the next coal type until it can fire. With the regular coal gun, the player has to select it, there is no automatic switching.

[The_Fireplace]

It seems that when the player changes the ammo type they are on, it doesn't send that information to the server. That explains why the items disappear on the client side only, because the server is trying to use the default, but the client thinks they are being consumed, and why, if you have coal and the ammo you are trying to use, coal gets spawned instead of the ammo type you are on, and when you relog, the coal is missing from that. Big moment of realization I just had there. The explaination fits perfectly. Now for how to fix it. I am guessing this will require me to send a packet to the server when the player changes ammo type. Does anyone have a link to a 1.8 tutorial on how to do this, or want to explain it?

[coolAlias]

Yes, you have to send a packet. You ALWAYS need to send a packet when you are performing actions on the client that need to impact the world in some way, including which item will be consumed, etc.

 

Never set a value on the client that has a correlating value on the server; instead, tell the server what the player wants to do, then let the server decide the value and inform the client player what that value is. E.g.:

 

1. Player wants to change ammo, presses 'R'

2. Key handler sends packet to server saying 'Player A pressed the change ammo key, now what?'

3. Server receives packet, determines player A's next ammo type

4. Server sends packet back to player A on the client side, saying 'Your new ammo type is X'

 

That's how your program flow should go, always, for things like this.

 

As for tutorials / explanations, did you try Google? There is a tutorial by diesieben07 right here in this forum in the Tutorials section, and I have one over on MCF that does things somewhat differently, but I have improved upon it quite a lot since I wrote that tutorial and haven't gotten around to updating it.

[The_Fireplace]

  On 6/1/2015 at 4:32 AM, coolAlias said:

Yes, you have to send a packet. You ALWAYS need to send a packet when you are performing actions on the client that need to impact the world in some way, including which item will be consumed, etc.

 

Never set a value on the client that has a correlating value on the server; instead, tell the server what the player wants to do, then let the server decide the value and inform the client player what that value is. E.g.:

 

1. Player wants to change ammo, presses 'R'

2. Key handler sends packet to server saying 'Player A pressed the change ammo key, now what?'

3. Server receives packet, determines player A's next ammo type

4. Server sends packet back to player A on the client side, saying 'Your new ammo type is X'

 

That's how your program flow should go, always, for things like this.

 

As for tutorials / explanations, did you try Google? There is a tutorial by diesieben07 right here in this forum in the Tutorials section, and I have one over on MCF that does things somewhat differently, but I have improved upon it quite a lot since I wrote that tutorial and haven't gotten around to updating it.

Actually, after posting that, I went ahead and searched for a tutorial, and found the tutorials by you and diesieben. I read diesieben's first, and followed along, though I did encounter the annoyances which I later learned that your tutorial addressed. Also, according to what you just said, I set up the flow wrong, so I will have to go back and fix that when I follow your tutorial. If it isn't too much to ask, since you said you have made improvements to it, and just haven't updated it, can you update your tutorial?