editor icon indicating copy to clipboard operation
editor copied to clipboard

When an Entity deletion is attempted, warn the user if it is referenced by a script in the scene

Open yaustar opened this issue 4 years ago • 11 comments

This is useful for teams where it's difficult to know if an Entity is referenced elsewhere in the scene. When deleting the Entity, we could warn the user that it is being referenced by other Entities and give them a list of paths to where they are in the scene hierarchy.

Edit: to be clear, the user would attempt to delete the entity and the popup would appear before the actual deletion of the entity.

A stretch goal would be to find all references to an Entity in a similar way that we do with assets.

yaustar avatar Jan 28 '21 16:01 yaustar

I would argue about "notification" approach (it is obtrusive), and follow same approach it is with assets: we have "References" context menu, which shows where it is referenced. So developer can make a decision himself, check references or remove without concerns.

"Replace" context menu on assets is really usefull too, but I'm afraid it doesn't make direct sense on entity, perhaps different wording. Would be a good Editor Plugin too ;)

A better way for showing missing references (entities and assets), would be a good feature too. There are a lot of cases where missing references can be a thing, like copying entities/assets, removing stuff, etc.

Maksims avatar Jan 28 '21 16:01 Maksims

Would that be that intrusive if it only happened when it is being referenced elsewhere though?

I would rather know when it happens rather than remembering to manually checking every time I consider deleting an entity. Is it often that someone would delete an Entity and not want to know if doing so could potentially break another Entity's logic?

yaustar avatar Jan 28 '21 16:01 yaustar

Would that be that intrusive if it only happened when it is being referenced elsewhere though?

Any conditional intrusion - is bad. Especially when it is conditional. Assets deletion for example is not conditional, and is really important save measure, as undoing assets deletion is not possible.

Is it often that someone would delete an Entity and not want to know if doing so could potentially break another Entity's logic?

I often do so, or am aware that it is refernced, so will delete something, than update reference for example.

What better would be, is a way to show that some entity requires an attention for example, subtle but visible in hierarchy. That attention can be based on missing mandatory attribute as suggested in https://github.com/playcanvas/editor/issues/288 or a missing reference. Same would apply to asset, like a missing reference to texture.

Subtle consistent icon, that could be used across everywhere - would be a good and non intrusive way to help developers. And missing references - is a valid state for application. Usually it is best to make sure your scripts take it in an account.

Maksims avatar Jan 28 '21 16:01 Maksims

I often do so, or am aware that it is refernced, so will delete something, than update reference for example.

In a team scenario, its harder to be aware that it is being referenced. We could add an option to enable/disable this warning in the user settings so it's up to the user?

Having an icon would be great but it's something that can be missed if they aren't looking at the hierarchy and instead, more focused in the 3D view.

yaustar avatar Jan 28 '21 17:01 yaustar

Intrusive popup, will only help to inform of deletion. It does not help to re-assign references, nor it helps to do anything after the deletion happened. Popups - is almost always a bad UX.

To solve an issue of missing references, assist developer of finding where references are missing, and provide a ways to change references, we need:

  1. "References" context menu on Entity, same as on Asset.
  2. Subtle icon "call for attention", on entities (in hierarchy) and assets (assets panel), to show missing references and/or mandatory attributes (discussed new feature in issue mentioned before).

Maksims avatar Jan 28 '21 18:01 Maksims

I agree with the other features you've described would be useful. However, they don't solve this not uncommon scenario:

Entity A references Entity B.

User deletes Entity B, then Entity C, do some work on materials and then delete Entity D. They see that Entity A has a warning about a missing reference, but they don't know which Entity deletion has caused it.

They would have to like at the diff in version control and either undo the whole stack or remake the Entity.

Having a popup/warning when Entity B is deleted saying: 'Entity A is referencing this, are you sure you want to delete this? Yes/No/Don't warn me again' would prevent this.

Knowing that a reference is missing is great, but doesn't help when they need to know what it used to reference to and how to get it back.

yaustar avatar Jan 28 '21 19:01 yaustar

but doesn't help when they need to know what it used to reference to and how to get it back.

Indeed, nor the popup helps them to identify it after referenced entity was deleted. This will go even worse with deleting hierarchies of entities. So intrusive popup does very little help. And no help at all in a collaborative context.

Maksims avatar Jan 28 '21 20:01 Maksims

Indeed, nor the popup helps them to identify it after referenced entity was deleted

It will pop up before it gets deleted though.

Edit: I've realised I've not made that clear. Editing original post to clarify

yaustar avatar Jan 28 '21 20:01 yaustar

It will pop up before it gets deleted though.

I understand a proposed UX, but again, this gives no use to after the deletion, or in teams it can also be not known to person who is deleting what to do with references.

Perhaps, being a popups intrusive, if there would be a way to mark a script attribute as "mandatory", then it would show a popup (unless ever marked to "do not show").

And still, it is only helps at the moment before deletion. While there is no way to find out before deletion (no "References" context menu), and does not help after deletion (no ways to know where mandatory references are missing).

Maksims avatar Jan 28 '21 22:01 Maksims

Oh yeah, I'm not saying that this is an either/or situation. Personally, I would like to have all these options. Popup warning, indicators in the scene hierarchy and in the inspector too.

Perhaps, being a popups intrusive, if there would be a way to mark a script attribute as "mandatory"

I was wondering about this too. Could definitely start with that and see if the indicators would be enough on its own?

yaustar avatar Jan 28 '21 22:01 yaustar

Could definitely start with that and see if the indicators would be enough on its own?

It definitely will help, but there is never enough 😆

Maksims avatar Jan 28 '21 22:01 Maksims