CardPool Improvements (#598)

* Replace CardPool simple type alias with a Map subclass to add a card count() cache.
* Use it everywhere!

* Uncovered and fixed some broken tests.
* Some random cleanups while I'm there.

src/CardTypes.ts

@@ -82,7 +82,58 @@ export class Card {
 
 export type DeckBasicLands = { [key in CardColor]: number };
 
-export type CardPool = Map<string, number>;
+// Implements a MultiSet using a standard Map.
+export class CardPool extends Map<CardID, number> {
+	private _count = 0;
+
+	constructor() {
+		super();
+	}
+
+	// Set the number of copies of a card in the pool.
+	// Note: If count is <= 0, the card entry will be removed entirely.
+	set(cid: CardID, count: number) {
+		if (super.has(cid)) {
+			this._count -= super.get(cid)!;
+			if (count <= 0) super.delete(cid);
+		}
+		if (count > 0) {
+			super.set(cid, count);
+			this._count += count;
+		}
+		return this;
+	}
+
+	clear(): void {
+		super.clear();
+		this._count = 0;
+	}
+
+	delete(key: string): boolean {
+		const oldValue = super.get(key);
+		if (oldValue) this._count -= oldValue;
+		return super.delete(key);
+	}
+
+	// Remove a single copy of a card from the pool.
+	removeCard(cid: CardID) {
+		const oldValue = this.get(cid);
+		if (!oldValue) {
+			console.error(`Called removeCard on a non-existing card (${cid}).`);
+			console.trace();
+			throw `Called removeCard on a non-existing card (${cid}).`;
+		}
+		// Purposefully bypassing our caching overload and calling super.set() and super.delete() directly here.
+		if (oldValue === 1) super.delete(cid);
+		else super.set(cid, oldValue - 1);
+		--this._count;
+	}
+
+	count() {
+		return this._count;
+	}
+}
+
 export type SlotedCardPool = { [slot: string]: CardPool };
 export type DeckList = {
 	main: Array<CardID>;

src/Connection.ts

@@ -13,7 +13,7 @@ export class Connection {
 	userName: string;
 
 	sessionID?: SessionID = undefined;
-	collection: CardPool = new Map();
+	collection: CardPool = new CardPool();
 	useCollection = true;
 
 	pickedCards: { main: Array<UniqueCard>; side: Array<UniqueCard> } = { main: [], side: [] };

src/CustomCardList.ts

@@ -1,7 +1,7 @@
 import { ColorBalancedSlot } from "./BoosterFactory.js";
 import { CardID, Card, SlotedCardPool, UniqueCard, CardPool } from "./CardTypes.js";
 import { getCard } from "./Cards.js";
-import { pickCard, removeCardFromCardPool } from "./cardUtils.js";
+import { pickCard } from "./cardUtils.js";
 import { MessageError } from "./Message.js";
 import { isEmpty, Options, random, weightedRandomIdx, shuffleArray } from "./utils.js";
 
@@ -74,7 +74,7 @@ export function generateBoosterFromCustomCardList(
 
 		const cardsBySlot: SlotedCardPool = {};
 		for (const slotName in customCardList.slots) {
-			cardsBySlot[slotName] = new Map();
+			cardsBySlot[slotName] = new CardPool();
 			for (const cardId in customCardList.slots[slotName])
 				cardsBySlot[slotName].set(cardId, customCardList.slots[slotName][cardId]);
 		}
@@ -85,7 +85,7 @@ export function generateBoosterFromCustomCardList(
 			// however I don't have a better solution for now.
 			for (const slotName in cardsBySlot)
 				for (const cardId of options.removeFromCardPool)
-					if (cardsBySlot[slotName].has(cardId)) removeCardFromCardPool(cardId, cardsBySlot[slotName]);
+					if (cardsBySlot[slotName].has(cardId)) cardsBySlot[slotName].removeCard(cardId);
 		}
 
 		// Color balance the largest slot of each layout
@@ -222,7 +222,7 @@ export function generateBoosterFromCustomCardList(
 
 		// Generate fully random 15-cards booster for cube (not considering rarity)
 		// Getting custom card list
-		const localCollection: CardPool = new Map();
+		const localCollection: CardPool = new CardPool();
 
 		let cardCount = 0;
 		for (const cardId in defaultSlot) {
@@ -241,7 +241,7 @@ export function generateBoosterFromCustomCardList(
 		// Workaround to handle the LoreSeeker draft effect with a limited number of cards
 		if (!options.withReplacement && options.removeFromCardPool) {
 			for (const cardId of options.removeFromCardPool)
-				if (localCollection.has(cardId)) removeCardFromCardPool(cardId, localCollection);
+				if (localCollection.has(cardId)) localCollection.removeCard(cardId);
 		}
 
 		const boosters = [];

src/LandSlot.ts

@@ -1,9 +1,8 @@
 "use strict";
 
-import { CardID, CardPool } from "./CardTypes";
+import { CardID, CardPool } from "./CardTypes.js";
 import { getUnique, getCard } from "./Cards.js";
 import { getRandomMapKey, getRandom } from "./utils.js";
-import { removeCardFromCardPool } from "./cardUtils.js";
 import BasicLandIDs from "./data/BasicLandIDs.json" assert { type: "json" };
 
 export class BasicLandSlot {
@@ -22,7 +21,7 @@ export class BasicLandSlot {
 export class SpecialLandSlot extends BasicLandSlot {
 	commonLandsIds: Array<CardID>;
 	rate: number;
-	landsToDistribute: CardPool = new Map();
+	landsToDistribute: CardPool = new CardPool();
 
 	constructor(set: string, commonLandsIds: Array<CardID>, rate: number, basicLandsIds?: Array<CardID>) {
 		super(set);
@@ -43,7 +42,7 @@ export class SpecialLandSlot extends BasicLandSlot {
 	pick() {
 		if (Math.random() <= this.rate && this.landsToDistribute.size > 0) {
 			const c = getRandomMapKey(this.landsToDistribute);
-			removeCardFromCardPool(c, this.landsToDistribute);
+			this.landsToDistribute.removeCard(c);
 			return getUnique(c);
 		} else {
 			return getUnique(getRandom(this.basicLandsIds));

src/Session.ts

@@ -1,6 +1,5 @@
 "use strict";
 import { UserID, SessionID } from "./IDTypes.js";
-import { countCards } from "./cardUtils.js";
 import { shuffleArray, getRandom, arrayIntersect, Options, getNDisctinctRandom, pickRandom } from "./utils.js";
 import { Connections, getPickedCardIds } from "./Connection.js";
 import {
@@ -507,7 +506,7 @@ export class Session implements IIndexable {
 	// Returns current card pool according to all session options (Collections, setRestrictions...)
 	cardPool() {
 		if (this.unrestrictedCardPool()) {
-			const cardPool: CardPool = new Map();
+			const cardPool: CardPool = new CardPool();
 			// Returns all cards if there's no set restriction
 			if (this.setRestriction.length === 0) {
 				for (const [cid, card] of Cards)
@@ -528,12 +527,12 @@ export class Session implements IIndexable {
 	restrictedCollection(sets: Array<string>) {
 		const cardPool = this.collection();
 
-		const restricted: CardPool = new Map();
+		const restricted: CardPool = new CardPool();
 		if (sets && sets.length > 0) {
 			for (const s of sets)
 				if (s in CardsBySet)
 					for (const cid of CardsBySet[s].filter((cid) => cardPool.has(cid)))
-						restricted.set(cid, cardPool.get(cid) as number);
+						restricted.set(cid, cardPool.get(cid)!);
 				else console.error(`Session.restrictedCollection Error: '${s}' not in CardsBySet.`);
 			return restricted;
 		} else return cardPool;
@@ -542,7 +541,7 @@ export class Session implements IIndexable {
 	// Compute user collections intersection (taking into account each user preferences)
 	collection(inBoosterOnly = true): CardPool {
 		const user_list = [...this.users];
-		const collection: CardPool = new Map();
+		const collection: CardPool = new CardPool();
 
 		const useCollection = [];
 		for (let i = 0; i < user_list.length; ++i)
@@ -562,29 +561,26 @@ export class Session implements IIndexable {
 
 		// Compute the minimum count of each remaining card
 		for (const c of intersection) {
-			collection.set(c, useCollection[0] ? (Connections[user_list[0]].collection.get(c) as number) : 4);
+			collection.set(c, useCollection[0] ? Connections[user_list[0]].collection.get(c)! : 4);
 			for (let i = 1; i < user_list.length; ++i)
 				if (useCollection[i])
-					collection.set(
-						c,
-						Math.min(collection.get(c) as number, Connections[user_list[i]].collection.get(c) as number)
-					);
+					collection.set(c, Math.min(collection.get(c)!, Connections[user_list[i]].collection.get(c)!));
 		}
 		return collection;
 	}
 
 	// Categorize card pool by rarity
 	cardPoolByRarity(): SlotedCardPool {
 		const cardPoolByRarity: SlotedCardPool = {
-			common: new Map(),
-			uncommon: new Map(),
-			rare: new Map(),
-			mythic: new Map(),
+			common: new CardPool(),
+			uncommon: new CardPool(),
+			rare: new CardPool(),
+			mythic: new CardPool(),
 		};
 		const cardPool = this.cardPool();
 		for (const [cid, count] of cardPool) {
 			const rarity = getCard(cid).rarity;
-			if (!(rarity in cardPoolByRarity)) cardPoolByRarity[rarity] = new Map();
+			if (!(rarity in cardPoolByRarity)) cardPoolByRarity[rarity] = new CardPool();
 			cardPoolByRarity[rarity].set(cid, Math.min(count, this.maxDuplicates?.[rarity] ?? 99));
 		}
 		return cardPoolByRarity;
@@ -593,14 +589,14 @@ export class Session implements IIndexable {
 	// Returns all cards from specified set categorized by rarity and set to maxDuplicates
 	setByRarity(set: string) {
 		const local: SlotedCardPool = {
-			common: new Map(),
-			uncommon: new Map(),
-			rare: new Map(),
-			mythic: new Map(),
+			common: new CardPool(),
+			uncommon: new CardPool(),
+			rare: new CardPool(),
+			mythic: new CardPool(),
 		};
 		for (const cid of BoosterCardsBySet[set]) {
 			const rarity = getCard(cid).rarity;
-			if (!(rarity in local)) local[rarity] = new Map();
+			if (!(rarity in local)) local[rarity] = new CardPool();
 			local[rarity].set(cid, this.maxDuplicates?.[rarity] ?? 99);
 		}
 		return local;
@@ -701,16 +697,16 @@ export class Session implements IIndexable {
 				);
 				// Make sure we have enough cards
 				for (const slot of ["common", "uncommon", "rare"]) {
-					const card_count = countCards((defaultFactory as BoosterFactory).cardPool[slot]);
-					const card_target =
+					const cardCount = (defaultFactory as BoosterFactory).cardPool[slot].count();
+					const cardTarget =
 						targets[slot] *
 						(boosterSpecificRules
 							? boosterQuantity
 							: customBoosters.reduce((a, v) => (v === "" ? a + 1 : a), 0));
-					if (card_count < card_target)
+					if (cardCount < cardTarget)
 						return new MessageError(
 							"Error generating boosters",
-							`Not enough cards (${card_count}/${card_target} ${slot}s) in collection.`
+							`Not enough cards (${cardCount}/${cardTarget} ${slot}s) in collection.`
 						);
 				}
 			}
@@ -770,7 +766,7 @@ export class Session implements IIndexable {
 					const multiplier = customBoosters.reduce((a, v) => (v === boosterSet ? a + 1 : a), 0); // Note: This won't be accurate in the case of 'random' sets.
 					for (const slot of ["common", "uncommon", "rare"]) {
 						if (
-							countCards((usedSets[boosterSet] as BoosterFactory).cardPool[slot]) <
+							(usedSets[boosterSet] as BoosterFactory).cardPool[slot].count() <
 							multiplier * playerCount * targets[slot]
 						)
 							return new MessageError(
@@ -921,10 +917,10 @@ export class Session implements IIndexable {
 
 		// Add a new card to skipped pile. (Make sure there's enough cards for the player to draw if this is the last pile)
 		if (s.cardPool.length > 1 || (s.currentPile < 2 && s.cardPool.length > 0))
-			s.piles[s.currentPile].push(s.cardPool.pop() as UniqueCard);
+			s.piles[s.currentPile].push(s.cardPool.pop()!);
 		// Give a random card from the card pool if this was the last pile
 		if (s.currentPile === 2) {
-			const card = s.cardPool.pop() as UniqueCard;
+			const card = s.cardPool.pop()!;
 			Connections[s.currentPlayer()].pickedCards.main.push(card);
 			Connections[s.currentPlayer()].socket.emit("winstonDraftRandomCard", card);
 			this.draftLog?.users[s.currentPlayer()].picks.push({
@@ -949,7 +945,7 @@ export class Session implements IIndexable {
 			piles: [...s.piles.map((p, idx) => p.slice(0, idx < s.currentPile ? -1 : undefined).map((c) => c.id))],
 		});
 		Connections[s.currentPlayer()].pickedCards.main.push(...s.piles[s.currentPile]);
-		if (s.cardPool.length > 0) s.piles[s.currentPile] = [s.cardPool.pop() as UniqueCard];
+		if (s.cardPool.length > 0) s.piles[s.currentPile] = [s.cardPool.pop()!];
 		else s.piles[s.currentPile] = [];
 		this.winstonNextRound();
 		return true;
@@ -1251,8 +1247,8 @@ export class Session implements IIndexable {
 			//                       Column           Row
 			const idx = choice < 3 ? 3 * i + choice : 3 * (choice - 3) + i;
 			if (s.boosters[0][idx] !== null) {
-				Connections[s.currentPlayer()].pickedCards.main.push(s.boosters[0][idx] as UniqueCard);
-				pickedCards.push(s.boosters[0][idx] as UniqueCard);
+				Connections[s.currentPlayer()].pickedCards.main.push(s.boosters[0][idx]!);
+				pickedCards.push(s.boosters[0][idx]!);
 				log.pick.push(idx);
 				s.boosters[0][idx] = null;
 			}

src/cardUtils.ts

@@ -4,21 +4,18 @@ import { CardID, Card, CardPool } from "./CardTypes.js";
 import { getUnique } from "./Cards.js";
 import { getRandomMapKey, random } from "./utils.js";
 
-export function removeCardFromCardPool(cid: CardID, dict: CardPool) {
-	if (!dict.has(cid)) {
-		console.error(`Called removeCardFromCardPool on a non-existing card (${cid}).`);
-		console.trace();
-		throw `Called removeCardFromCardPool on a non-existing card (${cid}).`;
-	}
-	const newValue = dict.get(cid)! - 1;
-	if (newValue > 0) dict.set(cid, newValue);
-	else dict.delete(cid);
-}
-
 // Returns a random card from the pool, choosen uniformly across ALL cards (not UNIQUE ones),
 // meaning cards present in multiple copies are more likely to be picked.
 function getRandomCardFromCardPool(cardPool: CardPool): CardID {
-	const idx = random.integer(0, countCards(cardPool) - 1);
+	const idx = random.integer(0, cardPool.count() - 1);
+
+	// Fast path (kinda, sadly we can't directly index into a map) for the singleton case.
+	if (cardPool.size === cardPool.count()) {
+		const r = cardPool.keys();
+		for (let i = 0; i < idx; ++i) r.next();
+		return r.next().value;
+	}
+
 	let acc = 0;
 	for (const [cid, count] of cardPool) {
 		acc += count;
@@ -60,18 +57,12 @@ export function pickCard(
 				(cid) => booster.findIndex((card) => cid === card.id) === -1
 			);
 			if (candidates.length > 0) {
-				const tmpMap = new Map();
-				for (const cid of candidates) tmpMap.set(cid, cardPool.get(cid) as number);
-				cid = randomFunc(tmpMap);
+				const tmpPool = new CardPool();
+				for (const cid of candidates) tmpPool.set(cid, cardPool.get(cid)!);
+				cid = randomFunc(tmpPool);
 			}
 		}
 	}
-	if (options?.withReplacement !== true) removeCardFromCardPool(cid, cardPool);
+	if (options?.withReplacement !== true) cardPool.removeCard(cid);
 	return getUnique(cid, options);
 }
-
-export function countCards(dict: CardPool): number {
-	let acc = 0;
-	for (const v of dict.values()) acc += v;
-	return acc;
-}

src/server.ts

@@ -205,7 +205,7 @@ function setCollection(
 ) {
 	if (!isObject(collection) || collection === null) return;
 
-	const processedCollection: CardPool = new Map();
+	const processedCollection: CardPool = new CardPool();
 	// Remove unknown cards immediatly.
 	for (const aid in collection) {
 		if (aid in MTGACards) {

test/collection.ts

@@ -54,6 +54,15 @@ describe("Collection Restriction", function () {
 		waitForClientDisconnects(done);
 	});
 
+	it(`Disable ignoreCollections`, function (done) {
+		ownerIdx = clients.findIndex((c) => getUID(c) === Sessions[sessionID].owner);
+		clients[(ownerIdx + 1) % clients.length].once("ignoreCollections", (value) => {
+			expect(value).to.be.false;
+			done();
+		});
+		clients[ownerIdx].emit("ignoreCollections", false);
+	});
+
 	it(`Submit random collections.`, function (done) {
 		collections = Array(clients.length).fill({});
 		// Generate random collections
@@ -75,7 +84,7 @@ describe("Collection Restriction", function () {
 
 	it(`All cards in Session collection should be in all user collections.`, function (done) {
 		const sessColl = Sessions[sessionID].collection();
-		for (const cid in sessColl) {
+		for (const [cid /*count*/] of sessColl) {
 			const arena_id = getCard(cid).arena_id!;
 			for (const col of collections) {
 				expect(col).to.have.own.property(arena_id.toString());
@@ -87,7 +96,7 @@ describe("Collection Restriction", function () {
 
 	it(`All cards in Session Card Pool (e.g. Session collection with applied set restrictions) should be in all user collections.`, function (done) {
 		const sessCardPool = Sessions[sessionID].cardPool();
-		for (const cid in sessCardPool) {
+		for (const [cid /*count*/] of sessCardPool) {
 			const arena_id = getCard(cid).arena_id!;
 			for (const col of collections) {
 				expect(col).to.have.own.property(arena_id.toString());