foundryvtt / foundryvtt

Public issue tracking and documentation for Foundry Virtual Tabletop - software connecting RPG gamers in a shared multiplayer environment with an intuitive interface and powerful API.
https://foundryvtt.com/
239 stars 9 forks source link

Referenced Owned Items #4451

Open aaclayton opened 3 years ago

aaclayton commented 3 years ago

Originally in GitLab by @schultzcole

Feature Summary

I propose a method whereby items can be added to an actor, but rather than outright copying all of the item data to the actor, the actor stores a reference to the id of the item. Then, similarly to un-linked tokens, it could store a "delta" of properties which get changed relative to the "prototype" item.

This would not be mutually-exclusive with the existing method of adding owned items to actors, and likely would not be a breaking change for existing actors.

I understand that this would be quite a significant overhaul (and probably has been suggested before), but I think it has some substantial benefits:

  1. Changes to the prototype can propagate to owned items.
  2. Significant space savings due to not having duplicate data for owned items.

Terminology

These are my initial suggestions for terminology to use to distinguish the two types of owned items:

More user friendly terms might become necessary.

User Experience

User choice Referenced vs Instanced owned items

  1. When a user drags an item onto an actor sheet, display a dialog pop up asking the user to select between adding the item as a referenced or instanced owned item (with a short description of the difference).
  2. When a user drags an item onto an actor sheet, add it as an instanced owned item, unless the user is holding a modifier key (i.e. Alt), in which case add the item as a referenced owned item.

I think 2 would be least confusing for both new and existing users, however it might result in fewer people being aware of the difference (or even that there is a difference at all). It might also be worth using 2 but inverted, where the default is adding as a referenced owned item. However, this might conflict with existing user's expectations.

Converting from a referenced to an instanced owned item

There may be situations where a user would elect to explicitly convert a referenced owned item to an instanced owned item. I propose that there should be a button in the sheet header of a referenced owned item which would convert that item to an instanced owned item by copying the data from the prototype item. I think such a function should be gated behind some kind of confirmation dialog, as this would likely be irreversible.

Deleting the prototype of a referenced owned item

If the user deletes an item from the items directory that is the prototype of a referenced owned item, those referenced owned items should not cease functioning correctly without at least a warning. When a user does this, I would suggest that a dialog should appear presenting the user with 4 options:

  1. Cancel deleting the prototype item.
  2. Convert all referenced owned items to instanced owned items, then delete the prototype item.
  3. Delete the prototype item and all owned items that reference it.
  4. Delete only the prototype item (and accept the consequences).

Nice to have extra features

Priority/Importance

Low priority. This would be very nice to have, IMO, but the existing system does work.

aaclayton commented 3 years ago

Thanks for taking the time to write this up. I've spent a fair bit of time thinking about this (almost exact) feature, but never got around to stubbing out an issue on gitlab about it.

I agree with mostly everything you mention regarding the advantages of this sort of system. I'm very focused on getting Owned Items (instanced) to function better as part of the 0.8.0 infrastructure overhaul, so this is one that will have to wait for a future cycle, but I appreciate having the thought written down.

aaclayton commented 1 year ago

Comment from @In3luki

I was thinking the server could resolve the UUID and merge the compendium item data before sending the actor to the client.

This would be extremely useful for systems where most items come from a bundled compendium pack and these items are rarely modified by the user. It would make it easier to keep embedded items up to date with system changes without the need to migrate or replace them in every actor. This would also benefit actor compendium packs which currently need to have their embedded items updated regulary.

In3luki commented 1 year ago

Unfinished sample implementation in the PF2e system: https://github.com/foundryvtt/pf2e/pull/6232 This implementation would add considerable overhead to the first load of a world which could be mitigiated if the server could provide the fully resolved items instead.

In3luki commented 1 year ago

I've tested my system implementation and can report that this would definitely be a significant improvement in regards to compendium size and data maintenance. While it would be possible to do this on the system side I believe it would be far better to do it on the server side. I've tested fetching ~250 items from the server with CompendiumCollection#getDocuments using a NeDB query. While the result was better than I expected, it still can take more than 10 seconds on slower connections (tested with the Chrome Slow 3G network throttle). Also there is no good place for such an operation in the game setup because ideally it would be an asynchronous operation that stops the rest of the setup workflow until it is resolved.

For my compendium size tests I converted all embedded spells with a valid flags.core.sourceId field by using diffObject to only store the differences to their compendium counterpart. Therefore the result includes partial system data as well as different names or in some cases full descriptions. Whether partial system data should be allowed at all is debatable.

The results of the conversion of 3 PF2e Bestiary packs are as follows:

Pathfinder Bestiary 1:
468 Actors
1977 Embedded Spells
287 unique Spells
Original size: 9,406 KB
After conversion: 5,935 KB
Reduction: 3,471 KB (36.90%)

Pathfinder Bestiary 2:
373 Actors
1422 Embedded Spells
295 unique Spells
Original size: 6,651 KB
After conversion: 4,441 KB
Reduction: 2,210 KB (33.23%)

Pathfinder Bestiary 3:
365 Actors
1749 Embedded Spells
338 unique Spells
Original size: 7,766 KB
After conversion: 4,790 KB
Reduction: 2,976 KB (38.32%)

An example spell reference without system data:

{
    _id: "l9yhRiDjFD7OKAxQ"
    sort: 400000,
    sourceId: "Compendium.pf2e.spells-srd.IxyD7YdRbSSucxZp",
}
aaclayton commented 1 year ago

Thanks for the detail and experimentation @In3luki. This is something we will likely revisit once some of our current work on utilizing the new V11 LevelDB backend progresses further. Tagging @Fyorl for visibility.