TyphonJS•17mo ago

Actor update error, TRL/Svelte Token interaction?

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.

26 Replies

RyanWAnderson•17mo ago

Nekro Darkmoon•17mo ago

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 😄

TyphonJS (Michael)•17mo ago

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.

Nekro Darkmoon•17mo ago

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

TyphonJS (Michael)•17mo ago

Yeah, I'm only vaguely aware that v11 makes some significant changes there hence testing on v11 is probably a good idea.

Nekro Darkmoon•17mo ago

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

TyphonJS (Michael)•17mo ago

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.

Nekro Darkmoon•17mo ago

yup it somehow populates the _sheet property via a hook somewhere

RyanWAnderson•17mo ago

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.

TyphonJS (Michael)•17mo ago

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.

RyanWAnderson•17mo ago

I don't have it set up. I'll take a look at it.

TyphonJS (Michael)•17mo ago

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.

RyanWAnderson•17mo ago

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.

TyphonJS (Michael)•17mo ago

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.

RyanWAnderson•17mo ago

Cool, I'll throw that their way and see what happens.

Gaming

Programming

Actor update error, TRL/Svelte Token interaction?