mbkarle / mind-the-fog

Roguelike JavaScript game--mind the fog!
2 stars 1 forks source link

Inventory Class #128

Closed akarle closed 6 years ago

akarle commented 6 years ago

I'm not gonna go into too much detail in this PR because I've done a lot of documenting along the way (see commits, #119 for planning notes on designing the Inv)

Introducing the Inventory Class

Def: Inventory -- any collection of items

Motivation

I found a lot of code duplication, particularly when it came to how inventories are displayed, which is very standard with the only exception being the callbacks of the buttons, the button text, etc.

There was also a lot of code that was splicing elements out of lists, and such, which is less readable and less reliable than having specific objects dedicated to collections of items, transferring them, visualizing them, etc.

Along the Way...

I've converted everything to an Inventory. Even the vendors. Which brings us too:

The Vendor Module

If the TextModule was the last big thing in my code refactor (#123 ), the Vendor Module was the next big step towards #120 (module separation).

All vendors sell from Inventories, so it was easily standardized by creating a system of transferring between inventories for money, etc.

Turned out that vendors aren't as simple as transferring items though (Merchants are). NPC's use the vendor module, and as such, the Vendor-Module uses the Inventory class to visualize an Inventory, but the main callback function of spending money at a vendor is completely done outside the vendor module (all NPC's have these in tempdb/npcs.js.

The only parts of module separation (#120) are the combat and spell tree modules. But for now, that is future work.

TempDB #126

This branch fixes #126 in that it plucked all our json objects out of the code itself and put them in a separate directory.

Why? See #126.

Future

Use the Inventories! It really makes transferring items reliable and effective and lets us focus on content!

Broken...

  1. Comparison Hover in inventory (I'll get to it? Not high on my list)
  2. 127 All items have 0 value! use randGenInvValues() in console in the meantime as a workaround. Really gonna wait until @mbkarle merges his #26 branch to do this.

akarle commented 6 years ago

Fixes #119