BasicWidget does not show text of active face #685

New issue

Closed

opened 2026-02-20 10:19:44 -05:00 by deekerman · 2 comments

deekerman commented

2026-02-20 10:19:44 -05:00

Owner

Originally created by @ArnoldSmith86 on GitHub (Mar 2, 2023).

Wow. Fun times. Turns out that setting activeFace in a basic widget does not copy the properties from the face to the widget. Never did.

It copies it to this.state which is basically the "actual JSON" of the widget saved inside the Widget object. That is never sent to the server but it is what all the editors will show you and once you save, it will be sent to the server (and the JSON editor saves automatically).

You can test this yourself. Add this widget to a new room:

{
  "id": "X",
  "faces": [
    { "text": "A" },
    { "text": "B" }
  ],
  "text": "X"
}

You would expect it to show X. It does. Click it. You expect it to show B. It does. Reload the room and it shows X again. This happens since github.com/ArnoldSmith86/virtualtabletop@f922834c2f.

Now try this:

{
  "id": "X",
  "faces": [
    { "image": "/assets/-807326126_1918" },
    { "image": "/assets/1742233512_1635" }
  ],
  "image": "/assets/1242550039_3992"
}

Works just fine. You see the image of the active face. And you can use the state API (https://virtualtabletop.io/state/<room>) to check that the property image of the widget does not change.

The problem is in this piece of code:

  applyDeltaToDOM(delta) {
    super.applyDeltaToDOM(delta);
    if(delta.activeFace !== undefined || delta.faces !== undefined) {
      let face = this.faces()[this.get('activeFace')];
      if(face !== undefined)
        this.applyDelta(face);
    }
    if(delta.text !== undefined)
      setText(this.domElement, delta.text);

    // ...
  }

super.applyDeltaToDOM(delta); applies the actual image property to the widget.
Then this.applyDelta(face); applies the image and text properties of the active face to the widget.
Then setText(this.domElement, delta.text); applies the actual text property to the widget.

I'm pretty sure that the fix is to replace setText(this.domElement, delta.text); by setText(this.domElement, this.get('text')); because this.get('text') actually returns the text property of the active face (so GET should as well). But after this roller coaster, I'll have to ponder about the meaning of life for a second before opening a PR.

This also means that I'll revisit https://github.com/ArnoldSmith86/virtualtabletop/pull/536. We can probably just do what that PR does without changing game behavior. That PR contains the fix since https://github.com/ArnoldSmith86/virtualtabletop/pull/536/commits/e319a30b537c5316463c68fd5ddc54731f3503dd...

Originally created by @ArnoldSmith86 on GitHub (Mar 2, 2023). Wow. Fun times. Turns out that setting `activeFace` in a basic widget does _not_ copy the properties from the face to the widget. Never did. It copies it to `this.state` which is basically the "actual JSON" of the widget saved inside the Widget object. That is never sent to the server **but** it is what all the editors will show you and once you save, it _will_ be sent to the server (and the JSON editor saves automatically). You can test this yourself. Add this widget to a new room: ```JSON { "id": "X", "faces": [ { "text": "A" }, { "text": "B" } ], "text": "X" } ``` You would expect it to show `X`. It does. Click it. You expect it to show `B`. It does. Reload the room and it shows `X` again. This happens since https://github.com/ArnoldSmith86/virtualtabletop/commit/f922834c2f9a9b776e292ad6501ff179bfff4156. Now try this: ```JSON { "id": "X", "faces": [ { "image": "/assets/-807326126_1918" }, { "image": "/assets/1742233512_1635" } ], "image": "/assets/1242550039_3992" } ``` Works just fine. You see the image of the active face. And you can use the state API (`https://virtualtabletop.io/state/<room>`) to check that the property `image` of the widget does not change. The problem is in this piece of code: ```Javascript applyDeltaToDOM(delta) { super.applyDeltaToDOM(delta); if(delta.activeFace !== undefined || delta.faces !== undefined) { let face = this.faces()[this.get('activeFace')]; if(face !== undefined) this.applyDelta(face); } if(delta.text !== undefined) setText(this.domElement, delta.text); // ... } ``` 1. `super.applyDeltaToDOM(delta);` applies the actual `image` property to the widget. 2. Then `this.applyDelta(face);` applies the `image` and `text` properties of the active face to the widget. 3. Then `setText(this.domElement, delta.text);` applies the actual `text` property to the widget. I'm pretty sure that the fix is to replace `setText(this.domElement, delta.text);` by `setText(this.domElement, this.get('text'));` because `this.get('text')` actually returns the `text` property of the active face (so `GET` should as well). But after this roller coaster, I'll have to ponder about the meaning of life for a second before opening a PR. This also means that I'll revisit https://github.com/ArnoldSmith86/virtualtabletop/pull/536. We can probably just do what that PR does without changing game behavior. That PR contains the fix since https://github.com/ArnoldSmith86/virtualtabletop/pull/536/commits/e319a30b537c5316463c68fd5ddc54731f3503dd...

deekerman

2026-02-20 10:19:44 -05:00

closed this issue
added the
bug

widget properties
labels

deekerman commented

2026-02-20 10:19:45 -05:00

Author

Owner

@robartsd commented on GitHub (Mar 2, 2023):

So if it is only the editor that is copying to the widget, this should be fixed in the editor (as well as your proposed fix for text).

I think changing away from a face leaves the property set by the face in the state unless it was also set on the face being changed to. This would mean if a property is only set on one face, after seeing that face, the property would remain in state; but a client joining later would not have the property in state. Anything that causes different clients to have a different idea of the shared state is clearly a bug; but I'm not sure how we should fix this.

@robartsd commented on GitHub (Mar 2, 2023): So if it is only the editor that is copying to the widget, this should be fixed in the editor (as well as your proposed fix for text). I think changing away from a face leaves the property set by the face in the state unless it was also set on the face being changed to. This would mean if a property is only set on one face, after seeing that face, the property would remain in state; but a client joining later would not have the property in state. Anything that causes different clients to have a different idea of the shared state is clearly a bug; but I'm not sure how we should fix this.

deekerman commented

2026-02-20 10:19:45 -05:00

Author

Owner

@ArnoldSmith86 commented on GitHub (Mar 3, 2023):

Didn't you fix that in #536?

@ArnoldSmith86 commented on GitHub (Mar 3, 2023): Didn't you fix that in #536?