Thread: Imbued Godcape Exchange

Page 1 of 2 12 LastLast
Results 1 to 10 of 16
  1. #1 Imbued Godcape Exchange 
    Extreme Donator

    Join Date
    Apr 2015
    Posts
    369
    Thanks given
    215
    Thanks received
    79
    Rep Power
    74
    Here is a snippet of a feature I coded today, I was trying to think of a way to implement the new godcapes, We decided to implement them from a basic, Pay and upgrade feature.

    I never release work on rune-server, I've decided I'm going to start contributing aswell.

    I OVER COMMENTED THIS FOR NEWBIES, I never comment like this in real life...
    its a basic feature, Use the item on an NPC or how ever you want to exchange said items,

    IMO this is a good example of how you should approach tasks regardless of how simple they are. This is clean and easy to read and modify.

    Code:
     
    package com.osrsunity.game.items.exchanges;
    
    import com.osrsunity.game.character.player.Player;
    import com.osrsunity.game.character.player.content.dialogue.Dialogue;
    import com.osrsunity.game.character.player.content.dialogue.Expression;
    import com.osrsunity.game.character.player.content.dialogue.Type;
    import com.osrsunity.game.items.ItemAssistant;
    
    /**
     * GodCape Exchange to bring the new godcapes in to the game.
     * 
     * @author Durky
     *
     */
    public class GodcapeExchange {
    	public enum GodCapes {
    		Saradomin(2412, 21791, 13331, 13332, 21776, 21778), 
    		Guthix(2413, 21793, 13335, 13336, 21784,21786), 
    		Zamorak(2414, 21795, 13333, 13334, 21780, 21782);
    
    		private int regularCape;
    		private int imbuedRegularCape;
    		private int maxCape;
    		private int maxHood;
    		private int imbuedMaxCape;
    		private int imbuedMaxHood;
    
    		GodCapes(int regularCape, int imbuedRegularCape, int maxCape, int maxHood, int imbuedCape, int imbuedHood) {
    			this.regularCape = regularCape;
    			this.imbuedRegularCape = imbuedRegularCape;
    			this.maxCape = maxCape;
    			this.maxHood = maxHood;
    			this.imbuedMaxCape = imbuedCape;
    			this.imbuedMaxHood = imbuedHood;
    
    		}
    
    		// small static iteration hurts nothing, No point in a set with such a
    		// small use feature.
    		public static GodCapes forId(int cape, boolean lookForMax) {
    			for (GodCapes godcape : GodCapes.values()) {
    				if(lookForMax ? godcape.maxCape == cape: godcape.regularCape == cape)
    					return godcape;
    			}
    			return null;
    		}
    
    	}
    
    	private static int COST = 25_000_000; // cost of the upgrade.
    
    	/*
    	 * Call me with item on NPC or how ever you want to handle it.
    	 */
    	public static void exchangeCape(Player player, int cape) {
    		boolean maxCape = false;
    		// look for regular first
    		GodCapes godcape = GodCapes.forId(cape, maxCape);
    
    		// if null look for max
    		if (godcape == null) {
    			maxCape = true;
    			godcape = GodCapes.forId(cape, maxCape);
    		}
    		// Yes we got a cape, so lets continue on.
    		if (godcape != null) {
    			if (player.getItems().playerHasItem(995, COST)) { //
    				// ALWAYS check to make sure the player has the item, when ever
    				// you write code.
    				// prevents dupes, and spamm clicks abuse.
    				if (player.getItems().playerHasItem(maxCape ? godcape.maxCape : godcape.regularCape, 1)) {
    					if (maxCape && !player.getItems().playerHasItem(godcape.maxHood, 1)) {
    						player.sendMessage("You need to have the max hood aswell to upgrade.");
    						return;
    					}
    					player.getItems().deleteItem(995, COST);
    					// Don't need to check for room when we are swapping items,
    					// Just delete first.
    					player.getItems().deleteItem(maxCape ? godcape.maxCape : godcape.regularCape, 1);
    					player.getItems().addItem(maxCape ? godcape.imbuedMaxCape : godcape.imbuedRegularCape, 1);
    					// tell the player
    					player.sendMessage("You exchange your " + ItemAssistant.getItemName(cape) + " for a "
    							+ ItemAssistant.getItemName((maxCape ? godcape.imbuedMaxCape : godcape.imbuedRegularCape))
    							+ "");
    
    					if (maxCape) {
    						// Don't forget about the hood!
    						player.getItems().deleteItem(godcape.maxHood, 1);
    						player.getItems().addItem(godcape.imbuedMaxHood, 1);
    					}
    				}
    			} else
    				// sorry this cost money
    				player.dialogue().start(new Dialogue() {
    
    					@Override
    					protected void start(Object... parameters) {
    						send(Type.NPC, 1603, Expression.CALM_TALK1, "To upgrade a god cape it cost 25m ");
    					}
    
    				});
    		} else {
    			// remind the player
    			player.dialogue().start(new Dialogue() {
    
    				@Override
    				protected void start(Object... parameters) {
    					send(Type.NPC, 1603, Expression.CALM_TALK1, "Please use a Saradomin, Guthix, or Zamorak ",
    							"max or regular cape on me to upgrade");
    				}
    
    			});
    		}
    
    	}
    
    }
    i'm not a professional developer, I'm a self taught programmer and if you have suggestions to any code i've written please post below.
    Reply With Quote  
     

  2. #2  
    'Slutty McFur'

    Owain's Avatar
    Join Date
    Sep 2014
    Age
    26
    Posts
    2,894
    Thanks given
    2,360
    Thanks received
    2,200
    Rep Power
    5000
    Thanks for the share, will give the noobs something to copy/paste

    Quote Originally Posted by durky View Post
    i'm not a professional developer
    I'm a bit worried that you're charging the prices you are on your other thread then.


    Spoiler for wat:
    Attached image
    Attached image

    Attached image


    Reply With Quote  
     

  3. Thankful users:


  4. #3  
    Respected Member


    Kris's Avatar
    Join Date
    Jun 2016
    Age
    26
    Posts
    3,638
    Thanks given
    820
    Thanks received
    2,642
    Rep Power
    5000
    Code:
    		public static GodCapes forId(int cape) {
    			for (GodCapes godcape : GodCapes.values()) {
    				if (godcape.regularCape == cape || godcape.maxCape == cape)
    					return godcape;
    			}
    			return null;
    		}
    Don't need the random nonsense you had there. If you want the boolean, you can simply check something like..
    ItemDefinition.getItemDefinitions(capeId).getName( ).contains("max")
    and you'll know whether it's a max cape or not. The way you have it right now looks a bit weird IMO.
    Attached image
    Reply With Quote  
     

  5. Thankful user:


  6. #4  
    Extreme Donator

    Join Date
    Apr 2015
    Posts
    369
    Thanks given
    215
    Thanks received
    79
    Rep Power
    74
    Quote Originally Posted by Kris View Post
    Code:
    		public static GodCapes forId(int cape) {
    			for (GodCapes godcape : GodCapes.values()) {
    				if (godcape.regularCape == cape || godcape.maxCape == cape)
    					return godcape;
    			}
    			return null;
    		}
    Don't need the random nonsense you had there. If you want the boolean, you can simply check something like..
    ItemDefinition.getItemDefinitions(capeId).getName( ).contains("max")
    and you'll know whether it's a max cape or not. The way you have it right now looks a bit weird IMO.
    I did not think of that, thanks.
    Reply With Quote  
     

  7. #5  
    Software Developer

    Tyrant's Avatar
    Join Date
    Jul 2013
    Age
    24
    Posts
    1,562
    Thanks given
    678
    Thanks received
    423
    Rep Power
    1060
    As mentioned above, the check is not too necessary and can be done differently, however, if you still chose to stay with that design,
    you might do want to cache the values somewhere, rather than keep creating the array of values (everytime #values() is being called).
    Just because there are "a few" values it means you shouldn't do that, you should, because it's called frequently.


    Also, some of the naming conventions should be changed, such as
    Code:
    public static int COST = 25_000_000;
    should be 'cost' or to be exact, declared as final.

    Overall, its not too bad, could've been designed differently probably as all it is is just an item combination system,
    should have a "generic" one that can be used for other item combinations, with appropriate dialogues.

    Thanks tho
    Reply With Quote  
     

  8. #6  
    Registered Member
    Exipe's Avatar
    Join Date
    Sep 2015
    Age
    24
    Posts
    150
    Thanks given
    29
    Thanks received
    59
    Rep Power
    202
    Quote Originally Posted by durky View Post
    I did not think of that, thanks.
    there's a reason we use integers to identify items & not their names. there's nothing wrong with you originally did.
    how ever, the name of your enum (GodCapes) should be in singular (GodCape) for the same reason class names are. it simply names the type.
    also you're calling player.getItems() an awful lot. should instead call it once & store it in a local variable of the type ItemHandler
    Reply With Quote  
     

  9. #7  
    Extreme Donator

    Join Date
    Apr 2015
    Posts
    369
    Thanks given
    215
    Thanks received
    79
    Rep Power
    74
    Quote Originally Posted by Exipe View Post
    there's a reason we use integers to identify items & not their names. there's nothing wrong with you originally did.
    how ever, the name of your enum (GodCapes) should be in singular (GodCape) for the same reason class names are. it simply names the type.
    also you're calling player.getItems() an awful lot. should instead call it once & store it in a local variable of the type ItemHandler
    Can you clarify this?
    but becasue the value is already stored in player, and calling it with player.getItems() vs creating a temp itemAssistant item = player.getItems();
    Doesn't change anything?
    Reply With Quote  
     

  10. #8  
    Donator

    Jason's Avatar
    Join Date
    Aug 2009
    Posts
    6,092
    Thanks given
    2,402
    Thanks received
    2,823
    Rep Power
    4550
    Quote Originally Posted by Tyrant View Post
    As mentioned above, the check is not too necessary and can be done differently, however, if you still chose to stay with that design,
    you might do want to cache the values somewhere, rather than keep creating the array of values (everytime #values() is being called).
    Just because there are "a few" values it means you shouldn't do that, you should, because it's called frequently.
    Even though values() is added at compile time so it's impossible to prove, I would be absolutely shocked if values() didn't reference a field, even if it's lazy initialized like Class#enumConstantsDirectory for Enum#valueOf. Do you have any sources that indicate whether or not the reference is cached or if it's created each time values() is referenced?
    Reply With Quote  
     

  11. #9  
    Registered Member
    Exipe's Avatar
    Join Date
    Sep 2015
    Age
    24
    Posts
    150
    Thanks given
    29
    Thanks received
    59
    Rep Power
    202
    Quote Originally Posted by durky View Post
    Can you clarify this?
    but becasue the value is already stored in player, and calling it with player.getItems() vs creating a temp itemAssistant item = player.getItems();
    Doesn't change anything?
    you wouldn't notice any significant difference in performance.
    just looks considerably better
    Reply With Quote  
     

  12. #10  
    Respected Member


    Join Date
    Jan 2009
    Posts
    5,743
    Thanks given
    1,162
    Thanks received
    3,603
    Rep Power
    5000
    Quote Originally Posted by Jason View Post
    Even though values() is added at compile time so it's impossible to prove, I would be absolutely shocked if values() didn't reference a field, even if it's lazy initialized like Class#enumConstantsDirectory for Enum#valueOf. Do you have any sources that indicate whether or not the reference is cached or if it's created each time values() is referenced?
    values() will create a new copy of the array every time you call it. I think it'd be fine for the usage OP is using and would have very little if any impact on performance.

    I personally don't like the nested if statements or the ternary statements everywhere, not sure why this snippet was released to be honest, I was expecting to see the real magic arena 2 exchanging
    Last edited by Spooky; 12-08-2017 at 07:27 PM.
    Reply With Quote  
     

  13. Thankful user:


Page 1 of 2 12 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. Skill Cape exchanger (Silab client)
    By MuZzA in forum Tutorials
    Replies: 7
    Last Post: 07-04-2008, 10:20 AM
  2. Universal User Exchange - New!
    By DJ Dan in forum RS2 Server
    Replies: 39
    Last Post: 06-26-2008, 10:27 PM
  3. Sigex - Castle wars Ticket Exchange 100%
    By littleplop in forum Tutorials
    Replies: 60
    Last Post: 02-22-2008, 12:49 PM
  4. 1 Item Exchange Command :d
    By Aggi in forum Tutorials
    Replies: 23
    Last Post: 02-09-2008, 08:43 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
  •