Thread: Perfect Rune Pouch

Page 2 of 3 FirstFirst 123 LastLast
Results 11 to 20 of 21
  1. #11  
    Extreme Donator

    JayArrowz's Avatar
    Join Date
    Sep 2008
    Posts
    113
    Thanks given
    101
    Thanks received
    111
    Rep Power
    851
    Quote Originally Posted by Remi View Post
    Just use EnumMap instead of HashMap when using enums as keys,

    inside your enum instead of using private static final HashMap<Integer, Rune> FOR_ID = new HashMap<>();

    you would use private static final Map<Integer, Rune> FOR_ID =
    EnumSet.allOf(Rune.class).stream().collect(Collect ors.toMap(r -> r.id, r -> r));

    than you would be able to call the rune like Rune rune = Rune.FOR_ID.get(item.getId());

    this allows for O(1) time when looking up a rune by its id

    for store, you would be able to use
    Code:
    private void store(Item item) {
        Rune rune = Rune.FOR_ID.get(item.getId());
        if (rune == null) {
            player.getPacketSender().sendMessage("You can only store runes into the rune pouch.");
            return;
        }
    
        if (!pouch.containsKey(rune) && pouch.size() == MAX_POUCH_SIZE) {
            player.getPacketSender().sendMessage("Your rune pouch is full.");
            return;
        }
    for take out you can streamline the removable of runes

    Code:
    private void takeOut(int slot) {
        Rune rune = new ArrayList<>(pouch.keySet()).get(slot);
        if (player.getInventory().hasFreeSlots() || player.getInventory().contains(rune.id)) {
            int amount = pouch.remove(rune);
            if (amount > 0) {
                player.getInventory().add(rune.id, amount);
                open();
            }
    for handleItemInteraction, you dont need a switch as its only one item id

    just use an if statement if its RUNE_POUCH

    Code:
    public boolean handleItemInteraction(Player player, Item item, int type) {
        if (item.getId() == RUNE_POUCH) {
            if (type == 1) {
                open();
            } else if (type == 2) {
                empty();
            }
            return true;
        }
    Why use a enum at all?

    Code:
    package com.elvarg.game.content.item;
    
    import java.lang.reflect.Type;
    import java.util.ArrayList;
    import java.util.HashSet;
    import java.util.Map;
    import java.util.Optional;
    import java.util.Set;
    
    import com.elvarg.game.World;
    import com.elvarg.game.entity.impl.player.Player;
    import com.elvarg.game.model.Item;
    import com.elvarg.game.model.JsonIO;
    import com.elvarg.game.model.ui.UserContainerInterface;
    import com.elvarg.net.packet.interaction.PacketInteraction;
    import com.elvarg.util.Misc;
    import com.google.gson.Gson;
    import com.google.gson.JsonElement;
    import com.google.gson.JsonObject;
    import com.google.gson.reflect.TypeToken;
    
    /**
     * 
     * @author Dexter Morgan <http://www.rune-server.org/members/dexter+morgan/>
     *
     */
    public class RunePouch extends PacketInteraction {
    
    	private static final int INTERFACE_ID = 24_200;
    
    	private static final int POUCH_CONTAINER_ID = 24206;
    
    	private static final int INVENTORY_CONTAINER_ID = 24209;
    
    	private static final int MAX_POUCH_SIZE = 3;
    
    	public static final int RUNE_POUCH = 12791;
    
    	private static final Set<Integer> RUNE_IDS = new HashSet<>(Set.of(
    		554,  // FIRE_RUNE
    		555,  // WATER_RUNE
    		556,  // AIR_RUNE
    		557,  // EARTH_RUNE
    		558,  // MIND_RUNE
    		559,  // BODY_RUNE
    		560,  // DEATH_RUNE
    		561,  // NATURE_RUNE
    		562,  // CHAOS_RUNE
    		563,  // LAW_RUNE
    		564,  // COSMIC_RUNE
    		565,  // BLOOD_RUNE
    		566,  // SOUL_RUNE
    		9075  // ASTRAL
    	));
    
    	private static final Type REFERENCE = new TypeToken<Map<Integer, Integer>>() {}.getType();
    
    	public static final JsonIO IO = new JsonIO("./data/saves/rune_pouch/") {
    
    	  @Override
    		public void init(String name, Gson builder, JsonObject reader) {
    			JsonElement element = reader.get("pouch");
    
    			Map<Integer, Integer> result = GSON.fromJson(element.toString(), REFERENCE);
    
    			Optional<Player> p = World.getPlayerByName(name);
    
    			if (!p.isPresent()) {
    				return;
    			}
    
    			p.get().runePouch.pouch.putAll(result);
    		}
    
    	  @Override
    		public JsonObject save(String name, Gson builder, JsonObject object) {
    
    			Optional<Player> p = World.getPlayerByName(name);
    
    			if (!p.isPresent()) {
    				return object;
    			}
    
    			object.add("pouch", builder.toJsonTree(p.get().runePouch.pouch, REFERENCE));
    			return object;
    		}
    	};
    
    	static {
    		new UserContainerInterface(POUCH_CONTAINER_ID) {
    
    		  @Override
    			public boolean handleOption(Player player, int id, int slot, int option, int amount) {
    				if (player.getInterfaceId() != INTERFACE_ID) {
    					return false;
    				}
    
    				takeOut(player, slot);
    				return true;
    			}
    		};
    
    		new UserContainerInterface(INVENTORY_CONTAINER_ID) {
    
    		  @Override
    			public boolean handleOption(Player player, int id, int slot, int option, int amount) {
    				if (player.getInterfaceId() != INTERFACE_ID) {
    					return false;
    				}
    
    				int existing = player.getInventory().getAmount(id);
    
    				if (existing == 0) {
    					return true;
    				}
    
    				store(player, new Item(id, existing));
    
    				open(player);
    				return true;
    			}
    		};
    	}
    
    	private Map<Integer, Integer> pouch = new HashMap<>();
    
    	private static void open(Player p) {
    		ArrayList<Item> pouchItems = new ArrayList<>();
    
    		for (Map.Entry<Integer, Integer> e : p.runePouch.pouch.entrySet()) {
    			pouchItems.add(new Item(e.getKey(), e.getValue()));
    		}
    
    		p.getPacketSender().sendItemContainerList(pouchItems, POUCH_CONTAINER_ID);
    
    		p.getPacketSender().sendItemContainer(p.getInventory(), INVENTORY_CONTAINER_ID);
    
    		p.getPacketSender().sendInterface(INTERFACE_ID);
    	}
    
    	private static void store(Player p, Item item) {
    		int id = item.getId();
    
    		if (!RUNE_IDS.contains(id)) {
    			p.getPacketSender().sendMessage("You can only store runes into the rune pouch.");
    			return;
    		}
    
    		if (!p.runePouch.pouch.containsKey(id) && p.runePouch.pouch.size() == MAX_POUCH_SIZE) {
    			p.getPacketSender().sendMessage("Your rune pouch is full.");
    			return;
    		}
    
    		if (!p.getInventory().contains(item)) {
    			return;
    		}
    
    		String name = item.getName();
    		int amount = item.getAmount();
    
    		p.getInventory().delete(item);
    
    		p.runePouch.pouch.merge(id, amount, Integer::sum);
    
    		p.getPacketSender().sendMessage("You store " + Misc.format(amount) + " " + name + " into the rune pouch.");
    	}
    
    	private static void takeOut(Player p, int slot) {
    		ArrayList<Integer> list = new ArrayList<>(p.runePouch.pouch.keySet());
    
    		int id = list.get(slot);
    
    		if (p.getInventory().getFreeSlots() > 0 || p.getInventory().contains(id)) {
    			int existing = p.runePouch.pouch.getOrDefault(id, 0);
    
    			if (existing == 0) {
    				return;
    			}
    
    			p.runePouch.pouch.remove(id);
    
    			p.getInventory().add(id, existing);
    
    			open(p);
    		} else {
    			p.getInventory().full();
    		}
    	}
    
    	private static void empty(Player p) {
    		Map<Integer, Integer> pouchCopy = new HashMap<>(p.runePouch.pouch);
    
    		for (Map.Entry<Integer, Integer> e : pouchCopy.entrySet()) {
    			int id = e.getKey();
    			int amount = e.getValue();
    
    			if (p.getInventory().getFreeSlots() > 0 || p.getInventory().contains(id)) {
    				p.runePouch.pouch.remove(id);
    				p.getInventory().add(id, amount);
    			}
    		}
    	}
    
    	public static boolean useRune(Player p, Item rune) {
    		int id = rune.getId();
    
    		if (!RUNE_IDS.contains(id)) {
    			return false;
    		}
    
    		if (!p.runePouch.pouch.containsKey(id)) {
    			return false;
    		}
    
    		int existing = p.runePouch.pouch.getOrDefault(id, 0);
    
    		if (rune.getAmount() > existing) {
    			return false;
    		}
    
    		p.runePouch.pouch.put(id, existing - rune.getAmount());
    		return true;
    	}
    
      @Override
    	public boolean handleItemInteraction(Player player, Item item, int type) {
    		if (item.getId() == RUNE_POUCH) {
    			if (type == 1) {
    				open(player);
    			} else if (type == 2) {
    				empty(player);
    			}
    			return true;
    		}
    		return false;
    	}
    
      @Override
    	public boolean handleItemOnItemInteraction(Player player, Item use, Item usedWith) {
    		if (usedWith.getId() == RUNE_POUCH) {
    			store(player, use);
    			return true;
    		}
    		return false;
    	}
    }
    Reply With Quote  
     

  2. #12  
    Chemist

    Advocatus's Avatar
    Join Date
    Dec 2009
    Posts
    2,624
    Thanks given
    202
    Thanks received
    813
    Rep Power
    1462
    Quote Originally Posted by Remi View Post
    Just use EnumMap instead of HashMap when using enums as keys,

    inside your enum instead of using private static final HashMap<Integer, Rune> FOR_ID = new HashMap<>();

    you would use private static final Map<Integer, Rune> FOR_ID =
    EnumSet.allOf(Rune.class).stream().collect(Collect ors.toMap(r -> r.id, r -> r));
    Thats not an enummap. Thats just a hashmap same as he was using. Is there a reason to use an enum set over just directly calling the stream off of the enum values array? If so, id like to know because i usually just call the stream directly off the values array.
    Quote Originally Posted by blakeman8192 View Post
    Quitting is the only true failure.
    Reply With Quote  
     

  3. #13  
    Registered Member
    Remi's Avatar
    Join Date
    Jan 2015
    Posts
    631
    Thanks given
    575
    Thanks received
    214
    Rep Power
    589
    Quote Originally Posted by Advocatus View Post
    Thats not an enummap. Thats just a hashmap same as he was using. Is there a reason to use an enum set over just directly calling the stream off of the enum values array? If so, id like to know because i usually just call the stream directly off the values array.
    You are right that using EnumSet.allOf(Rune.class).stream().collect(Collect ors.toMap(r -> r.id, r -> r)) creates a HashMap rather than an EnumMap. The main idea was to initialize the FOR_ID map efficiently.

    However, still converting the pouch itself to an EnumMap is highly optimized for enums when the keys are enums;

    private EnumMap<Rune, Integer> pouch = new EnumMap<>(Rune.class);

    Streaming and using an EnumSet are both suitable for creating the map, but streaming is slightly simpler. I used EnumSet because it makes the intent of working with all enum constants clear and provides better performance and maintainability.

    Quote Originally Posted by JayArrowz View Post
    Why use a enum at all?
    [/code]
    Limited functionality
    Where the fuck is my cigarettes, I need my cancer. [C]44..
    Reply With Quote  
     

  4. Thankful user:


  5. #14  
    Extreme Donator

    JayArrowz's Avatar
    Join Date
    Sep 2008
    Posts
    113
    Thanks given
    101
    Thanks received
    111
    Rep Power
    851
    Quote Originally Posted by Remi View Post
    You are right that using EnumSet.allOf(Rune.class).stream().collect(Collect ors.toMap(r -> r.id, r -> r)) creates a HashMap rather than an EnumMap. The main idea was to initialize the FOR_ID map efficiently.

    However, still converting the pouch itself to an EnumMap is highly optimized for enums when the keys are enums;

    private EnumMap<Rune, Integer> pouch = new EnumMap<>(Rune.class);

    Streaming and using an EnumSet are both suitable for creating the map, but streaming is slightly simpler. I used EnumSet because it makes the intent of working with all enum constants clear and provides better performance and maintainability.



    Limited functionality
    What are you talking about? I dont understand what you mean by limited functionality. There is no need to use a enum here
    Reply With Quote  
     

  6. Thankful user:


  7. #15  
    Registered Member

    Join Date
    Nov 2014
    Posts
    257
    Thanks given
    39
    Thanks received
    152
    Rep Power
    269
    Quote Originally Posted by Advocatus View Post
    Thats not an enummap. Thats just a hashmap same as he was using. Is there a reason to use an enum set over just directly calling the stream off of the enum values array? If so, id like to know because i usually just call the stream directly off the values array.
    I think its fine, either way, streams just make this pretty common operation into a 1-liner. But even using the static block to initialize is okay.

    Quote Originally Posted by JayArrowz View Post
    What are you talking about? I dont understand what you mean by limited functionality. There is no need to use a enum here
    Its definitely useful. Like in the second enum, you can have something like Rune#isRune which hides away implementation details. You could also have a Rune#forItemId which can either throw exception or return Optional. You can use a hashset, enumset, hashmap, whatever, it can change without it impacting your code.

    But beyond that, a rune enum could be useful for multiple reasons. we can use it to define runes for spells. We could have a system entire "CastSpell" code could directly interact via Runes without ever having to convert to item ids - at least when interacting with the rune pouch and not inventory.

    And its nice to have organizationally. There is literally no reason to prefer a hashset with a bunch of random numbers (but we added comments). Idk why we'd prefer that even if we do literally nothing else with this enum lol.
    Reply With Quote  
     

  8. Thankful users:


  9. #16  
    Extreme Donator

    JayArrowz's Avatar
    Join Date
    Sep 2008
    Posts
    113
    Thanks given
    101
    Thanks received
    111
    Rep Power
    851
    Quote Originally Posted by Intrice Joe View Post
    I think its fine, either way, streams just make this pretty common operation into a 1-liner. But even using the static block to initialize is okay.



    Its definitely useful. Like in the second enum, you can have something like Rune#isRune which hides away implementation details. You could also have a Rune#forItemId which can either throw exception or return Optional. You can use a hashset, enumset, hashmap, whatever, it can change without it impacting your code.

    But beyond that, a rune enum could be useful for multiple reasons. we can use it to define runes for spells. We could have a system entire "CastSpell" code could directly interact via Runes without ever having to convert to item ids - at least when interacting with the rune pouch and not inventory.

    And its nice to have organizationally. There is literally no reason to prefer a hashset with a bunch of random numbers (but we added comments). Idk why we'd prefer that even if we do literally nothing else with this enum lol.
    The enums main purpose is to store the rune ID's, and ensure the subsequent logic checks that the rune can be used with the pouch.
    You do not need a enum to store a simple "contains" functionality. You could even use a List, array (with a helper function) or a set.
    There is no point of using a enum, you do not need to strongly reference any of the enum items, just declare them as what they are an array of IDs.

    Infact in the enum the OP uses he even uses a type which he does not need and uses more than needed: HashMap<Integer, Rune>
    Why? Why doesn't the OP just use an array, list or set INSTEAD of transfering all the enum items to a HashMap to be able to use "contains"?


    Yes and "contains" is all he needs, because even when OP gets the rune for the HashMap he's only using the "contains" function AND the "Rune" object to save in json when he can use the ID directly and completely remove the useless rune enum and hashmap which makes the code overly complex

    Some chatgpt if my explanation is hard to understand:
    Attached image
    Last edited by JayArrowz; 05-19-2024 at 06:06 PM.
    Reply With Quote  
     

  10. Thankful user:


  11. #17  
    Community Veteran

    Dexter Morgan's Avatar
    Join Date
    Nov 2008
    Age
    29
    Posts
    4,431
    Thanks given
    1,209
    Thanks received
    763
    Rep Power
    3115
    Quote Originally Posted by JayArrowz View Post
    The enums main purpose is to store the rune ID's, and ensure the subsequent logic checks that the rune can be used with the pouch.
    You do not need a enum to store a simple "contains" functionality. You could even use a List, array (with a helper function) or a set.
    There is no point of using a enum, you do not need to strongly reference any of the enum items, just declare them as what they are an array of IDs.

    Infact in the enum the OP uses he even uses a type which he does not need and uses more than needed: HashMap<Integer, Rune>
    Why? Why doesn't the OP just use an array, list or set INSTEAD of transfering all the enum items to a HashMap to be able to use "contains"?


    Yes and "contains" is all he needs, because even when OP gets the rune for the HashMap he's only using the "contains" function AND the "Rune" object to save in json when he can use the ID directly and completely remove the useless rune enum and hashmap which makes the code overly complex

    Some chatgpt if my explanation is hard to understand:
    Attached image
    I have to agree with you. A Set of the rune ids would of been better than an whole enum.

    Thanks for your criticism!
    Reply With Quote  
     

  12. #18  
    Registered Member GameMaster's Avatar
    Join Date
    Jun 2015
    Posts
    160
    Thanks given
    48
    Thanks received
    23
    Rep Power
    71
    Quote Originally Posted by Remi View Post
    Just use EnumMap instead of HashMap when using enums as keys,

    inside your enum instead of using private static final HashMap<Integer, Rune> FOR_ID = new HashMap<>();

    you would use private static final Map<Integer, Rune> FOR_ID =
    EnumSet.allOf(Rune.class).stream().collect(Collect ors.toMap(r -> r.id, r -> r));

    than you would be able to call the rune like Rune rune = Rune.FOR_ID.get(item.getId());

    this allows for O(1) time when looking up a rune by its id

    for store, you would be able to use
    Code:
    private void store(Item item) {
        Rune rune = Rune.FOR_ID.get(item.getId());
        if (rune == null) {
            player.getPacketSender().sendMessage("You can only store runes into the rune pouch.");
            return;
        }
    
        if (!pouch.containsKey(rune) && pouch.size() == MAX_POUCH_SIZE) {
            player.getPacketSender().sendMessage("Your rune pouch is full.");
            return;
        }
    for take out you can streamline the removable of runes

    Code:
    private void takeOut(int slot) {
        Rune rune = new ArrayList<>(pouch.keySet()).get(slot);
        if (player.getInventory().hasFreeSlots() || player.getInventory().contains(rune.id)) {
            int amount = pouch.remove(rune);
            if (amount > 0) {
                player.getInventory().add(rune.id, amount);
                open();
            }
    for handleItemInteraction, you dont need a switch as its only one item id

    just use an if statement if its RUNE_POUCH

    Code:
    public boolean handleItemInteraction(Player player, Item item, int type) {
        if (item.getId() == RUNE_POUCH) {
            if (type == 1) {
                open();
            } else if (type == 2) {
                empty();
            }
            return true;
        }
    Code:
            private static final Map<Integer, Rune> FOR_ID = new EnumMap<>(Rune.class);
    
            static {
                for (Rune r : values()) {
                    FOR_ID.put(r.id, r);
                }
            }
    
            public static Rune getById(int id) {
                return FOR_ID.get(id);
            }
        }
    Reply With Quote  
     

  13. #19  
    Registered Member

    Join Date
    Nov 2014
    Posts
    257
    Thanks given
    39
    Thanks received
    152
    Rep Power
    269
    Quote Originally Posted by JayArrowz View Post
    The enums main purpose is to store the rune ID's, and ensure the subsequent logic checks that the rune can be used with the pouch.
    You do not need a enum to store a simple "contains" functionality. You could even use a List, array (with a helper function) or a set.
    There is no point of using a enum, you do not need to strongly reference any of the enum items, just declare them as what they are an array of IDs.

    Infact in the enum the OP uses he even uses a type which he does not need and uses more than needed: HashMap<Integer, Rune>
    Why? Why doesn't the OP just use an array, list or set INSTEAD of transfering all the enum items to a HashMap to be able to use "contains"?


    Yes and "contains" is all he needs, because even when OP gets the rune for the HashMap he's only using the "contains" function AND the "Rune" object to save in json when he can use the ID directly and completely remove the useless rune enum and hashmap which makes the code overly complex

    Some chatgpt if my explanation is hard to understand:
    Attached image
    true, we should design things only for the immediate use case with no consideration of how it may or may not be useful otherwise

    SomeSpell(cost=Rune.FIRE, Rune....)

    Or also runes can contain additional data within them for combination runes
    LAVA_RUNE(combination=FIRE, EARTH)

    But lets ignore that. Practically speaking, creating an enum here doesn't take any more actual dev time, and the enum is not what makes the code complex here lol. Things like strict typing and the code being more readable are widely more valuable than the least complex data structure. Much more preferrable to a bunch of lists/maps of integers hanging around only being known by comments and variable names.
    Reply With Quote  
     

  14. Thankful user:


  15. #20  
    WVWVWVWVWVWVWVW

    _jordan's Avatar
    Join Date
    Nov 2012
    Posts
    3,058
    Thanks given
    126
    Thanks received
    1,874
    Rep Power
    5000
    these dudes really arguing about an enum
    Attached image
    Attached image
    Attached image
    Reply With Quote  
     

  16. Thankful user:


Page 2 of 3 FirstFirst 123 LastLast

Thread Information
Users Browsing this Thread

There are currently 1 users browsing this thread. (0 members and 1 guests)


User Tag List

Similar Threads

  1. Looting bag / Rune Pouch
    By Matt _ in forum Buying
    Replies: 27
    Last Post: 01-28-2016, 11:16 PM
  2. [PI] Rune Pouch
    By Pandemia in forum Snippets
    Replies: 5
    Last Post: 01-22-2016, 01:55 PM
  3. working rune pouch
    By bruap in forum Requests
    Replies: 1
    Last Post: 12-28-2015, 11:42 PM
  4. Buying Rune pouch interface
    By Someone in forum Buying
    Replies: 2
    Last Post: 06-13-2015, 06:22 PM
  5. Replies: 6
    Last Post: 06-07-2015, 09:08 PM
Posting Permissions
  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •