T
TyphonJS

typhonjs-runtime

Actor update error, TRL/Svelte Token interaction?

RRyanWAnderson5/2/2023
Ran into a bug I think, not sure if this is with Foundry or TRL. Related to the duplicate actor sheet issue Geoidesic ran into a few months back. Same issue, I'm getting duplicate character sheets when I try to run
actor.update()
actor.update()
. I traced the error back, looks like it's coming from
TokenDocument._onUpdateBaseActor()
TokenDocument._onUpdateBaseActor()
making a
this.actor.sheet.render(false)
this.actor.sheet.render(false)
call on line 565 in Foundry client/data/documents/token.js. I think this call to render is unnecessary for TRL and is causing SvelteApplication.js to throw a cannot read properties of null error on line 876 at
this.onSvelteMount({ element: this._element[0]...
this.onSvelteMount({ element: this._element[0]...
, _element[0] is undefined in this case. Looks like
TokenDocument._onUpdatedBaseActor()
TokenDocument._onUpdatedBaseActor()
isn't respecting the
{render: false}
{render: false}
context property, so passing render: false doesn't fix the issue. Not sure if that's intended behavior or not. Right now I'm looking into extending TokenDocument and just removing that line from the override as a workaround, but that's territory that I haven't really been involved in up to this point so I'm not 100% sure how to go about it. Any help would be greatly appreciated. :) You can check out my code here: https://github.com/Lekyaira/itoa/tree/19-re-format-side-bar The offending call starts here on line 19: https://github.com/Lekyaira/itoa/blob/19-re-format-side-bar/src/sheets/actor/Sidebar.svelte
GitHub
GitHub - Lekyaira/itoa at 19-re-format-side-bar
Contribute to Lekyaira/itoa development by creating an account on GitHub.
GitHub
itoa/Sidebar.svelte at 19-re-format-side-bar · Lekyaira/itoa
Contribute to Lekyaira/itoa development by creating an account on GitHub.
RRyanWAnderson5/2/2023
No description
NDNekro Darkmoon5/2/2023
this.actor.sheet.render(false) doesn't actually say to not render the first parameter of that function is force all you're saying is to not force it you want to get rid to the entire line 😄
TMTyphonJS (Michael)5/2/2023
Let me know if the suggestion from ND above is a solution. TRL / SvelteApplication guards against subsequent calls to render, but this is a strange occurrence seemingly on an unintended first render. I could adjust the onSvelteMount access to this._element[0] which is just getting that actual HTMLElement from the Foundry JQuery reference. IE change it to this._element?.[0], but that is just masking the actual problem. A second sheet / unintended render would still occur. You can go in and locally make that mod and see what happens, but I gather it'll still be a bit borked. If you do have more debugging info please do share it here. I'm pretty deep in another area currently, so don't want to do a context switch this week if possible. IE track down the bug. However, if it is a problem in Foundry now is the best time to get a suggestion in through the mothership to get it fixed in v11. It wouldn't hurt either to test on v11 as there were changes to tokens / "synthetic actors" or something like that.
NDNekro Darkmoon5/2/2023
so the second sheet rendering is actually the token sheet there's an entire thing with unlinked tokens and stuff that is super annoying hopefully it gets better with v11
TMTyphonJS (Michael)5/2/2023
Yeah, I'm only vaguely aware that v11 makes some significant changes there hence testing on v11 is probably a good idea.
NDNekro Darkmoon5/2/2023
yup there's also a thing where a linked actor sheet can't tell if its been opened by a token or from the sidebar These lines are the fix for that https://github.com/Pjb518/FoundryVTT-Level-Up-Official/blob/main/src/apps/ActorSheet.js#L64 https://github.com/Pjb518/FoundryVTT-Level-Up-Official/blob/main/src/apps/ActorSheet.js#L70 https://github.com/Pjb518/FoundryVTT-Level-Up-Official/blob/main/src/apps/ActorSheet.js#L74-L76 https://github.com/Pjb518/FoundryVTT-Level-Up-Official/blob/main/src/apps/ActorSheet.js#L353-L357 essentially a problem that arises without the above fix is that token header icons for linked actors always show the prototype token settings instead of the token settings. 😄 I believe the last time I checked @solidor system suffered from the same issue as well
TMTyphonJS (Michael)5/2/2023
Do keep me informed if there is anything actionable on the TRL side of things. From what I can tell it's core Foundry weirdness somewhere in the chain.
NDNekro Darkmoon5/2/2023
yup it somehow populates the _sheet property via a hook somewhere
RRyanWAnderson5/5/2023
That's what I suspected was going on. Thanks for the info! Hopefully v11 will fix it. Until then, it's workaround time! Haven't had time to look at this again until now. I implemented the changes @nekrodarkmoon highlighted above, didn't seem to affect the issue. Going to try overriding token and see what that breaks now.
TMTyphonJS (Michael)5/5/2023
Are you able to test in v11? I think that is probably the best environment to do any testing in presently. If there are additional problems w/ v11 quite possibly they can be addressed now before v11 ships.
RRyanWAnderson5/5/2023
I don't have it set up. I'll take a look at it.
TMTyphonJS (Michael)5/5/2023
At least for modules there is nothing really standing in the way of running on v11; hopefully the same more or less for systems. Though the DB swap is the biggest sticking point probably. Are you using the Node distributions of Foundry? That is the easiest way to set things up. I have v9, v10, v11 setup side by side and can easily switch between them. You can also make symlinks between them IE in the Data/modules or presumably even Data/systems can make symlinks to the v10 Data/modules or systems directory and it works fine. Not sure actually how that would affect packs though. I've run FQL on v11 w/ the symlink and it has packs. I haven't looked too deep into the DB swap angle. In the next FQL release I'm rewriting things to use the legacy mode of TRL and removing the packs in favor of a unified settings menu built into FQL and moving all of the packs which were for macros into the unified settings menu.
RRyanWAnderson5/5/2023
Oh wow it's all different looking now lol Issue persists. The functions are written differently now though. It's under
if ( !this.isLinked && this.delta ) {
this.delta.updateSyntheticActor();
this.actor.sheet.render(false);
}
if ( !this.isLinked && this.delta ) {
this.delta.updateSyntheticActor();
this.actor.sheet.render(false);
}
Makes me think this is intended behavior, might should be caught under
actor.sheet.render
actor.sheet.render
instead of at the token
_onUpdateBaseActor
_onUpdateBaseActor
They're trying to call an update of the displayed sheet data, we're the ones who are hooking that behavior.
TMTyphonJS (Michael)5/5/2023
I don't have much visibility into the system angle w/ demo code to work on directly. I think it's unfortunate that they don't pass along a context about why the sheet is being rendered. This is one thing that can absolutely be brought up in the v11 dev or dev-support channels on the main Foundry Discord server right now. In ClientDocumentMixin a fair amount of the render calls from Foundry include a context object that give the reason why the render was invoked. For instance look at ClientDocumentMixin._onUpdate. It invokes the associated apps render method with a context object:
this.render(false, {
action: "update",
data: data
});
this.render(false, {
action: "update",
data: data
});
You should really make a post now on the mothership asking the devs to make that render call contain a context object with action specified with perhaps updateSyntheticActor like this:
this.actor.sheet.render(false, { action: "updateSyntheticActor" });
this.actor.sheet.render(false, { action: "updateSyntheticActor" });
That way it is trivial to override render in the associated SvelteApplication and at least be able to specifically respond if necessary to this particular render call if any workarounds need to be implemented. --- FYI the render context data is the only reason TJSDocument works reactively. TJSDocument registers with ClientDocumentMixin apps and fakes the app API called (render / close). The crud updates w/ context to render allow TJSDocument to function. The Foundry document model is rather weak when it comes to a proper event model and the current app registering is a kludge. ----------- Come to think of it I can add into SvelteApplication an easy to use "renderHandler" capability where in SvelteApplication I provide the render override and when a context option with "action" key is in the object I invoke a registered "renderHandler" with the context object and if the renderHandler returns a "falsey" value the render is cancelled automatically. I'd probably put that under SvelteApplication.reactive.renderHandler where you could in the constructor do something like this:
this.reactive.renderHandler = ({ action }) => {
return action !== 'updateSyntheticActor';
}
this.reactive.renderHandler = ({ action }) => {
return action !== 'updateSyntheticActor';
}
This would return false if the core Foundry team adds the context like I mentioned above and cause the render to not occur.
RRyanWAnderson5/5/2023
Cool, I'll throw that their way and see what happens.
TMTyphonJS (Michael)5/5/2023
RRyanWAnderson5/5/2023
Awesome, thanks!
TMTyphonJS (Michael)5/6/2023
You might actually want to chime in on that message as it's hard to say how much my suggested advice will get listened to by the core team. Perhaps @nekrodarkmoon too. Having a concrete use case for why adding the context object to all core invocations of Application.render otherwise might not be taken as a pressing concern. It's really a relatively simple change, but the core team may not realize the impact it can have especially for the updateSyntheticActors use case to be able to easily tell when a render is invoked in that case. I updated my post about at least covering the "passive rendering" use cases which there are far fewer, but this does cover the updateSyntheticActors use case. Well we tried... Just hope and prepare for disappointment.. ;P About Foundry standard.. ;P The whole render / context object thing is already way fractured w/ little consistency. The ClientDocumentMixin callbacks for direct documents use "create", delete", update" under an action attribute, but embedded documents context objects use renderContext instead of action. On v11 the renderContext strings have changed with an additional . separator. And that is just the start; no consistency. The whole it's too big of an add to add a context object even for updateSyntheticActor is more a symptom of the core team not knowing the scope of what exists in general. I'm not sure what can be done to at least convince them that this one context object aspect is not going to cause a problem and solve a tough problem as it seems the issue persists on v11. The App v1 API has always been a weak link IMHO.... At least they fixed some issues I reported for a couple of areas like the Dialog.wait implementation. Fingers crossed, but don't get your hopes up.
RRyanWAnderson5/6/2023
Yup. I'm not sure why token treats its actor sheets separately to begin with. Actor sheet should be the actor sheet, token should be a token. I suspect it's because of the prototype/token difference. Maybe because of NPCs where you want different data for multiple duplicate tokens... I'm also getting some weird desync between player token and actor sheet data. I haven't spent the time to track that down yet but I suspect it's related.
TMTyphonJS (Michael)5/6/2023
It's a minor miracle I got TJSDocument / TJSDocumentCollection to work as there isn't a real event system; though I think I already brought that up. IMHO ClientDocumentMixin and DocumentCollections would benefit from a very simple subscribe capability just like the bare bones Svelte readable store contract. A single method that returns an unsubscribe function. No silly business with faking an "app" and inserting oneself. That whole idea / implementation is a hold over from the very beginnings of Foundry where it was a quick hack that snowballed. IMHO the App v1 infrastructure is so pervasive across so many areas. It's an intractable situation and I don't think the core team knows how to dig out of that hole. Hopefully, I can work with them a bit in the future; nothing Svelte related, but kind of in this holding period. Wait and see 1 or 2 more core versions.
RRyanWAnderson5/7/2023
Aaaaaand it was that easy! I made a TokenDocument class that passed options to the render function:
/**
* When the base Actor for a TokenDocument changes, we may need to update its Actor instance
* @param {object} update
* @param {object} options
* @internal
*/
_onUpdateBaseActor(update={}, options={}) {

// Update synthetic Actor data
if ( !this.isLinked && this.delta ) {
this.delta.updateSyntheticActor();

// This is the only change lol
this.actor.sheet.render(false, options);
}

this._onRelatedUpdate(update, options);
}
/**
* When the base Actor for a TokenDocument changes, we may need to update its Actor instance
* @param {object} update
* @param {object} options
* @internal
*/
_onUpdateBaseActor(update={}, options={}) {

// Update synthetic Actor data
if ( !this.isLinked && this.delta ) {
this.delta.updateSyntheticActor();

// This is the only change lol
this.actor.sheet.render(false, options);
}

this._onRelatedUpdate(update, options);
}
And in my function that updates the actor I passed options.action = 'heropointClick'; then in the ActorSheet's render function I just checked for that action and returned if I found it. Error fixed! Everything works as intended. Now if only we can get this change pushed to live. -_-
TMTyphonJS (Michael)5/7/2023
You should really post this as a follow up. As I mentioned they could easily change that code to this:
this.actor.sheet.render(false, { action: 'updateSyntheticActor', options: { ...options } });
this.actor.sheet.render(false, { action: 'updateSyntheticActor', options: { ...options } });
Do you modify the options data in the render method? That is the reason I made a shallow copy of it above so that it loosely is not modified.
RRyanWAnderson5/7/2023
Will do
TMTyphonJS (Michael)5/7/2023
So do you really need the options object or do you just need the action: 'updateSyntheticActor' part? The way Atro responded took the negative towards passing on the options. Even if options wasn't passed on would this suffice?
this.actor.sheet.render(false, { action: 'updateSyntheticActor' });
this.actor.sheet.render(false, { action: 'updateSyntheticActor' });
It is being dismissed for the wrong reason per se.
RRyanWAnderson5/7/2023
Yes that would work just fine. I just need to know where the render call is coming from.
TMTyphonJS (Michael)5/7/2023
You should communicate that as it is being dismissed for passing the options object through. IE that muddied the water. There is no reason a context object can't be added that simply has the action attribute or much less of a reason not to. Oooo Ooooo... Maybe.... maybe... Heh: referring to recent discussion in the v11 feedback channel on the mothership.. ;P @ryanwanderson you might want to chime in there and confirm. I think Atro is open to changing it to:
this.actor.sheet.render(false, { renderContext: 'updateActor' });
this.actor.sheet.render(false, { renderContext: 'updateActor' });
And you can inspect this.actor to determine if it is a synthetic actor.

Looking for more? Join the community!

T
TyphonJS

typhonjs-runtime

Join Server
Want results from more Discord servers?
Add your server
Recommended Posts
Debugging TJSMenu for Auto Animations on Level Up - 5eJust a forum thread to discuss and explore an interesting phenomenon w/ TJSMenu not responding on thZ-index on TJSDialogsFor some reason TJSDialogs have an insane z-index value which makes them always appear on top of othRequest for Comment: Have any quality of life change suggestions for the quest log for v11?Greets, @FVTT ▹ Quest Log users! I am currently planning small modifications and quality of life cQuestion: Modifying class list of SvelteApplicationA question from @dorako: > so, my module hooks into renderApplication and modifies the html[0].classBlur on moving elementsAfter a few months of being gone, this started showing up for me again. Not sure why. It seems thatQuest Log: Is there a way to arrange quests within folders or collapsible quest lines?Hi @Arkangel. Apologies it took me a bit to get around to addressing your question. This is a two paCreating a svelte store of a document's flagsIs this wise, if even possible? Like I want to create a writable store of a message's flags, so thaDropdowns are not mouse-interactable inside ApplicationShellI can't click on the entries created by either Svelecte or svelte-select if that dropdown is inside Welcome to our Foundry VTT package discussion forum!Hello fellow Foundry VTT enthusiasts. This forum channel is for discussing and asking questions abouChat Message Failed to RenderAttempting to load multiple svelte elements into a chat message. So far I was successful, but the moSettings are not a store and Chat Messages TalkI wanted to grab the `TJSGameSettings` stores, but it seems that they are not actual stores? They haSettings PageI was looking at ways to override the default foundry tab for settings and was wondering if there's Release: `svelte-standard` 0.0.23 - "secrets" in editor components respect GM user level.Greets @FVTT ▹ Developer! Just a small patch note for `svelte-standard` that went out earlier thisRelease: `svelte-standard` 0.0.22 - small fix for new a11y warnings in latest Svelte.Greets @FVTT ▹ Developer. I just pushed out a small update to `svelte-standard` to handle the new a1Core chat sidebar extensionContinuing a discussion with @voodoofrog about a project to extend the core chat sidebar panel.Release: `svelte-standard` 0.0.21 - Svelte sidebar apps refinement (final?)Greets @FVTT ▹ Developer. This update provides what should be the final refinement for `FVTTSidebarCSequencer: Powered by SvelteI've started on the long overdue conversion for sequencer to have its UI powered by Svelte, which alRelease: `svelte-standard` 0.0.20 - More Svelte sidebar appsGreets @FVTT ▹ Developer. Following on the heels of `svelte-standard` 0.0.19 the latest release furtRelease: `svelte-standard` 0.0.19 - Svelte sidebar applicationsGreets @FVTT ▹ Developer. I have put out a patch to `svelte-standard` fixing the ProseMirror librarySidebar Tab ApplicationI have, after diving deep into the Foundry Code, figured out how to actually add new Sidebar tabs.