Skip to content

Refactor Installed Items

jalensailin requested to merge jalenml/refactor-installed-items into master

Intent

It is no secret that our Installed Item code has a lot of issues. This is not a sleight towards the original writer of this code. The system itself allows for a lot of flexibility, but there is currently also a lot of unnecessary complexity.

Here are some examples of where things go wrong:

How do we keep track of what is installed and where?

Currently, we keep track of this in a few places. Container items have the field item.system.installedItems.list which is a list of UUIDs and is meant to be the "source of truth" for what is installed in that item.

Installable items have item.system.installedIn (UUID string) and item.system.isInstalled (Boolean). These are updated concurrently but separately with item.system.installedItems.list. I.e., when a new item is installed into something else, the target items installedItem.list gets updated and the installed item's installedIn/isInstalled get updated at the same time. This causes issues if the installable fields are not updated at the same time. What if something gets removed from installedItems.list but isInstalled and installedIn do not also get updated? Now we might have items missing from the character sheet even though they exist on the actor or othjer unpredictable behavior.

If the installed item is a Program being installed into a Cyberdeck, then we also have to update the cyberdeck.system.programs.installed and cyberdeck.system.programs.rezzed lists. Those currently carry copies of the program data for programs that are installed and rezzed respectively.

If the installed item is an upgrade, then we also have to update item.system.upgrades which contains copies of installed upgrade items data.

See how complicated this is?

UUIDs:

Currently, item.system.installedItems.list is an Array of UUIDs. Here is an example of a UUID from an item on an unlinked token: Scene.6EnbsWygtAc9i7nB.Token.VuC9qemndVkyYm9S.Actor.HnGAQC8i4jIMJgiZ.Item.Z1xCDPmwt0RTaqfZ. If we duplicate this token, all of the actor's items keep their IDs, but get new UUIDs. Unfortunately, all of the entries in both actor.system.installedItems.list and item.system.installedItems.list are the UUIDs of the old items. These IDs don't correspond to any of the items on the newly created actor and now items may seem like they have disappeared from the character sheet.

Situations where UUIDs can point to the wrong document or become mismatched:

  • Token with installed items is copied
  • Items with installed items are dragged/dropped between players.
  • Items are duplicated in the world
  • Migration migrates some uuids/fields (like installedItems.list) but doesn't get all necessary fields (like 'installedIn' or programs.rezzed)
  • World actors with installed items get duplicated.
  • World items with installed items get duplicated.

UUIDs are useful when you don't know the source of the ID. But for installed items on actors, we always know where they live. They live on the actor. Installed items should never reference items outside of their scope (i.e. the world, or the actor they are on).

How we fix it:

There are a few techniques I've employed to solve this problem: First, instead of UUIDs, we migrate to regular IDs. Regular IDs on actor-owned items are not regenerated when the actor is duplicated. And, we know if an item is not embedded in an actor (aka, the item is a world item), any items installed in it must also be world items. So we can always use just regular IDs to find items that are installed.

Second, make installedItems.list the actual "source of truth" and get rid of superfluous fields: isInstalled and installedIn are now getters. Their values now depend on whether the item is actually installed somewhere. We also don't need system.programs.rezzed, system.programs.installed, or system.upgrades. Instead we will just filter the list of installed items for those properties.

The Main Takeaways (or "How does this impact me as a developer?")

  • document.system.installedItems.list is now actually and always the source of truth for what is installed. It stores an array of owned item IDs (not uuids).
    • Changing these to IDs has solved many issues related to trading/duplication of items with installed items.
    • Where previously we required a lot of complex code to make sure there was parity between all of the UUID fields and the actual IDs of the actors/items that they were supposed to refer to, now actions like trading items between characters or copy/pasting tokens just works.
    • To get a list of installed items, you can call container.getInstalled(itemType) where itemType is an optional String parameter which will filter by item type.
    • installable.system.isInstalled and installable.system.installedIn are now both getters, and the latter returns an Array of ID Strings (because some items can be installed in multiple items at once). Turning these into getters means they will stay up-to-date when insalledItems.list is updated.
  • How to Uninstall/Install items:
    • To install items into a container
      • Item: container.installItems([Array of Items to Install]) for.
      • Actor: actor.installItems([Array of Items to Install])
    • To uninstall items from a container,
      • Item: container.uninstallItems([Array Of Items to Uninstall])
      • Actor: actor.uninstallItems([Array Of Items to Uninstall])
    • Note, the container mixin is now a mixin of both Container Items and Character/Mook Actors, which is why the functions can apply to both. Nifty!
    • When un/installing things programmatically, you should always use one of these two functions.
    • If an installable needs special handling when it is installed or uninstalled, make a separate function and call it from installItems() or uninstallItems(). This way there is a single function each for installing and uninstalling and all items go through the same process. You don't have to make sure the special logic is applied in every place an item might be (un)installed. It is all centralized to these two functions.
  • Container items with installed items can now be exported to JSON and Compendia.
    • This works by turning the installed item tree into a nested array of installed item data and storing this tree in a flag.
    • When the parent item is reimported, the child items are dynamically created where the item was dragged to (world or actor).
    • If imported to the world, a nested folder structure is created to help organize / keep track of these newly made items.
  • Ammo is now an installable item.
    • Any item that has the Loadable mixin should also have the Installable mixin.
    • Weapons should by default have Ammo as an installable type (not yet implemented in the compendia, waiting for migration refactor)
    • Theoretically this would allow multiple ammo installed in the same item, but currently the system prevents users from doing this. I realized that adding that as a feature in this MR would be too much bloat.
  • This is meant for a hotfix
  • This is meant for the next release (see milestone)
  • This needs more reviewers than normal; there may be controversy or high complexity
  • This intentionally introduces regressions that will be addressed later
  • There is/will be documentation changes on the wiki
  • Please do not send commits here without coordinating closely with the owner
  • This is a Build System change

Related Issues

Steps to Test

  • Cyberware:
    1. Install cyberware on a character actor using the install glyph.
    2. Uninstall cyberware on a character actor using the install glyph.
    3. Install multiple nested layers of cyberware (e.g. skill chip in a chipware socket in a neural link)
    4. Install cyberware via drag/drop on a mook sheet,
    5. Uninstall cyberware on a mook actor via hovering over the item entry and hitting the 'delete' key.
    6. Duplicate an actor, ensure all cyberware remains and is installed in the correct place.
  • Programs:
    1. Install programs into a cyberdeck in the world (sidebar).
    2. Install programs into a cyberdeck on an actor sheet via manageInstalledDialog (from gear tab)
    3. Uninstall programs on an actor sheet via manageInstalledDialog (from gear tab)
    4. Roll ATK and DEF and DMG from programs.
    5. Uninstall a program on a sheet by hitting the red uninstall button on the net tab.
    6. Rez a non-black-ICE program.
      1. Reduce the rez on installed programs.
      2. Reset the rez on installed programs.
      3. Derez an installed program.
    7. Rez a black ice program from an actor with a token (should work for linked and unlinked tokens).
      1. Check that it creates a black ice token. If one exists in the world already it should do that one, if not, it will create one (i think this ithe expected behavior).
      2. Roll damage from that token.
      3. Check that REZ is updated from token to sheet and vice-versa.
  • Ammo:
    1. Configure weapon so that it can take ammo as an installed item.
    2. Load ammo into a weapon. Ensure ammo is deducted from the ammo item correctly.
    3. Unload ammo. Ensure ammo item gets its amount back.
  • Upgrades:
    1. Install upgrades into weapons.
    2. Ensure that they modify the correct things and display correctly.
  • Export/Duplicate:
    1. Create a world item in the sidebar.
    2. Install multiply nested items into the world item.
    3. Duplicate the world item. Check that it works.
    4. Export the item and reimport it. Check that it imports correctly with a nested folder structure.
    5. Export the item to compendia, reimport it. Check that it imports correctly with a nested folder structure.
    6. Drag and drop installed items between actors and make sure all installed items come over.
  • General:
    1. Ensure nested installed item drop down in the gear tab displays correctly.
    2. Ensure cyberware tab displays nested installed cyberware correctly.

Future work

This still needs migration written and compendia updated but I want to wait until after our large refactor of migration to do that.

Edited by Ryan Walder

Merge request reports