From 4e421c1bd90ae3fc0bc3b74da404e35fe616ab54 Mon Sep 17 00:00:00 2001
From: Kirk Scheibelhut <scheibo@users.noreply.github.com>
Date: Sat, 23 Feb 2019 17:28:06 -0800
Subject: [PATCH] Refactor getEffect for speed safely (#5201)

Rollforward of 7a20245 which retains the `hasOwnProperty` checks.

Also changes the method to call `toId` earlier and use the id
as the key to the cache to ensure 'Stealth Rock' and 'stealthrock'
return the same. NOTE: 'move: Stealth Rock' and 'Stealth Rock' will
still continue to return different results.
---
 sim/dex.js                 | 28 ++++++++++++----------------
 test/simulator/misc/dex.js | 11 +++++++++++
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/sim/dex.js b/sim/dex.js
index 98b4c1da5f..f42fbf62e3 100644
--- a/sim/dex.js
+++ b/sim/dex.js
@@ -476,7 +476,8 @@ class ModdedDex {
 			return /** @type {PureEffect} */ (name);
 		}
 
-		let effect = this.effectCache.get(name);
+		let id = toId(name);
+		let effect = this.effectCache.get(id);
 		if (effect) {
 			return /** @type {PureEffect} */ (effect);
 		}
@@ -489,25 +490,20 @@ class ModdedDex {
 			effect = this.getAbility(name.slice(8));
 		}
 		if (effect) {
-			this.effectCache.set(name, effect);
+			this.effectCache.set(id, effect);
 			// @ts-ignore
 			return effect;
 		}
 
-		let id = toId(name);
-		if (this.data.Statuses.hasOwnProperty(id)) {
-			effect = new Data.PureEffect({name}, this.data.Statuses[id]);
-		} else if (this.data.Movedex.hasOwnProperty(id) && this.data.Movedex[id].effect) {
-			name = this.data.Movedex[id].name || name;
-			effect = new Data.PureEffect({name}, this.data.Movedex[id].effect);
-		} else if (this.data.Abilities.hasOwnProperty(id) && this.data.Abilities[id].effect) {
-			name = this.data.Abilities[id].name || name;
-			effect = new Data.PureEffect({name}, this.data.Abilities[id].effect);
-		} else if (this.data.Items.hasOwnProperty(id) && this.data.Items[id].effect) {
-			name = this.data.Items[id].name || name;
-			effect = new Data.PureEffect({name}, this.data.Items[id].effect);
-		} else if (this.data.Formats.hasOwnProperty(id)) {
+		let found;
+		if (this.data.Formats.hasOwnProperty(id)) {
 			effect = new Data.Format({name}, this.data.Formats[id]);
+		} else if (this.data.Statuses.hasOwnProperty(id)) {
+			effect = new Data.PureEffect({name}, this.data.Statuses[id]);
+		} else if ((this.data.Movedex.hasOwnProperty(id) && (found = this.data.Movedex[id]).effect) ||
+							 (this.data.Abilities.hasOwnProperty(id) && (found = this.data.Abilities[id]).effect) ||
+							 (this.data.Items.hasOwnProperty(id) && (found = this.data.Items[id]).effect)) {
+			effect = new Data.PureEffect({name: found.name || name}, found.effect);
 		} else if (id === 'recoil') {
 			effect = new Data.PureEffect({name: 'Recoil', effectType: 'Recoil'});
 		} else if (id === 'drain') {
@@ -516,7 +512,7 @@ class ModdedDex {
 			effect = new Data.PureEffect({name, exists: false});
 		}
 
-		this.effectCache.set(name, effect);
+		this.effectCache.set(id, effect);
 		return /** @type {PureEffect} */ (effect);
 	}
 	/**
diff --git a/test/simulator/misc/dex.js b/test/simulator/misc/dex.js
index 40ac190e96..fd747b2dc4 100644
--- a/test/simulator/misc/dex.js
+++ b/test/simulator/misc/dex.js
@@ -20,3 +20,14 @@ describe('Mod loader', function () {
 		}
 	});
 });
+
+describe('Dex#getEffect', function () {
+	it('returns the same object for the same id', function () {
+		assert.strictEqual(Dex.getEffect('Stealth Rock'), Dex.getEffect('stealthrock'));
+		assert.notStrictEqual(Dex.getEffect('move: Stealth Rock'), Dex.getEffect('stealthrock'));
+	});
+
+	it('does not return elements from the Object prototype', function () {
+		assert.false(Dex.getEffect('constructor').exists);
+	});
+});