[1.7.10] Looping Item Subtypes

[HalestormXV]

Alright, I have some code here that works fine as is. But I want to make it a bit more efficient and cleaner. Currently there is an item with 12 different subtypes and the each item has a different effect. Specifically each item summons a different entity. Here is a portion of the code now: http://pastebin.com/Dn7kzbzu

 

I know for a fact I can probably for loop this and it will be nicer to look at and probably more efficient. Any ideas on how I can go about For looping that? Now I know for loops are elementary in most cases. But I can't quite figure it out because I want to make sure it calculates the values properly. As you can tell each summon has a different cost attributed to it that is dynamic based on the summoner's EXP.

 

Any help is appreciated. I could always go the "if way" but perhaps a for loop I think would be more effective, quicker and efficient?

 

Or maybe it isn't possible to handle all of it in a loop but perhaps a private function for the calculations?

[coolAlias]

Your code for creating each entity is effectively identical, which is the perfect case for using Reflection - store the class of Entity to summon as a class field in your Item, then when your item is used use Reflection to construct a new instance of that class.

[HalestormXV]

I understand reflection is very powerful however in doing my research I have also read that it can be quite difficult. Now I do want to learn as I am still new to all of this. Can you provide an example of how this may look? I did do my own research to and found this video which seems to explain the basics nicely:

 

I also understand that reflection may be resource heavy? Although I imagine with a situation where tehre are 12 different subsets of an item that probably won't even be recognized.

 

And I also stumbled onto this: http://www.minecraftforge.net/wiki/Using_Access_Transformers (not needed for this though from what I understand)

[shadowfacts]

public static YourItem extends Item {
    public static Class<? extends Entity>[] entityClasses = new Class<? extends Entity>[11];
    // the Class is the entity to summon and the index in the array is the metadata it should be used for
    // insert here: populate the list

    public boolean onItemUse(ItemStack stack, EntityPlayer player, World world, int x, int y, int z, int meta, float f1, float f2, float f3) {
        
        if (player.canPlayerEdit(x, y, z, meta, stack) && !world.isRemote) {
            Entity e = entityClasses[meta].getConstructor(World.class).newInstance(world);
            // set any other entity properties
            // spawn entity
            // catch various potential reflection exceptions
        }
    }

}

[HalestormXV]

alright so currently this is the entire itemClass file: http://pastebin.com/px29McDj

 

So ideally with the reflection way. I would essentially replace all of those if checks in the onUse boolean with the reflection ways above?

[coolAlias]

Yes, using Reflection will allow you to replace all of your if statements with a single block of code such as shadowfacts'.

 

In your case, you would use the ItemStack's damage value to retrieve the correct values from your various arrays, of which you will need one containing either the various Entity classes or, as diesieben suggested, the constructors for those classes. You will need that in order to create a new instance of that entity, just as shadowfacts showed in his code.

[HalestormXV]

Alright so after taking in all of your comments and looking at some tutorials I finished up the code. It "SEEMS" to be functioning as it should. Here is the link to the whole code:

http://pastebin.com/FCMm79uB

 

Here is the link to block using reflection: http://pastebin.com/SyLqQaPP

 

Now although this works from what I can tell, would it be SMP friendly? IS there anything that maybe my nooby eyes are missing? Do the calculations seem correct, in terms that the costs will always have fresh values?

 

Secondly, diesieben07 I know you were suggesting NOT to call or store the class in the array but I couldn't figure out what you mean by only storing each class's constructor since the code uses the .getConstructor

 

Perhaps you or someone can show an example of what you meant?

 

This is my first time working with this reflection and I do see incredibly how much it cleaned the code up. So any more help and/or feedback is appreciated.

[coolAlias]

The only thing in there that might not be SMP friendly is 'public boolean keyActive;' - what is that for? I don't see it used anywhere in your Item code, but any class field of Item (or Block) is effectively static as Items and Blocks are all singletons.

 

What this means is that if you set keyActive to true for one player using your Item, ALL ItemStacks containing your Item are affected, so player 2 who is not using the Item but has it now tries to use it and can't, for example, because keyActive is true for him/her too.

 

Any dynamic information must, therefore, be stored in the ItemStack's NBT, which is unique per ItemStack.

[HalestormXV]

The keyActive was actually something I was using to test and decided against it. I just forgot to take it out. Although you bring up a good point with setting the key active for one player and not the other. This way each summon is almost "unique." So that one players doesn't have 4 Aries running around as that would be just odd. So that is something I would like to implement. I am not that familar with item NBT just yet. Although I did read this tutorial:

 

http://www.minecraftforge.net/wiki/Creating_NBT_for_items

 

So am I correct in assuming that I could add

	itemstack.stackTagCompound = new NBTTagCompound();
	itemstack.stackTagCompound.setString("Owner", player.getUniqueID().toString());
	itemstack.stackTagCompound.setBoolean("Active", false);

into my item class?

 

And then possibly have an

			boolean activeKey = itemstack.stackTagCompound.getBoolean("Active");

			if (!activeKey)
//Execute summoning code here (Reflection code block that we worked out above)
else
Send chat: You already have a creature out.

 

 

My question though is how would I start this up. These items aren't craftable. They are rewards. So to that end I can't use the "onCreate" method to set up the NBT data. However if I try to add in the NBT setup into the onUse method it will just overwrite itself with the new data every time it is right clicked, correct?

 

If there really is no way around this I don't mind making the items craftable at all, if it would make things that much easier.

 

Another thought that occurred to me, is in-since this item has subtypes will that cause any issues with NBT? Granted each key is a MaxStack of 1. So you won't be able to stack them but that also just popped into my head while reading the above tutorial. It seemed like it wasn't subtype friendly?

[coolAlias]

You can check if the key exists before you 'overwrite' it, as well as take advantage of the default return values when a key doesn't exist, e.g.:

public onItemUse() { // obviously not correct signature, but you know what I mean
  // Only set to a blank compound if it doesn't already have one
  if (!stack.hasTagCompound()) { stack.setTagCompound(new NBTTagCompound()); }
  // default is 'false' if the key doesn't exist, which is great since that means the item hasn't been used yet
  if (stack.getTagCompound().getBoolean("Active")) {
    stack.getTagCompound().setBoolean("Active", true); // now it's 'active'
  }
}

If, however, you are trying to create an item that can only summon one of each creature per player, you would be better off, in my opinion, storing such data in IExtendedEntityProperties rather than trying to cram it all into the stack's NBT tag.

 

For one, if the player has more than one "Item of Summon X", each of those stacks has a different NBT tag and the player could then summon 2 of the same entity by using both items. If that is how your item is intended to work, then great, but if not, it'd be much more straightforward to simply check 'has this player already summoned entity X?' via IEEP which, in this case, would store data per player rather than per item.

[HalestormXV]

Alright, well the way it is supposed to work is you have the Item of summoning. You use it to summon your entity. By summoning your entity that item is now your's and bound to you and your "contract" is formed. The player is supposed to be able to summon that entity so long as they have that item. Now the NBT tag method seems to do exactly that. However, you bring up a valid point. If the item is duplicated (via dupe bug) or if I do decide to make them craftable they can summon them with one of each item, since, as you stated, each stack has its own NBT.

 

I am interested in this IExtendedEntityProperties though. That seems to make a safeguard against potential dupe bugs with other mods. I am however, totally unfamiliar with it. I actually came across your tutorials:

https://github.com/coolAlias/Forge_Tutorials/blob/master/EventHandlerTutorial.java

https://github.com/coolAlias/Forge_Tutorials/blob/master/IExtendedEntityPropertiesTutorial.java

http://www.minecraftforum.net/forums/mapping-and-modding/mapping-and-modding-tutorials/2137055-1-7-x-1-8-customizing-packet-handling-with

 

Would these be what I have to look into? Ideally all I want to do is have the code say, I'm already summoned by you and you can't summon me again while I am out here. Furthermore, if you managed to dupe my item I still don't want you to summon me if I have already been summoned by you and am active.

 

Would it be perhaps easier/more effective or efficient just to make sure that a player cannot have more than one of the item in their inventory and if they do take out the duplicate? Or maybe even better? If the entity is already spawned and summoned and you use the item again it gets rid of the entity and sets it dead? Although that still doesn't handle, multi item multi summon.

[coolAlias]

It really depends on how you want your design to work, and a combination of approaches may be best:

 

If you want each Item to 'belong' to a particular player, then you will need to store that player in the ItemStack's NBT so that only they can use it.

 

If you want each entity to only be summonable once per player at any given time, then you need to store which entity(s) the player currently has summoned in IEEP. Likewise, if you want each player to only be able to summon any one entity at any given time, i.e. not 1 of entity A and one of entity B, but only one or the other, then you can just store a simple boolean 'summonedEntity' in IEEP.

 

If, however, you want each entity to only be summonable globally at any given time, i.e. if player A summons entity X, then player B cannot also summon it, then you'd want to store which entities are summoned not per player, but somewhere else like your CommonProxy - whenever one of your special entities joins the world (EntityJoinWorldEvent), update the array or whatever in your CommonProxy. When a player attempts to summon one, check that array and see if one already exists.

 

The last 2 options protect you from any 'dupe' bugs, while the 1st addresses item ownership. These are just examples of how your implementation depends on your design.

[HalestormXV]

I understand. Well judging by your examples it would seem a combination of both might be the best option.

 

The NBT data can be used on the itemstack to give it some fancy properties and set an owner and all that good stuff. However it appears for my design to work I will need to use IExtendedEntityProperties.

 

Perhaps a nudge in the right direction?

 

I have read over your tutorial and I created my class here: http://pastebin.com/J6c1ZUPf

 

Already registered via my main class and what not. So I would essentially need to create a tag for every one of my entities, correct? I can then use the single boolean and change it to true once an entity has been summoned. Write that to the player NBT file, then in the itemclass file check that boolean and if it is returning true prevent them from summoning the entity again?

 

Or am I totally off-base. I apologize for all these questions but I am still learning and this has been incredibly helpful. I want to actually learn how to do this hence why i am researching all these tutorials and such but I do need some guidance from my superiors.

 

I know that everything works on the NBT side of things as opening up my .dat file shows the new category has been created an all 12 values are added in that new category and are all set to false. What I can't seem to figure out is how to make it so that the flag gets set to true in the item onUse function so that they cannot be double summoned by the same player. And I can't figure it out because that part is using the reflection block and I don't know howt o make it auto determine which flag it needs to set to true depending on the key you are using.

 

IE: Using Key with a damage value of 6 is the Aries key. I need to make sure that when you use that key that the ariesIsActive gets set to true in the player's NBT data.

[coolAlias]

1. Do NOT use 'static' on your mutable class fields

2. You need to actually save the value of the variables - your save method currently sets every one to 'false'

3. Getting and setting these values is about as basic of Java as it gets - just make methods to do what you want

[HalestormXV]

2. You need to actually save the value of the variables - your save method currently sets every one to 'false'

- that was an oversight on mypart. I changed each one to

properties.getBoolean(PLAYER_HAS_CONTRACT))

This way it will get the current value, which ideally would be changed after a successful summon.

 

3. Getting and setting these values is about as basic of Java as it gets - just make methods to do what you want

And that is why I am dumbstruck. I know it is incredibly basic.

Summon Entity Success -> Set Correct Flag to True -> Update/Save the Player's NBT -> Poof they can't summon that entity -> Entity Dies or Despawnes, no problem, reset flag back to false -> update/save NBT

 

So yeah simply reach out from the item class when the item is used and the entity is successfully summoned/created to set the flag and then in the entityClass on its onDeath function set the flag for its owner (the player) to false.

 

I guess my problem is how to reach out from the itemClass into the ExtendedPlayer class and set the correct value for the correct spirit/entity.

 

Using the Reflection code block automates everything for me yes and is super efficient and works great. But where I am stuck is how do you have the code say we are using the Summoning Stone with the damage value of 6 so set the flag in the ExtendedPlayer array of the 6th index to true.

[coolAlias]

You need to get the IEEP instance for the given player, and then call whatever class methods you have, e.g.

YourIeepClass instance = YourIeepClass.get(player);
instance.setSummoned(6, true);

[HalestormXV]

Alright that small example actually helped alot and now I am getting somewhere, so thank you for that. It seems my code is now actually "doing something" although it is causing a crash.

http://pastebin.com/CqFmmrGF  (Crash Log Full)

http://pastebin.com/0LVPn3VZ  (Point in the log where the error occured)

 

That crash log narrows it down. I totally did my method wrong OR it is ticking because I didn't create my packet handler yet.

I am fairly certain that perhaps the crash is caused by not sending or even creating the packets though, but I am still new to all of this so I may be wrong. Although i do recall or at least I think I recall in reading over some tutorials that when you are manipulating client data that is to be saved on the server you will almost always need a packet handler. Because as well all know the information is stored in packets and you need to send those packets to the server so it can store them.

 

Here is the fixed up code for the IEEP

http://pastebin.com/3p7e5tf1  which includes the function I am calling to set the tags from my itemClass.

 

Here is the complete itemClass file:

http://pastebin.com/KfQa8aNj

 

I am a visual learner, so by seeing that example or even a type of pseudocode it helps tremendously.

Thank you again for all the help and responses. I apologize for my noobishness but I am learning and your responses are not in vain. As with my previous threads, every single one of my issues that I have brought up on this forums I have resolved with the help of everyone, so I am learning and not only learning but retaining.

 

EDIT: Just thought of this, would it be easier to set these true and false values with a data watcher since the entites are not supposed to stay summoned?

[coolAlias]

It looks like you are trying to set NBT data directly in your IExtendedEntityProperties class - this is not the way. Store data in appropriate class fields, such as a boolean array.

 

E.g. your #setSummoned method should take neither an ItemStack nor an NBTTagCompound (where the heck is that coming from anyway?), nor should you pass in an 'int Index' simply to set it equal to the stack's damage value. None of that makes any sense. All you want this method to do is mark a specific entity type as summoned or not, so you need 2 arguments: the type of entity, and whether it is currently summoned or not.

 

When you call this method from the Item, you will almost certainly only ever be passing 'true' as the 2nd argument, and you would call this method again in your entity's 'setDead()' method to make sure it gets reset to 'false' for the summoning player and summoned entity type.

 

I know you say you learn from examples, so let me direct you to some basic Java tutorials that cover methods and method arguments. Seriously - go through all of the lessons, practice what each one discusses, and you will find your current problems to be trivial.

[HalestormXV]

Alright, maybe you can answer me this question perhaps? I am going to take another look at it and heed your advice. Because I know in its current state my code is a mess, but I am learning and cleaning it up as I go. And the NBT data is coming from my itemclass with the player.getEntityData() which I had no idea existed until i went digging around in some of the source code.

 

My methods seem to work now after I created the packet handler. It is working correctly now. But I don't know why. I followed your network handler tutorial and set up a packet handler and I took the examples you gave in your tutorial and altered them and changed up their fields to fit my mod as well as added some new functions to my customNBT. But in order for me to get the best out of this I want to know why it is working now.

 

 

My code is as follows:

Here is my ExtendedPlayer now:

http://pastebin.com/qkyCqc9A

 

My Itemclass:

http://pastebin.com/DnQeUb8a

 

Your Packet Dispatcher

http://pastebin.com/JM8yh52F

 

The Sync Packet for the ExtendedPlayer:

http://pastebin.com/rJXVrm2A

 

So combining all of these and registering my event handler makes my items now do EXACTLY what I want. You can have as many keys of the same subType as you want but once you summon one entity you cannot summon it again. Now if you select a different key with a different subtype you can summon that entity and then you cannot summon EITHER entity again. So they are not "overwriting" each other and it seems to be working perfectly. All that is left now is to change the flag back in the entity class once they despawn which I am sure I can figure out.

 

But what I want to know is why all of a sudden now this is working? Becasue of the packets? And is it still going to work that way on an SMP. It seems like it would since it is literally only altering that player's NBT data.

Or is it literally because I fixed my method and the packet isn't really doing anything.

 

[coolAlias]

You're not listening: do NOT send in an NBTTagCompound to that method - that makes NO sense at all, and you don't even use it in your method:

public void setSummoned(ItemStack itemstack, int Index, NBTTagCompound compound) //NBTTagCompound compound)
        {
                // where do you use 'properties' ? Right, nowhere. Where do you use 'compound'? Again, nowhere. Get rid of them.
                NBTTagCompound properties = (NBTTagCompound) compound.getTag(CELESITAL_CRAFT_INFO);

                Index = itemstack.getItemDamage(); // 'Index' already equals the stack damage, so this is pointless, too, meaning the ItemStack parameter is not needed at all
                if (summonedState[index] == false)
                {
                        summonedState[index] = true;
                }else{
                        summonedState[index] = false;
                }
                // that's a lot of lines for a simple operation (toggling a boolean value):
                summonedState[index] = !summonedState[index];

                this.sync(); // not necessary unless you plan to display your information in a GUI, and even then, this is overkill - use a more specific packet that sends only the information that has changed
        }

Also, you should NOT use #getEntityData when you are already using IEEP.

 

Furthermore, you have 12 private boolean fields, and then a boolean array in your IEEP class - the 12 fields are not used except as defaults for the array, but you never initialize them (which is fine - they default to false, but so does the array). This is silly:

private boolean aquariusActive, taurusActive, cancerActive, virgoActive, sagittariusActive, leoActive, ariesActive, scorpioActive, geminiActive, capricornActive, piscesActive, libraActive;
        
public boolean[] summonedState = new boolean[]{aquariusActive, taurusActive, cancerActive, virgoActive, sagittariusActive, leoActive, ariesActive, scorpioActive, geminiActive, capricornActive, piscesActive, libraActive};

// why not this instead:
public boolean[] summonedState = new boolean[12];

You also have two identical arrays, 'celKeys' and 'celKeys2', in your Item class...

 

As for your question - it works because of all of the reasons I already explained, as well as basic logic: set a flag and use it as a conditional check before allowing an action. I really don't know what to tell you other than that, because that is all that it is, and seems so rudimentary and fundamental a concept as to not even need further explanation. If you don't get it, perhaps take a programming 101 course? At the very least, start from the beginning of the Java tutorials I linked earlier. You will thank yourself for it later.

 

It has nothing to do with the packets which, in your case, you do not even need since you are not displaying any information on the client side. You should remove all of the packet code unless and until you actually need it.

[HalestormXV]

Excellent, seeing those examples helped me out greatly. I have also been looking into those basic tutorials. I have somewhat o a programming background its just a matter of saying alright Java is object based this is different from C++ etc. etc. I find it more in line with PhP. Unfortunately Java is not something I ever practiced with. But those tutorials I am sure will help. So I have changed the method and cleaned up the code. The method is much simpler now:

public void setSummoned(int Index, boolean active)
{
	summonedState[index] = active;
}

 

SNIP

 

Just editing this post. I managed to try out my question. My question was if I could just store the entities owner in its initialization as a variable which I did do with:

this.ownerStored = (EntityPlayer) this.getOwner();

 

Then simply in the onUpdate event where it checks if the owner is null (person logged out or crashed) it then goes to the stored data and passes that along and sets the isSummoned flag to false. Once the player logs back in they can resummon the entity and it works fine.

 

I also managed to play with the itemstacks NBT and set it up so it now tracks how many times you have used that particular item and the item becomes contracted to you and indicates who the owner of the item is on the actual item. So if another person steals your key they can't use it and they are forever haunted by the name of the rightful owner showing every-time they hover over the key. So that was an added little bonus that I get to thank you for.

 

I got the idea from your original code before you introduced me to the ExtendedProperties. So, as it stands now everything is working exactly as it should along with some bonus features. It seems like it should work fine on a server as well.

 

So thank you for all the awesome help, as frustrating as it may have been for you. I will take a lot out of this.

 

Finally [sOLVED]