ArnoldSmith86 / virtualtabletop

a virtual surface in the browser on which you can play board, dice and card games
https://virtualtabletop.io
GNU General Public License v3.0
173 stars 31 forks source link

De-parenting has unwanted side effects #12

Open rogerl50 opened 3 years ago

rogerl50 commented 3 years ago

If I'm editing a widget's JSON and remove its parent attribute, or add one, then usually it disappears off the board, or at least moves to some unexpected location. This of course is because the coordinates change from absolute to relative. It would be really user-friendly if a change in parentage caused the code to revisit the coordinates, probably by assuming that the widget is at the place on the screen where the user wants it and computing new xy coordinates from the position of the holder.

UltimateGeek commented 3 years ago

Another scenario of bad coordinates I've run into is the opposite of de-parenting:

  1. I hand modify a widget to make it a child of another widget
  2. I forget to modify the X & Y values to be relative
  3. It disappears off the board
  4. So I have to move the parent's position on the board to recover the child widget

Just another scenario where it would be nice for the client to "revisit" (i.e. validate) the coordinate of a widget, and it automatically changes the X & Y to a safe value.

rogerl50 commented 3 years ago

Yes. And sometimes you don't know where it went, and sometimes it's off the board and you have to go to Ghetto to find it. That's basically one of the two scenarios I was trying to describe in the original issue.

RaphaelAlvez commented 3 years ago

This can be easily fixed using a macro on Ghetto. I was thinking about creating on for parenting but I may do both

robartsd commented 3 years ago

Perhaps the built in JSON editor should offer to change coordinates if parent has changed but x, y position has not.

96LawDawg commented 2 years ago

Closed by #521.

rogerl50 commented 2 years ago

This is not the same problem. While fixedParent prevents de-parenting when moving a widget, this PR wants the widget to not physically move onscreen when de-parented in the editor.

96LawDawg commented 2 years ago

Sure. Makes sense. I was too eager to close some old issues. Actually this might be easier to solve now with _absoluteX and Y.

robartsd commented 2 years ago

With #903 setting x to _absoluteX, y to _absoluteY, scale to _absoluteScale, and rotation to _absoluteRotation, then setting parent to null should cause the widget to not move at all except for any layering effect changes.

robartsd commented 1 year ago

One difficulty with fixing this is that the JSON editor tries to update live as you type (which generally we want to keep), so there is not really an way to determine if x and/or y have been changed without a fairly complicated system for tracking edits.

We could add a Change Parent button. I think the button should give the option to update x,y so that the widget's relative position in the room does not change (default option if widget parent get changed to or from null, but defaulting to keeping the widget x, y unchanged if changing from one parent to another).

96LawDawg commented 1 year ago

I've been thinking about this issue as well. I don't think we need or should do that. It is overly complicated. When building a room, I sometimes change parents just by changing the parent name by one letter or number. An automated process would get in the way.

And after more thought, I don't think this is as big as a problem as it once was. We have a zoomed out room view, we have the JSON Editor with its search features and the tree. None of that existed when this issue was created. The fact that it is 2 years old with no solution has as much to do with the fact that we have adequate work around solutions as it does with the fact that it is a little complicated to fix.

I would close this issue.

robartsd commented 1 year ago

I've been thinking about this issue as well. I don't think we need or should do that. It is overly complicated. When building a room, I sometimes change parents just by changing the parent name by one letter or number. An automated process would get in the way.

I agree that we should leave the current behavior as is when editing JSON text directly in the JSON editor.

We might consider changing the behavior when editing the raw JSON in the "basic" editor (although I don't think anyone is actually doing that). We might also want to make a button in one or both editors.