Validity check for deleted `Panel`s

[Alf21]

Describe the bug Currently, even if a Panel was deleted, you can still access it like before via code. If you wanna check in a Panel, whether another panel was deleted, there is no way doing so. This is needed as some UI elements require to live onto the Rootpanel directly in order to avoid overflow issues (e.g. Tooltips, Dropdown boxes, Drag and Drop boxes).

To Reproduce

label.Delete(); // some label with text for easier debugging. You could use `true` as a param as well, that doesn't change anything except could lead into a `Collection was modified; enumeration operation may not execute.` error if running inside a panel's event

Log.Error($"{label.Text}"); // This will still print the valid text

Expected behavior A way to check whether a panel was deleted.

Currently I did the following:

1. Creating a custom Panel with a var IsDeleted.
2. Setting IsDeleted = true on the panelsOnDeleted` method (No event, so I can't do this for other panel's on fly and have to create my own panels).
3. Checking in another panel, whether IsDeleted is true (via Tick).

I thought it would be cool to have a OnDeleted action, but that doesn't work as well as this could result in errors if you leave the game and everything gonna get deleted. That's why flags working the best currently as Tick() method is just called when the game is still running, obviously.

Screenshots

Here an example of a panel that exists on the Rootpanel in order to avoid overflow issues (The dropdown options): PS: No, I don't speak Russian

Example https://github.com/TTTReborn/tttreborn/blob/settings-integration/code/ui/components/dropdown/DropdownOptionHolder.cs#L22-L27 and https://github.com/TTTReborn/tttreborn/blob/settings-integration/code/ui/components/dropdown/Dropdown.cs#L112-L117

[Ryhon0]

You can check the Parent property, should be null if it was deleted

[Alf21]

You can check the Parent property, should be null if it was deleted

Already tried, valid as well (as it's the same thing, if it was deleted, it's still valid).

[ogniK5377]

Did you try to immediately delete it?

_myPanel.Delete( true );

[Alf21]

Did you try to immediately delete it?

_myPanel.Delete( true );

Oh yea, sure, check out the inline comment in the code above :D

Just to make sure, I did a test:

1. Deleted a panel via Delete(true); and set IsDeleted = true; in that panel in OnDeleted()
2. Runned following code in another panel (Tick) regarding the deleted panel:

   if (RelatedPanel.IsDeleted)
   {
   Sandbox.Log.Error($"Parent: '{RelatedPanel.Parent}', Text: '{RelatedPanel.TextLabel.Text}'");
   }

[garrynewman]

This really sounds like a code design error. Who has ownership over the panel?

[Alf21]

This really sounds like a code design error. Who has ownership over the panel?

The Rootpanel as there is no other possibility regarding overflow issues (as described above). The deleted panel's parent is a normal Panel that got deleted via DeleteChildren(true); in my case. But as described in the history of this issue, it can be reproduced directly with Delete() as well.

Additionally, the one flag / var is set on a panel's OnDeleted method. As you can see, I'm still able to access data of that panel in the following Tick method (after the panel was deleted). Even with a my sided code design issue, the panel is still valid after deleting it.

[garrynewman]

You'll have to explain the problem for me again, I'm not sure what it is. Is this right:

Tooltip shows when you hover panel.. when panel is deleted tooltip manager doesn't know that the panel is gone and carries on showing tooltip rather than adjusting to its deletion?

[Rohansi]

The existing IsDeleting property should work in place of your custom IsDeleted property.

I don't really see why we should explicitly disallow accessing properties like Text on panels which have been deleted. Despite the method name being Delete the object is still perfectly valid aside from being removed from the UI.

[Alf21]

Okay I gonna try to explain it in detail, why it's done in that way and what's buggy exactly :D

---

Why Rootpanel has to be the Parent of the Tooltip

Default implementation

With css, you could create a Tooltip in this way:

1. Add a label A
2. Set the parent of label A to label B
3. Hide the label A with opacity: 0; by default
4. Position label A with position: absolute; top: 0px; transform: translateY(-100%);
5. Add a :hover to label A with opacity: 1;

If label B would be deleted, label A would get deleted as well (as B is the parent of A).

Problem with default implementation

Tooltips, Dropdowns, etc. are positioned above or below the hovered label. Because of overflow issues (tooltips would get cut if they are out of a Panel that has overflow: scroll / hidden; e.g.), this has to be handled in a different way!

Example of the overflow issue with a dropdown:

The alternative of using Rootpanel as Parent

Using Rootpanel as a parent is a workaround to always display the tooltip on top of everything else, so overflow issues do not appear.

1. Add a label A
2. Set the parent of label A to Rootpanel, e.g. via Hud.Current.Rootpanel
3. Set label B as a related var / label on initialization of label A.
4. Hide the label A with opacity: 0; by default
5. Position label A with css position: absolute; and cs Style.Left/Top/Width functions regarding their connected label B.
6. set opacity to 1 on label B's onmouseover event, remove opacity 1 on label B's onmouseout event.

The problem with the alternative

Now, when label B is getting deleted, the label A (the tooltip or whatever) will still exists on the Rootpanel, as it was not automatically deleted when label B got deleted.

As described above, there is no way to check, whether label B was deleted.

Things I tried

• checking if label B is null
• checking if the parent of label B is null
• checking if the Text of labek B changed.
• checking if IsDeleting is true
• using different deletion methods.

Current solution:

Currently, I've to create an own Label, that stores an own IsDeleted var which is set to true on the protected override void OnDeleted(); method. Then, I'm able checking for the IsDeleted var on another panel to determine whether the panel was deleted.

Or in direct comparison with the first image of this post:

How it should work

The label B should just be null or checkable, whether it was deleted.

---

Using IsDeleting does not work as well

The existing IsDeleting property should work in place of your custom IsDeleted property.

IsDeleting describes an ongoing process, not the finished process (that the panel was removed). Additionally, it didn't worked for me, always returned false.

if (RelatedPanel.IsDeleted)
{
    Sandbox.Log.Error($"1. IsDeleting? '{RelatedPanel.IsDeleting}'");
}

It's not even true before IsDeleted is set to true (or any other time), tested with:

public override void Tick()
{
    base.Tick();

    if (RelatedPanel.IsDeleting)
    {
        Sandbox.Log.Error("IsDeleting!"); // This is NEVER getting printed. RelatedPanel is getting deleted via DeleteChildren(true/false)
    }
}

Maybe, IsDeleting is just set if a panel has a css :outro delayed animation in that exact time (when the animation is run before the panel is finally getting deleted).

I don't really see why we should explicitly disallow accessing properties like Text on panels which have been deleted. Despite the method name being Delete the object is still perfectly valid aside from being removed from the UI.

Why shall we be able to access an UI object that isn't valid anymore? We could carry tons of deleted UI elements (memory leaks) that does not have any advantage / sense.

[Rohansi]

Is the only issue here the lack of an IsDeleted property?

Why shall we be able to access an UI object that isn't valid anymore? We could carry tons of deleted UI elements (memory leaks) that does not have any advantage / sense.

There's no memory leak. It will be collected by the GC as long as nothing holds onto it.

[Alf21]

Is the only issue here the lack of an IsDeleted property?

Yes, even that I disagree about the sense of having forever deleted UI elements still accessible.

There's no memory leak. It will be collected by the GC as long as nothing holds onto it.

That's right (anyways, something that could happen with lists etc. For example, I'm storing panels into a static list to fastly access them). I more or less meant "keeping data alive that isn't needed", even in comparison how data / DOM is handled on the web.