Thread: [667/***] Ash scattering

Originally Posted by MadDoggzz

if (ash.getId() == 20264) {
player.setNextGraphics(new Graphics(56));
} else if (ash.getId() == 20266) {
player.setNextGraphics(new Graphics(47));
} else if (ash.getId() == 20268) {
player.setNextGraphics(new Graphics(40));
}

Why not include this in the enumeration instead of making a bunch of if statements for each type of ash? That said if this is just for ashes and there are only 3 types why not include this as another type of bone and add parameters to burying the bone to include graphic and animation?

Thanks for contributing this to the community none the less, good job friend

This was done already [Only registered and activated users can see links. ]

Code:

public long getAshDelay() {
		return 0;
	}

	public void addAshDelay(int i) {	
	}

???

• as kaleem said remove the unused methods
• store the graphic in the enum def!
• remove either of the two logic methods
• you're locked for 3 ticks
• xp is added immediately on rs
• item is removed instantly too on rs
• message is filterable

idk if u wanted c&c but gj nonetheless

Code:

// Ash Scattering
// 9 November, 2014
// Bart Pelle

.enum "core.skill_ids" as Skill


on item_option_1(20264) { scatter(20264, 40, 56); } // Impious ashes
on item_option_1(20266) { scatter(20266, 125, 47); } // Accursed ashes
on item_option_1(20268) { scatter(20268, 625, 40); } // Infernal ashes
on item_option_1(34159) { scatter(34159, 2000, 56); } // Searing ashes

proc scatter(int item, int xp, int graphic) {
	lock();
	animate(445);
	graphic(graphic);
	message("You scatter the ashes in the wind.", true);
	add_xp(Skill::PRAYER, xp, true);
	//message("You gain extra experience as the ancestral barbarian spirit watches over you.");
	delete_item(item, 1);
	wait(3);
	unlock();
}

Point #1: Cache your values.

Originally Posted by MadDoggzz

Code:

		private static Map < Integer, Ash > ashes = new HashMap < Integer, Ash > ();

		static {
			for (Ash ash: Ash.values()) {
				ashes.put(ash.getId(), ash);
			}
		}

Some advice I heard from Velocity was to cache these values.

Spoiler for how you would go about doing this:

Code:

		private static Map < Integer, Ash > ashes = new HashMap < Integer, Ash > ();
		private static Ash[] values = values();
		public static Ash[] valuesCached() {
			return values;
		}

		static {
			for (Ash ash: valuesCached()) {
				ashes.put(ash.getId(), ash);
			}
		}

Atleast, that's how I think is the way to go about that. Somebody do correct me.

Point #2: Why do you have have two scatter methods?

Consider removing the scatter method from Ash, because it shouldn't belong there.

Spoiler for how you would go about fixing this:

Remove the scatter method in Ash, then change the code in InventoryOptionsHandler.

Code:

Ash ash = Ash.forId(itemId);
if (ash != null) {
	AshScattering.scatter(player, slotId);
	return;
}

Point #3: Utilize your enumeration more.

Originally Posted by MadDoggzz

Code:

		if (ash.getId() == 20264) {
			player.setNextGraphics(new Graphics(56));
		} else if (ash.getId() == 20266) {
			player.setNextGraphics(new Graphics(47));
		} else if (ash.getId() == 20268) {
			player.setNextGraphics(new Graphics(40));
		}

Add the GFX to the enumeration. These if-statements are kind of ruining the purpose of the enumeration..

Spoiler for how you would go about doing this:

Code:

	public enum Ash {
		IMPIOUS(20264, 4, new Graphics(56)),
		ACCURSED(20266, 12, new Graphics(47)),
		INFERNAL(20268, 62, new Graphics(40));

		private int id;
		private double experience;
		private Graphics graphics;

		private static Map < Integer, Ash > ashes = new HashMap < Integer, Ash > ();
		private static Ash[] values = values();
		public static Ash[] valuesCached() {
			return values;
		}

		static {
			for (Ash ash: valuesCached()) {
				ashes.put(ash.getId(), ash);
			}
		}

		public static Ash forId(int id) {
			return ashes.get(id);
		}

		private Ash(int id, double experience, Graphics graphics) {
			this.id = id;
			this.experience = experience;
			this.graphics = graphics;
		}

		public int getId() {
			return id;
		}

		public double getExperience() {
			return experience;
		}

		public Graphics getGraphics() {
			return graphics;
		}
	}

Replace:

Code:

		if (ash.getId() == 20264) {
			player.setNextGraphics(new Graphics(56));
		} else if (ash.getId() == 20266) {
			player.setNextGraphics(new Graphics(47));
		} else if (ash.getId() == 20268) {
			player.setNextGraphics(new Graphics(40));
		}

to:

Code:

		player.setNextGraphics(ash.getGraphics());

Pet peeve #1: Instead of suppressing warnings with @SuppressWarnings("unused"), remove the unused variable.

Spoiler for how you would go about doing this:

Remove:

Code:

@SuppressWarnings("unused")
final ItemDefinitions itemDef = new ItemDefinitions(item.getId());

Originally Posted by Velocity

• as kaleem said remove the unused methods
• store the graphic in the enum def!
• remove either of the two logic methods
• you're locked for 3 ticks
• xp is added immediately on rs
• item is removed instantly too on rs
• message is filterable

idk if u wanted c&c but gj nonetheless

Code:

// Ash Scattering
// 9 November, 2014
// Bart Pelle

.enum "core.skill_ids" as Skill


on item_option_1(20264) { scatter(20264, 40, 56); } // Impious ashes
on item_option_1(20266) { scatter(20266, 125, 47); } // Accursed ashes
on item_option_1(20268) { scatter(20268, 625, 40); } // Infernal ashes
on item_option_1(34159) { scatter(34159, 2000, 56); } // Searing ashes

proc scatter(int item, int xp, int graphic) {
	lock();
	animate(445);
	graphic(graphic);
	message("You scatter the ashes in the wind.", true);
	add_xp(Skill::PRAYER, xp, true);
	//message("You gain extra experience as the ancestral barbarian spirit watches over you.");
	delete_item(item, 1);
	wait(3);
	unlock();
}

is that rune-script or velocity-script what the hell?

Originally Posted by MadDoggzz

Something like this then?

Code:

package com.rs.game.player.content;

import java.util.HashMap;
import java.util.Map;

import com.rs.game.Animation;
import com.rs.game.Graphics;
import com.rs.game.item.Item;
import com.rs.game.player.Player;
import com.rs.game.player.Skills;
import com.rs.game.tasks.WorldTask;
import com.rs.game.tasks.WorldTasksManager;
import com.rs.utils.Utils;

public class AshScattering {

	public enum Ash {
		IMPIOUS(20264, 4, new Graphics(56)),
		ACCURSED(20266, 12, new Graphics(47)),
		INFERNAL(20268, 62, new Graphics(40));

		private int id;
		private double experience;
		private Graphics graphics;

		private static Map < Integer, Ash > ashes = new HashMap < Integer, Ash > ();
		private static Ash[] values = values();
		public static Ash[] valuesCached() {
			return values;
		}

		static {
			for (Ash ash: valuesCached()) {
				ashes.put(ash.getId(), ash);
			}
		}

		public static Ash forId(int id) {
			return ashes.get(id);
		}

		private Ash(int id, double experience, Graphics graphics) {
			this.id = id;
			this.experience = experience;
			this.graphics = graphics;
		}

		public int getId() {
			return id;
		}

		public double getExperience() {
			return experience;
		}

		public Graphics getGraphics() {
			return graphics;
		}
	}
	
	public static boolean scatter(final Player player, int slotId) {
		final Item item = player.getInventory().getItem(slotId);
		if (item == null || Ash.forId(item.getId()) == null)
			return false;
		if (player.getAshDelay() > Utils.currentTimeMillis())
			return true;
		final Ash ash = Ash.forId(item.getId());
		player.addStopDelay(4);
		player.setNextAnimation(new Animation(445));
		player.setNextGraphics(ash.getGraphics());
		WorldTasksManager.schedule(new WorldTask() {
			@Override
			public void run() {
				player.getInventory().deleteItem(item.getId(), 1);
				player.getPackets().sendGameMessage("You scatter the ashes in the wind.");
				player.getSkills().addXp(Skills.PRAYER, ash.getExperience());
				stop();
			}

		}, 2);
		return false;
	}
}

Precisely.