More items renamed in inventory.

Hello everyone!

I am remaking my game’s inventory and items system. I tried another aproach than what i was using and now i have one big problem.
I am loading the items from a XML file (template items) into an arraylist. Every item has some properties (stored in an 2D String arrray). Everytime i want to create a new item (like if i kill a mob and he drops an item) i copy the item (based on an item id) from the templates arraylist, i modify some properties (if needed) and add it to the inventory arraylist.

The problem is that when i modify, let’s say, the name of an item, all the items (with it’s id, based on same template item) change their names.

Here is how i load the items:

package com.game.util;

import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.DocumentBuilder;

import org.newdawn.slick.util.ResourceLoader;
import org.w3c.dom.Document;
import org.w3c.dom.NodeList;
import org.w3c.dom.Node;
import org.w3c.dom.Element;

import com.game.items.Item;

import java.io.InputStream;
import java.util.ArrayList;

public class ItemsLoader {
	public static ArrayList<Item> itemsList = new ArrayList<Item>();
	public static NodeList nodeList;
	
	public static void loadItems(String location) {		
		try {
			InputStream fXmlFile = ResourceLoader.getResourceAsStream(location);
			DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
			DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
			
			Document doc = dBuilder.parse(fXmlFile);
			doc.getDocumentElement().normalize();
			
			nodeList = doc.getElementsByTagName("item");
			
			for (int i = 0; i < nodeList.getLength(); i++) {
	            Element element = (Element) nodeList.item(i);
	            
	            Item item = new Item(Integer.parseInt(element.getAttribute("id")));
	            
	            for(int j = 0; j < element.getAttributes().getLength(); j++) {
	            	Node attribute = element.getAttributes().item(j);
	            	String attributeName = attribute.getNodeName();
	            	String attributeValue = attribute.getNodeValue();
	            	
	            	item.addProperty(attributeName, attributeValue);
	            }
	            
	            itemsList.add(item);
	        }
		} catch (Exception e) {
			e.printStackTrace();
		}
	}
	
	public static Item searchItemByItemID(int itemID) {
		for(Item item : itemsList)
			if(item.getItemID() == itemID)
				return item;
		return null;
	}
}

Item class:

package com.game.items;

import org.newdawn.slick.Graphics;
import org.newdawn.slick.Image;
import org.newdawn.slick.geom.Polygon;

import com.game.util.ResourceLoader;

public class Item {
	private int x, y, width, height;
	private int itemID;
	private Image dropedImage, inventoryImage;
	private String[] properties, propertiesValue;
	private int propertiesNumber;
	private Polygon hitBox;
	
	public Item(int itemID) {
		this.setProperties(new String[10]);
		this.setPropertiesValue(new String[10]);
		this.setPropertiesNumber(0);
		this.setItemID(itemID);
		this.setDropedImage(ResourceLoader.dropSheet.getSprite(this.getItemID() % ResourceLoader.dropSheet.getHorizontalCount(), this.getItemID() / ResourceLoader.dropSheet.getHorizontalCount()));
		this.setInventoryImage(ResourceLoader.inventorySheet.getSprite(this.getItemID() % ResourceLoader.inventorySheet.getHorizontalCount(), this.getItemID() / ResourceLoader.inventorySheet.getHorizontalCount()));
	}
	
	public Item(Item item) {
		this.setProperties(item.getProperties());
		this.setPropertiesValue(item.getPropertiesValue());
		this.setPropertiesNumber(item.getPropertiesNumber());
		this.setItemID(item.getItemID());
		this.setDropedImage(item.getDropedImage());
		this.setInventoryImage(item.getInventoryImage());
	}

	public void addProperty(String property, String propertyValue) {
		this.getProperties()[getPropertiesNumber()] = property;
		this.getPropertiesValue()[getPropertiesNumber()] = propertyValue;
		this.setPropertiesNumber(this.getPropertiesNumber() + 1);
	}
	
	public void setProperty(String property, String propertyValue) {
		boolean found = false;
		
		for(int i = 0; i < getPropertiesNumber(); i++) {
			if(this.getProperties()[i] == property) {
				this.getPropertiesValue()[i] = propertyValue;
				found = true;
				break;
			}
		}
		
		if(!found) {
			this.getProperties()[getPropertiesNumber()] = property;
			this.getPropertiesValue()[getPropertiesNumber()] = propertyValue;
			this.setPropertiesNumber(this.getPropertiesNumber() + 1);
		}
	}
	
	public String getValueOfProperty(String property) {
		for(int i = 0; i < this.getPropertiesNumber(); i++) {
			if(this.getProperties()[i].equals(property))
				return this.getPropertiesValue()[i];
		}
		return property;
	}
	
	public void render(Graphics g, boolean inInventory) {
		if(inInventory) g.drawImage(getInventoryImage(), this.x, this.y);
		else g.drawImage(getDropedImage(), this.x, this.y);
	}
	
	//GETTERS AND SETTERS
	public int getX() { return x; }
	public int getY() { return y; }
	public void setX(int x) { this.x = x; }
	public void setY(int y) { this.y = y; }
	public int getWidth() { return width; }
	public int getHeight() { return height; }
	public int getItemID() { return itemID; }
	public String[] getProperties() { return properties; }
	public Polygon getHitBox() { return hitBox; }
	public void setWidth(int width) { this.width = width; }
	public void setHeight(int height) { this.height = height; }
	public Image getDropedImage() { return dropedImage; }
	public void setHitBox(Polygon hitBox) { this.hitBox = hitBox; }
	public void setProperties(String[] properties) { this.properties = properties; }
	public int getPropertiesNumber() { return propertiesNumber; }
	public void setPropertiesNumber(int propertiesNumber) { this.propertiesNumber = propertiesNumber; }
	public String[] getPropertiesValue() { return propertiesValue; }
	public void setPropertiesValue(String[] propertiesValue) { this.propertiesValue = propertiesValue; }
	public void setItemID(int itemID) { this.itemID = itemID; }
	public void setDropedImage(Image dropedImage) { this.dropedImage = dropedImage; }
	public Image getInventoryImage() { return inventoryImage; }
	public void setInventoryImage(Image inventoryImage) { this.inventoryImage = inventoryImage; }
}

ItemFactory:

package com.game.items;

import org.newdawn.slick.geom.Polygon;

import com.game.util.ItemsLoader;

public class ItemFactory {
	public static Item createNewDropItem(int itemID, int x, int y) {
		Item source = ItemsLoader.searchItemByItemID(itemID);
		
		Item item = new Item(source.getItemID());
		item.setProperties(source.getProperties());
		item.setPropertiesValue(source.getPropertiesValue());
		item.setPropertiesNumber(source.getPropertiesNumber());
		item.setItemID(source.getItemID());
		item.setDropedImage(source.getDropedImage());
		item.setInventoryImage(source.getInventoryImage());

		item.setX(x);
		item.setY(y);
		item.setWidth(item.getDropedImage().getWidth());
		item.setHeight(item.getDropedImage().getHeight());
		item.setHitBox(new Polygon(new float[]{
				(float) item.getX(), (float) item.getY(),
				(float) (item.getX() + item.getWidth()), (float) item.getY(),
				(float) (item.getX() + item.getWidth()), (float) (item.getY() + item.getHeight()),
				(float) item.getX(), (float) (item.getY() + item.getHeight())
		}));

		return item;
	}

	public static Item createNewInventoryItem(Item sourceItem) {
		Item item = new Item(sourceItem.getItemID());
		item.setProperties(sourceItem.getProperties());
		item.setPropertiesValue(sourceItem.getPropertiesValue());
		item.setPropertiesNumber(sourceItem.getPropertiesNumber());
		item.setItemID(sourceItem.getItemID());
		item.setDropedImage(sourceItem.getDropedImage());
		item.setInventoryImage(sourceItem.getInventoryImage());

		return item;
	}
}
private void dropItem() {
		if(this instanceof Enemy) {
			Random rand = new Random();
			EntityManager.getEnemies().remove(this);
			if(rand.nextInt(6) > 3) {
				if(this instanceof Slime) {
					Item dropItem = ItemFactory.createNewDropItem(rand.nextInt(ItemsLoader.itemsList.size()), (int) this.x,(int) this.y + this.height);
					dropItem.setProperty("name", "Test 1");
					EntityManager.getDropedItems().add(dropItem);
				}
			}
		}
	}
public void getItem(Item item) {
		Item inventoryItem = ItemFactory.createNewInventoryItem(item);
		boolean search = false, alreadyExist = false;
		
		switch(inventoryItem.getValueOfProperty("type")) {
			case "weapon": case "head": case "chest": case "legs": case "boots": case "hands": search = false; break;
			default : search = true;
		}
		
		if(search)
			for(ItemSlot itemSlot : itemSlots) {
				if(itemSlot.getItemID() == inventoryItem.getItemID()) {
					itemSlot.setQuantity(itemSlot.getQuantity() + 1);
					itemSlot.updateItem();
					alreadyExist = true;
					break;
				}
			}
		if(!search || !alreadyExist)
			for(ItemSlot itemSlot : itemSlots) {
				if(itemSlot.getItemID() == -1) {
					itemSlot.setItem(inventoryItem, 1);
					itemSlot.updateItem();
					break;
				}
			}
	}

I tend to do two different classes. One for the definition of an item and one for its instance. A definition exists only once in a global manager. When you create an item (for example in a drop), you create an instance, that refers to the definition. All properties, that are fixed (basic name, description, weight, base price etc.) for an item are stored in the definition object. The instance class contains properties like location (either location on the map, or the inventory of the owner), instance name (overrides the definition name), instance id, effects, charge (for magic items) or the price you payed to buy the item at a shop.

I’m not entirely sure about the effects. In my opinion they belong to the definition, but the instance should provide an option to override them. Like that you could create one “Health potion” and give it different effects (strong potion, weak potion etc). I think in the end it depends on the complexity of your game, how you implement this.

Thank you, but i don’t think this might help me. My problem is that although i don’t have any reference to my base “template” item, when i change a property to one item, all items of that kind also changes that property, including the template, the items created before and the items created after the item changed. >:(

I can’t see the code, where you copy the properties but it looks like you do not copy the properties themselves. When you create a copy of your base item, make sure that you also make copies of the properties… If you just do something like “copy.name = source.name;”, the reference will be copied (means both item classes share the same string, means that if you change the string in copy, the string in source will be changed too).

[edit]
Also, maybe try using a HashMap<String, String> for your property collection instead of the two arrays. You can make your code significantly easier that way.
[/edit]

Sorry, I had to read your code multiple times to fully understand, what you do with the properties… In my opinion those two lines are the source of the problem:


      item.setProperties(sourceItem.getProperties());
      item.setPropertiesValue(sourceItem.getPropertiesValue());

You would need to change the setProperties method to something like this:



	 public void setProperties(String[] properties) 
	 {
		 this.properties = new String[properties.length];
		 for(int x = 0; x < properties.length; x++)
		 {
			 String prop = properties[x];
			 this.properties[x] = prop == null || "".equals(prop) ? "" : new String(prop);
			 
		 }
	 }

I changed to HashMap, same problem.

You would still have to copy the strings, otherwise you again just copy the reference to the same string.



	 public void setProperties(HashMap<String, String> properties) 
	 {
		 this.properties.clear();
		 for(String key : properties)
		 {
			 String prop = properties.get(key);
			 this.properties.put(key, prop == null || "".equals(prop) ? "" : new String(prop));
		 }
	 }

a) Use JSON
b) Use HashMaps instead of trying to emulate them with arrays; load the values into primitive types so you aren’t always doing string-checks. The Item class shouldn’t hold a HashMap unless you truly need dynamically changing properties; and if that’s the case, you should take care to copy the hash map rather than pass it around as a reference.
c) Use unique string names for IDs instead of magic numbers and integers
d) Voila: your code is easier to read, easier to debug, and works.

IT WOKS!!! 8)

Thank you very much. I tried almost all day to repair this.

You’re welcome…

@davedes: I’m curious about your JSON remark. Could you please be a bit more specific? Is there a class or a library, that you can work with JSON object without much serialization/deserialization overhead? I really love json, but it always seemed to much to me to always serialize/deserialize the json string.

There are plenty of JSON libraries for Java that will make your life easier – just search Google. Some of them, like FlexJSON or Gson, will deserialize JSON into your own class. e.g. You can deserialize a JSON object into a predefined “ItemDescriptor” class.

Thanks for the infos. I’ll have a look at it.