0 votes
by (220 points)

I am using modified inventory system from https://twinery.org/forum/discussion/8215/inventory-system-sugarcube-2-0-twine-2. I am no a coder, but my current code works.   

I have showInv macro that displays all items in categories. All categories are ordered according to array $categories, items are ordered as received. If inventory has no item from a category, that category isn't displayed. My current code seems very inefficient, especially that I am using forEach for the same array twice - first time to get categories present, second to get data to actually display.

I am looking for ideas how to improve showInv macro code.

Part of StoryInit passage:

<<initInv>>
<<set $categories = [ "Cats", "Fruits", "Pies", "Horrors" ]>>
<<set
$apple to { id:"1", name:"Apple", desc:"An apple, red and wormy.", icon:"/icons/blank.png", category:"Fruits", use:"Apple", count:"" }
$pear to { id:"2", name:"Pear", desc:"A pear, whatever that is.", icon:"/icons/blank.png", category:"Fruits", use:"", count:"" }
$banana to { id:"3", name:"Banana", desc:"Lusciosly curved bannana with sensualy yellow peel.", icon:"/icons/blank.png", category:"Fruits", use:"", count:"" },
$kitten to { id:"4", name:"Kitten", desc:"A small fuzzy feline.", icon:"/icons/blank.png", category:"Cats", use:"", count:"" },
$apple_pie to { id:"4", name:"Apple Pie", desc:"Pie made from red and wormy apples.", icon:"/icons/blank.png", category:"Pies", use:"", count:"" }
>>

Part of Story JavaScript:

Macro.add("initInv", {
	handler: function () {
		State.variables.inventory = [];
	}
});
Macro.add('showInv', {
	handler: function () {
		var $list = $(document.createDocumentFragment());
		var current = [];
		State.variables.inventory.forEach(function (item) {
			current.push(item.category);
		});
		State.variables.categories.forEach (function (cat) {
			if (current.includes(cat)) {
				if (cat !== State.variables.categories[0]) {
					$list.wiki('<br>');
				}
				$list.wiki('<b>' + cat + '</b><br>');
				State.variables.inventory.forEach(function (item) {
					if (item.category === cat) {
						$list.wiki(item.name + ' ×' + item.count + ' ');
					}
				});
			}
		});
		$list.appendTo(this.output);
	}
});
Macro.add("addItem", {
	handler: function () {
		var obj = this.args[0];
		var num = this.args[1] || 1;
		if (typeof obj !== "object" || !obj.hasOwnProperty("id")) {
			return this.error('Incorrect item id');
		}
		if ( !Number.isInteger(num) || num < 1) {
			return this.error('Incorrect item count');
		}
		if ( this.args.length > 2) {
			return this.error('Too many arguments');
		}
		var idx = State.variables.inventory.findIndex(function (item) {
			return item.id === obj.id;
		});
		new Wikifier(this.output, obj.name + ' ×' + num);
		if (idx === -1) {
			obj.count = num;
			State.variables.inventory.push(obj);
		}
		else {
			State.variables.inventory[idx].count += num;
		}
	}
});

1 Answer

+1 vote
by (64.2k points)
selected by
 
Best answer

I believe something like the following is what you want:

Macro.add('showInv', {
	handler : function () {
		var $list   = $(document.createDocumentFragment());
		var sv      = State.variables;
		var catList = sv.categories.filter(function (cat) {
			return sv.inventory.some(function (item) {
				return item.category === cat;
			});
		});

		catList.forEach(function (cat, i) {
			if (i > 0) {
				$list.wiki('<br>');
			}

			$list.wiki('<b>' + cat + '</b><br>');

			sv.inventory.forEach(function (item) {
				if (item.category === cat) {
					$list.wiki(item.name + ' ×' + item.count + ' ');
				}
			});
		});

		$list.appendTo(this.output);
	}
});

 

Though, unless there's a pressing need for $categories to be a story variable in the first place, I'd suggest putting it on setup so you're not bloating the story history for no good reason.  For example:

<<set setup.categories = ["Cats", "Fruits", "Pies", "Horrors"]>>

And the updated macro:

Macro.add('showInv', {
	handler : function () {
		var $list   = $(document.createDocumentFragment());
		var sv      = State.variables;
		var catList = setup.categories.filter(function (cat) {
			return sv.inventory.some(function (item) {
				return item.category === cat;
			});
		});

		catList.forEach(function (cat, i) {
			if (i > 0) {
				$list.wiki('<br>');
			}

			$list.wiki('<b>' + cat + '</b><br>');

			sv.inventory.forEach(function (item) {
				if (item.category === cat) {
					$list.wiki(item.name + ' ×' + item.count + ' ');
				}
			});
		});

		$list.appendTo(this.output);
	}
});

 

by (220 points)
Thank you! It is indeed what I wanted.

Your macro visually gives the same I had, and the parts that I didn't like in my code are fixed. After reading up on them, using filter and some methods indeed make more sense.

I didn't know about setup variables. That's what I get for reading documentation not thoroughly enough.
by (64.2k points)
Please select this answer if it met with your satisfaction.
Welcome to Twine Q&A, where you can ask questions and receive answers from other members of the community.

You can also find hints and information on Twine on the official wiki and the old forums archive.

See a spam question? Flag it instead of downvoting. A question flagged enough times will automatically be hidden while moderators review it.
...