#V9-Dev API Requests

1 messages · Page 1 of 1 (latest)

dry shadow
#

Hello there <@&596076164104323104>s! On behalf of the <@&182849491915767808> I wanted to take the time to reach out and remind you all that we're in the API Development phase for Version 9 right now and after V9 Development 2 releases we expect to close the API to further requests for this version. If you have feedback, feature gaps, or places where you're working around the API in kind of a hacky way and would prefer to use a more native approach, now's the time to get your feedback into us.

This thread will serve as a sort of catch pen to help us classify and scope potential API additions or changes that we can make to make your lives easier.

Update: The goal of this thread is to resolve issues with the API in the areas we're presently working as detailed in the milestone here: https://gitlab.com/foundrynet/foundryvtt/-/milestones/88. We will take requests for API features we can implement beyond that milestone, especially if they are deemed to be low effort on our part, but as with all things it must be balanced in terms of priority.

Please make use of it!

vagrant carbon
#

API enhancement request: It would be great if the _pre<lifecycle> (i.e. _preCreate etc.) methods on the Document classes could also return a boolean value to prevent the creation / update / deletion, similar to how this works with the pre... hook events.

Context: In my system, I am currently overriding the methods to implement some functionality. Overriding instead of using the hooks seems to be the appropriate thing, given that I already override the class (Actor and Item). But in order to prevent the creation of some items, I need to use the hook instead. My implementation would be a lot more consistent if there was a single place to do this.

arctic spade
#

All of the actor sheet drop functions are marked as private. There's no officially endorsed way for handling drops. That is, for rejecting them, for altering them, or for doing something completely different (effectively rejecting them). It makes the dragDrop part of defaultOptions somewhat of limited use.

whole forge
#

The current return type is Promise<void>, so that might be breaking in the worst possible way.

vagrant carbon
#

yeah, at the moment, the return value is just ignored. So if some systems were to return false now, introducing this would break those systems' functionality.

whole forge
#

in 0.8 (and presumably v9), all document creation gets funneled through the static createDocuments method unless they're "snuck" in via an update of the parent document

#

the natural procedure for preventing creation ends up being to override that method and pruning elements from the array

arctic spade
#

Unfortunately that loses context of the creation (most things lack any info on context, tbh).

vagrant carbon
#

(if a module were to create the documents for example)

whole forge
#

a module can always misbehave :\

vagrant carbon
#

don't you just do Actor.create in the console sometimes? Instead of CONFIG.Actor.documentClass.create?

arctic spade
#

Actor.implementation.create() is "simpler"

vagrant carbon
#

yeah, that works, too. or getDocumentClass('Actor').create

#

But I would assume that many people just do Actor.create

whole forge
#

there are maybe different standards of naughtiness for playing console cowboy and writing a module

arctic spade
#

Actual comment on the thread topic: Roll.implementation does not exist even though you can assign new default Roll class.

whole forge
#

I think subclassing that hasn't been as well-tested

#

it'd be good to for v9

arctic spade
#

Subclassing it works fine(-ish) in my testing.

vagrant carbon
#

anyways, I have made my suggestion and would be happy to see that addition. But if the cons (which is mostly that it might break things if people already return false now, if I understand correctly) outweigh the pros in the core team's eyes, that's also fine for me. Don't really have anything else to add

whole forge
#

I'd be interested in hearing your solution for the fact that most overrides wouldn't return anything and would truthy-test to false

#

would this be a strict boolean test?

vagrant carbon
#

yeah, exactly the same way it works for the hooks

arctic spade
#

Probably returnValue === true like the other similar cases.

vagrant carbon
#

it would check === false

arctic spade
#

Or that.

whole forge
#

ok sounds good 🙂

vagrant carbon
# arctic spade All of the actor sheet drop functions are marked as private. There's no official...

I agree in general that at least Application_onDrop should be promoted to the public API. Lot's of modules and systems override it. The more specific _onDrop... methods in the actor sheet are also often overridden, e.g. in dnd5e: https://gitlab.com/foundrynet/dnd5e/-/blob/master/module/actor/sheets/base.js#L739. So I think it would also make sense to promote those to the public API

GitLab
module/actor/sheets/base.js · master · Foundry Network / Foundry VT...

An implementation of the Dungeons & Dragons 5th Edition game system for Foundry Virtual Tabletop (http://foundryvtt.com). This work is permitted under the Open...

#

I have a feeling that this thread might get very noisy with discussions about specific API suggestions, is this the way it's intended to work? Or should we create new threads for each suggestion somehow?

dry shadow
#

I perhaps naively didn't expect suggestions to sponsor hot debate.

whole forge
#

I think we all still pals

#

❤️ ghost ❤️ mana

vagrant carbon
#

well, maybe this is only an outlier and it works better with the next suggestions.. 😄

whole forge
#

❤️ nath

dry shadow
#

perhaps post-debate, one of the contributors can add a simple summary post highlighting whether or not the request should still go through

#

For the record: nothing here is guaranteed to be added. We will make every attempt but we are still beholden to our scope and prioritization for v9.

vagrant carbon
#

yeah, that's a given. These are just suggestions. In the end it's your choice what you do with them 🙂

arctic spade
dry shadow
#

They're on a different track for absorption- so if they don't currently have visible gitlab issues, feel free to mention them here. The scope of this channel is specifically things that should make it into v9 before the API closes; things which will unblock you or make something you're currently doing with a workaround easier.

vagrant carbon
#

So, to summarise my previous suggestion regarding enabling the _pre... methods to be able to return false to prevent the action:

  • It would be helpful to some
  • It might break existing systems if they already return false now (by accident as it just doesn't do anything right now)
  • An alternative solution could be to simply override the createDocuments etc. methods (though that would not work, if the createDocuments method of the base class is called instead of the configured one)

In total, there's both pros and cons.

arctic spade
#

Didn't spot an issue for it, so TextEditor.enrichHTML should have option to not roll inline rolls and just display the formula as if they had /r. Useful for informative display of the content before the results are wanted or needed (e.g. informative pre-roll display, tooltips, descriptions (at edit locations), etc.).
Currently doing that requires re-implementing the entire function.

distant rover
#

Not sure if this is the correct place to put it other than #feature-suggestions but I'll say it anyways:

Title: Include list of online player names for status API
Description: When using services like The Forge, it helps to know which players are online before actually launching the game first. Currently, the status API (https://dnd5e.doomnaught.com/api/status) doesn't include such a field; it only shows how many players are on, but not who.

unreal kettle
#

Alongside this, it would be nice if things like controlling tokens or adding/removing combatants from the HUD, triggered a bulk update instead of many individual ones

whole schooner
arctic spade
#

diffObject needs an upgrade or alternative that handles arrays correctly. It currently works only if you directly diff an array, but if an object you're diffing has an array inside of it, it'll always claim the array has changed no matter what.
This includes unlinked token updates where you have to supply item array always to not get it smashed, so they always show up as having changed because of this.
As it is, it requires some custom diffing.
Though as it is, it may be acceptable anyway, though the function should mention how it treats inner arrays differently.

autumn cypress
#

Borderline useless but still,
It be nice if the 'Duplicate' context menu a SidebarDirectory entry would flag the clone with a core.sourceId from the original.
(Atm, unlike an export to file or a dragDrop to a compendiumCollection, the new clone doesn't contain any reference to it's original.)

dry shadow
#

Non-api requests are outside the scope of this thread

whole forge
#

Maybe not viable as a v9 request, but it'd be swell were there an option on diffObject have deletion properties ({ -=deleteme: null }) generated

obsidian otter
whole forge
# obsidian otter I don't understand, can you elaborate?

given, e.g.,

before = { foo: true, bar: true };
after = { bar: true, baz: true };
diff = diffObject(before, after);

would result in diff equaling

{ "-=foo": null, baz: true }

such that mergeObject(before, diff) would return an object identical to after

arctic spade
#

Could concat handlebars helper be added?
Just take all arguments, pop off options, join the rest.

concat = (...args) => args.slice(0, args.length - 1).join('');
arctic spade
#

Would be nice there was a public debounced window reload function so that modules/systems that need to reload the page can do so with unified reload function instead all of them having their own (eventually leading to 100 different debounced reloads trying to fight).

arctic spade
#

Also, shouldn't active effect name getter return its label?

dry shadow
arctic spade
#

Adjusting point of vision currently is really hard. Same for adding more of them. Many systems I deal with have point of vision on all corners rather than center. I believe this is the case even with dnd5e.

whole forge
obsidian otter
whole forge
#

I think comments in foundry client code refer to them as "deletion keys"

obsidian otter
whole forge
#

diffObject and mergeObject are almost like unixy diff and patch tools except to delete properties you have to manually set deletion keys

#

it'd be super convenient for migrating data if one could just diff an old and new pair of objects and get a "patch" that would set those keys for the API user

obsidian otter
#

gotcha, I see

#

yeah, it's a valid request and doesn't sound too hard to facilitate

whole forge
#

with diffObject's current behavior, in a way there is information loss with the diff:

diffObject({ foo: true, bar: true}, { bar: true })

returns simply {}

arctic spade
arctic spade
whole schooner
#

A solution to allow GM-Only (i.e. Blind) chat messages to be created by any connected client would rock.

If I'm reading things right, ChatMessage.data.blind is only used in ChatMessage.isContentVisible's block about chatmessages that are of type CHAT_MESSAGE_TYPES.ROLL. Otherwise this boolean is ignored.

It strikes me as a small change to allow blind on ChatMessageData to affect the ChatMessage.visible logic as well to allow an API path to creating an invisible chat message from player clients.

fast basalt
#

what is it that isn't possible to do currently?

whole schooner
# fast basalt what is it that isn't possible to do currently?

As far as I can tell, it's currently impossible to use ChatMessage.create from a player user to create a chat message Document which only displays for GM users.

I.e. to Create a "Blind" arbitrary chat message similar to creating a blind roll chat message.

#

The workarounds I've seen involve setting a flag on the ChatMessage document, then registering a hook for renderChatMessage which then changes the DOM in some way (add a class and hide via css or literally remove the DOM element) for the messages with the matching flag.

whole schooner
# 
// ChatMessage
  get visible() {
    if ( this.data.whisper.length ) {
      if ( this.data.type === CONST.CHAT_MESSAGE_TYPES.ROLL ) return true;
-     return (this.data.user === game.user.id) || (this.data.whisper.indexOf(game.user.id) !== -1);
+     return (!this.data.blind && (this.data.user === game.user.id)) || (this.data.whisper.indexOf(game.user.id) !== -1);
    }
    return true;
  }

This is in essence what I am suggesting. The rest of the plumbing is already present.

snow lantern
#

Perhaps a small client API change... In Application.setPosition is it necessary to stop non-popout applications from utilizing this method?

The workaround I have used essentially is overriding setPosition somewhat like this:

   setPosition(pos)
   {
      this.options.popOut = true;
      super.setPosition(pos);
      this.options.popOut = false;
   }

This has been handy for making non-popout apps still using Draggable for instance. Just a nice to have potential change to remove the override like the method above.

obsidian otter
#

you can set popOut true and minimizable false if you want a moveable window that doesn't minimize

#

resizable false if you want it to be the same size

snow lantern
#

To separate the app from the automatic z-level placement / management. To separating it from the appId tracking and automatic closing w/ Esc key. Removing if ( !this.popOut ) return; / 1st line of that method doesn't break anything w/ core as far as I can see as use of setPosition is not automatic.

Draggable will work for any element, but does indeed use setPosition. The same is true for minimize / maximize & _onToggleMinimize. The same set to true then false in an overrided _onToggleMinimize as long as the HTML provided has some similar structure .window-header / .window-content. IE minimize and maximize are also gated, but don't need to be per se.

So I suppose you can say this is just a convenience for non-popOut applications that are draggable. But otherwise fully managed apps that react / respond like pop out apps.

In FQL and the new quest log I'm working on the Quest Tracker is not a popout and supports Draggable and resizing can be turned on and off dynamically by the user. It is fully managed including UI bounding / snapping and visibility (Esc can't close it)

I do have a lot of ideas re: Application v2. This would technically fall under this category. The above suggestion is just perceived low hanging fruit.

unreal kettle
snow lantern
#

That is one of the possible solution for just the Esc key handling, but still not fully managed. The z-order manipulation and other popOut handling remains. Just a thought that there seems to be some artificial barriers protecting wider usage of some of the API that isn't actually automatically called for non-popout apps, but can be leveraged by them if desired from a manual management angle. It's nice to reuse Draggable for instance rather than a custom home rolled solution, but a note must be left in the code to indicate why popOut must be set to true / false when invoking the parent setPostion.

unreal kettle
#

Yeah, I think we want to do the same thing as I also don't want the z-order to be handled, so I would love this to be improved upon

snow lantern
#

Just interested in a bit of clean code too and thought this might just fit the thread / discussion for small API tweaks that aid in a bit better 3rd party code.

obsidian otter
unreal kettle
#

It would be cool if there was a helper to test whether a string is an ID

arctic spade
unreal kettle
#

It would be nice if it checked if the ID exists in the databases too

dry shadow
#

i'm curious what your usecase is for needing that functionality

#

there isn't a realistic probability of the same id being reused, assuming you aren't manually creating IDs.

arctic spade
#

It would need to go through every single collection to verify the ID.

arctic spade
#

Compendiums don't count as documents (in 0.8 at least). Is that going to change?
There's no updateDocuments for it either or similar.
I feel these are all kind of necessary to go with the oft recommended route of managing packs inside foundry itself instead of external tools.

fast basalt
arctic spade
# fast basalt I will be very surprised if there is not already an established API for what you...

pack.updateDocuments([lots of disparate updates])
like I would when pushing large number of item updates on actors for example.
I was only looking at the functions available in the debug console at the time though, so that might've given incomplete view.
Not terribly high priority for anything, it just seems every system that has notably large compendiums do everything outside of Foundry because the tools I presume are not there to do it in Foundry (though also probably just for git update clarity, so that might be the bigger reason).

vagrant carbon
arctic spade
#

Oh. Alright. That feels so obscure.

vagrant carbon
#

basically, you can use all regular document life cycle methods to manipulate documents in compendium packs. you just need to provide the pack property

whole schooner
vagrant carbon
#

if you just need the ids, you should probably use index instead

whole schooner
#

oh nope, I tried updateAll, not updateDocuments

vagrant carbon
#

because they are fetched automatically

#

I think updateAll is a method on the CompendiumCollection, right?

whole schooner
#

yeah, it's not related to what we were talkin about before, ignore me lest i derail it

vagrant carbon
#

actually it comes from DocumentCollection, which WorldCollection and CompendiumCollection are subclasses of. But to me it looks like updateAll should probably be overridden in CompendiumCollection so that the compendium specific stuff is handled properly. I think it probably just needs to add the pack to the options. So while not related to what we were talking about, it is a valid bug report / API request

unreal kettle
#

Re: things that would remove the need for a hacky workaround

It would be helpful if authenticated GM users were able to make fetch requests to the database files (it currently returns a 404).

dry shadow
unreal kettle
#

I can concatenate the JSON of the Documents, but that's a workaround which might have unforseen issues

dry shadow
unreal kettle
#

I'd like to have a module capable of creating a distributable package which can be installed and therefore must have database files

dry shadow
#

There's a lot of ways for it to go wrong, there's a lot of ways for it to result in lost data. NEDB is somewhat fickle when it comes to working with files which might be actively changed, moved, copied, or otherwise open by another process.

dry shadow
unreal kettle
unreal kettle
dry shadow
#

i think i'm failling to recognize what you're doing that the compendium packs system doesn't do already--- db files that are intended to be distributed is the very definition of compendium packs in their current state

unreal kettle
#

Yes, I will use compendiums, but I can't fetch those for uploading elsewhere without getting 403s

#

I'm working on a GitHub sync module for world package creators

dry shadow
#

as in, a module that will trigger a git push of the world folder/db files?

fast basalt
#

it's not the most efficient it could possibly be, but doing this in a "safe way" that uses the scope of our API would be:

const content = await pack.getDocuments();
const source = content.map(c => JSON.stringify(c)).join("\n");
#

this has added advantages over reading the raw DB, since the raw DB may not have yet been compacted and so it could contain duplicate (append-only) records or records for documents which have since been deleted.

unreal kettle
unreal kettle
vagrant carbon