Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System-provided initial compendium index fields #8776

Closed
stwlam opened this issue Jan 22, 2023 · 19 comments
Closed

System-provided initial compendium index fields #8776

stwlam opened this issue Jan 22, 2023 · 19 comments
Labels
compendium Issues related to Compendium packs data-models Issues related to data models and schema changes packages Issues related to add-on packages, systems, modules, and worlds wontfix Issues which will not be worked on

Comments

@stwlam
Copy link

stwlam commented Jan 22, 2023

User Experience

On initial load of the Foundry client, compendium index fields are a hard-coded set, and there is no API for altering them. If a system author wishes to have other (typically system data) fields included in actor or item indexes, they must edit the global CONFIG object from the client and then request new indexes from the server. Being able to provide compendium index fields from a system.json would allow for skipping a second--moderately heavy--network request.

@aaclayton
Copy link
Contributor

Yeah, I think what I would like to do here is support pack-specific index fields to be defined as part of the compendium pack declaration in the module manifest file, for example:

  "packs": [
    {
      "name": "heroes",
      "label": "Starter Heroes",
      "type": "Actor",
      "indexFields": ["details.level", "details.biography.value"]
    },

This would only allow the package which initially provides a compendium pack to instruct the server to vend additional index fields, but I think that solves the primary use case without an excess of complexity.

Would this solution satisfy your needs?

@aaclayton aaclayton added compendium Issues related to Compendium packs packages Issues related to add-on packages, systems, modules, and worlds data-models Issues related to data models and schema changes labels Jan 23, 2023
@aaclayton aaclayton added this to the Version 11 - Prototype 2 milestone Jan 23, 2023
@stwlam
Copy link
Author

stwlam commented Jan 24, 2023

Personally I envisioned it being available per subtype for an entire system (or for a V11 module that introduces new subtypes). As a system author I want the same index fields for whatever a content module introduces so that the new content can be integrated with features that utilize the index fields. The content module's author presumably wouldn't care what index fields are available for their own compendiums--though maybe there's a use case I'm not seeing for lack of first-hand experience making such modules.

@cswendrowski
Copy link
Contributor

From the Lancer dev in Discord:

the proposed solution of adding the index fields to the pack itself wouldn't work for Lancer, since the packs are generated in the world, not shipped with the system.

@aaclayton
Copy link
Contributor

@cswendrowski can we get some clarification on the Lancer use case? I don't think I understand from the above quote. If the above proposed solution would not work for Lancer, is there an alternative solution that would? Or is Lancer just an un-solveable use case and they will have to manually request indices in-JS?

@cswendrowski
Copy link
Contributor

@cswendrowski can we get some clarification on the Lancer use case? I don't think I understand from the above quote. If the above proposed solution would not work for Lancer, is there an alternative solution that would? Or is Lancer just an un-solveable use case and they will have to manually request indices in-JS?

Lancer generates compendiums on-the-fly from an external resource - Lancer has an official digital toolkit: https://compcon.app/#/

So long as it's possible to define indexFields when creating a pack they should be fine

@Eranziel
Copy link

For more context, part of the digital toolkit ecosystem for Lancer is content packs (renamed zip files). Both Comp/Con and the Foundry system can import those packs; for the system the items in the packs are used to generate or add to the compendiums. We decided it was the best UX to provide those compendiums on a world-by-world basis for a few reasons - sandboxing homebrew, avoiding users having to re-import everything each time the system updates, etc...

cswendrowski is right, as long as we can define indexFields when creating a pack our use case is covered. Ideally it would be nice to be able to update those on existing packs as well, to make migration to v11 simpler. (i.e. so we don't have to delete all the packs in a world in order to re-create them with the correct indexFields)

@aaclayton
Copy link
Contributor

One alternative that is being considered instead of defining indexFields on a per-pack basis is instead allowing the system to define indexFields on a per-subtype basis as suggested by @stwlam. How would that approach work in the Lancer use case?

@Eranziel
Copy link

That would work perfectly fine for Lancer. Probably less prone to misuse, also.

@kakaroto
Copy link

kakaroto commented Feb 1, 2023

User Experience

On initial load of the Foundry client, compendium index fields are a hard-coded set, and there is no API for altering them. If a system author wishes to have other (typically system data) fields included in actor or item indexes, they must edit the global CONFIG object from the client and then request new indexes from the server. Being able to provide compendium index fields from a system.json would allow for skipping a second--moderately heavy--network request.

Can I please chime in and say that this feature request should not be happening please?

People are already suffering from long load times due to the massive amount of data being sent on load (see #5942) and the indexing of compendiums is a way to send just the minimal data without saturating someone's network. While this would be great in theory for a use case of someone on localhost, or with a fiber optic internet, the reality is that many of Foundry's users are on low bandwidth, rural internet, and many worlds are rather large and made even larger by hundreds of MBs of compendium data.

Something that many of us have been praying for is for Foundry to not even load all the content from the world db, as sending a multi-hundred MB database to the client on load can have devastating effects on load time of course, and it would be more efficient to only send an index and load documents as they are requested. This will of course never happen (or at least, not for years) since on the ready hook, all modules are already expecting the actors/journals/etc.. to be fully populated, and there is no async hook support, and modules don't use await right now to access an actor's data, etc.. This was discussed back in the 0.3.x days of Foundry and it was already out of the question because of how breaking it would be for the design and API, so I don't expect that to ever happen.

@stwlam's suggestion is (in his own words) "for skipping a second--moderately heavy--network request", but it would just mean that it would make the initial moderately heavy network request be even more moderately heavy! Sending a bigger index at the game load would not avoid the network request, it would just bundle it up in one initial blocking request.

Basically, adding the ability to specify index fields for compendiums would be a step in the opposite direction of where we should ideally be.
The current status is :

  • Foundry serves the world db + small index
  • User waits for that data to be downloaded and Foundry to load
  • System/Module has access to the index and decides if it needs any additional index fields
  • System/Module calls an async API to get the information it needs

What this issue plans on doing is

  • System/Module specifies which index fields to provide during load
  • Foundry serves the world db + index + additional fields
  • User waits for that data to be downloaded and Foundry to load... potentially doubling their loading time, increasing their frustration and making them either ask for support for "why is this not loading" or leave the platform entirely for how slow it is to load (I have hundreds/thousands of example use cases of this exact thing happening with the current design already)
  • System/Module saves itself an await call because it gets all its data right from the start

The pros/cons here are (in my personal opinion) quite obvious. and we all KNOW that there will be someone who will make the mistake of specifying every possible field they can. Heck, even @aaclayton's example actually puts the actor's biography in the list as an example, which would be catastrophic as it would be the largest amount of data in an actor's db.

Imagine having to download 200MB of data on a rural internet connection because of a 'misconfiguration' of the index fields in a system.
It would be much better for the user experience to actually see Foundry load instead of watching the grey screen of death for 10 minutes, and then if they try to access some specific compendium, they see a "please wait..." message while it's indexing the additional data the system might be needing (which in most cases, it likely would receive that data before the user even gets to click on any button that would require it).
That's exactly what we do for example in the Compendium Library, and no, you don't need to edit the CONFIG, you can specify your fields to getIndex directly, see:
https://github.com/ForgeVTT/forge-compendium-browser/blob/main/src/hierarchy.js#L257

You may disagree with my assessment, but please do not make game loading even slower for users for the sake of "having everything ready right from the start" and to avoid doing a simple async operation after the game loads.

@aaclayton
Copy link
Contributor

aaclayton commented Feb 2, 2023

Hey @kakaroto thanks for the thoughtfully worded reply. I agree with you that keeping the initial world load as small as possible has big advantages.

The main proponent of this proposal and the primary use case we are trying to better support is PF2E (represented by @stwlam) who are currently using NEDB query language to retrive a subset of documents from compendium packs. With the NEDB query language going away in V11+ I've been working with them to scope alternative strategies. Most of these alternative strategies involve more use of compendium indices.

After discussion with @stwlam, however, we both agree about your points regarding performance and I believe that they have agreed to pursue an alternative approach which would request expanded indices lazily at the latest possible moment in order to fulfill certain requests.

I will leave it to @stwlam to confirm that this path forward is one that the PF2E system can commit to, and if so I am comfortable to close this issue as a "wontfix".

@stwlam
Copy link
Author

stwlam commented Feb 2, 2023

Yeah, I'm fine with closing it.

@Eranziel
Copy link

Eranziel commented Feb 2, 2023

For what it's worth, while Lancer would like custom indices, I agree that it's more important for UX that the initial load is as fast as possible.

@aaclayton
Copy link
Contributor

Thanks for the consensus, folks.

@aaclayton aaclayton closed this as not planned Won't fix, can't repro, duplicate, stale Feb 2, 2023
@aaclayton aaclayton removed this from the Version 11 - Prototype 2 milestone Feb 2, 2023
@aaclayton aaclayton added the wontfix Issues which will not be worked on label Feb 2, 2023
@kakaroto
Copy link

kakaroto commented Feb 2, 2023

Thank you @aaclayton and @stwlam and others for reading and accepting my input on the matter! 🙇‍♂️

I hadn't realized this was for the pf2e query support. If you need to bounce ideas off me (or just acting as a debugging duck) to help find some kind of optimal alternate solution, I'd be more than happy to help! Just let me know
Thanks again!

@BoltsJ
Copy link

BoltsJ commented Sep 8, 2023

I'm not sure that kakaroto's concerns are well founded, at least for the use case of Lancer. I created a new world, imported the core data as well as 9 additional content packs, refreshed the index then crunched an approximate for the amount of additional data this would add to the indices. This calculation assumes ascii encoded JSON with intact spaces and doesn't account for any compression, but it should be in the ballpark. The result I got was ~68KB or four orders of magnitude fewer than the 200MB unsourced estimate from kakaroto, which I don't think is at risk of doubling loading times.

That said, any performance impact of this could be mitigated by limiting it to a single additional index field, rather than an array, which would be sufficient for Lancer's use case. Failing that, adding a new optional top-level field to Documents for content importers to use for identifying content would also be sufficient for lancer.

@kakaroto
Copy link

kakaroto commented Sep 8, 2023

I'm not sure that kakaroto's concerns are well founded, at least for the use case of Lancer. I created a new world, imported the core data as well as 9 additional content packs, refreshed the index then crunched an approximate for the amount of additional data this would add to the indices. This calculation assumes ascii encoded JSON with intact spaces and doesn't account for any compression, but it should be in the ballpark. The result I got was ~68KB or four orders of magnitude fewer than the 200MB unsourced estimate from kakaroto, which I don't think is at risk of doubling loading times.

That's your very specific use case. Trust me that there are thousands of users who have hundreds of MBs of compendium data. pf2e alone is 116MB and most pf2e users have other pf2e specific modules (bestiaries and whatever else) with even more hundreds of MBs of compendiums.
You're giving a single example for your specific use case of Lancer and your personal compendium packs. Unfortunately, from my experience with Forge users, there's an incredibly large number of people who will also download the entirety of the d&d universe into compendiums "just in case".

I just recently had to help someone who was having trouble launching Foundry and it turned out that they had 1080 modules installed and 327 compendium packs enabled in their world (Foundry was taking 45 seconds just to list all the modules and parse their manifest and load their compendiums, before it starts listening on port 30000, which was enough for the initial request to timeout...)
I can guarantee you that this user would not have ~68KB of extra data when loading into their world.
And while, yes, 1080 installed modules is an extreme use case, the rest of what I said with the 200MB+ of compendiums is extremely common among Foundry users.
Also, you're assuming that there isn't a misconfiguration that would cause more data than strictly required to be sent (i.e: like I mentioned before, about adding by mistake the actor bio to be included in the index)

That said, any performance impact of this could be mitigated by limiting it to a single additional index field, rather than an array, which would be sufficient for Lancer's use case. Failing that, adding a new optional top-level field to Documents for content importers to use for identifying content would also be sufficient for lancer.

add one additional index field and the next issue that gets created is someone saying they need to specify 2 fields.
I don't see what the issue is with doing an await pack.getIndex(fields) after the world is loaded.

@BoltsJ
Copy link

BoltsJ commented Sep 9, 2023

It's like 40 bytes per record once again assuming uncompressed json for lancer.

I don't see what the issue is with doing an await pack.getIndex(fields) after the world is loaded.

It forces every operation that involves the index to be async.

@anathemamask
Copy link
Collaborator

I'm not even sure this still applies in a post-LevelDB world, but i will note that this issue has been tagged #wontfix and closed. Is there a particular reason you dredged it out of the archive to start arguing on it, 7 months later?

@aaclayton
Copy link
Contributor

In @BoltsJ defense:

  1. This does definetly still apply in the case of LevelDB
  2. The desire to have expanded indices for system use cases is an entirely reasponable one

For most systems with a "normal" amount of compendium data and "normal" use cases for indexing, this would be an almost trivial amount of additional overhead to add to the initial world load and would unlock the ability to use indices in powerful ways inside synchronous workflows like Actor or Item data preperation.

We did close this issue as wontfix, but I think the arguments here are compelling to support some way of expanding the initial indices in the future. The main concern is that as soon as people can do it, they will do things like request absolutely everything under the sun, completely defeating the purpose of compendium packs not being automatically loaded.

Some systems, like Pathfinder 2e as mentioned, are very large. PF2E currently clocks in by my check with around 60MB of compendium data (not the 116MB as mentioned above). It's possible this feature could be somehow limited to a reduced scope, or a maximum number of fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compendium Issues related to Compendium packs data-models Issues related to data models and schema changes packages Issues related to add-on packages, systems, modules, and worlds wontfix Issues which will not be worked on
Projects
None yet
Development

No branches or pull requests

7 participants