Prevent crash when calling set_text() on a removed TreeItem

[jsjtxietian]

Fixes #85650

Also, should I use add more description or just use ERR_FAIL_NULL is sufficent ?

[AThousandShips]

This should be before updating the text IMO, in the relevant methods calling these methods

Unless we can confirm it's valid and doesn't have strange sideeffects to update the text off the tree

Also what about other methods like TreeItem::_cell_selected?

[YuriSizov]

This would just raise a pointless error. It should be a normal if-check, not an error macro.

Also what about other methods like TreeItem::_cell_selected?

And indeed, other similar methods should also be updated with such checks.

[jsjtxietian]

This should be before updating the text IMO, in the relevant methods calling these methods

Yes but there are too many places that might call _changed_notify, since the previous assumption tree items were always in a tree

[AThousandShips]

Yes but there are too many places that might call

I don't think that's a valid argument against adding it there, but if you confirm that it's safe and doesn't cause any side effects to manipulate the text etc. not on the tree we can add it just to the places where tree is accessed, otherwise I'd say we shouldn't allow the manipulation if it's not safe

For example, adding a item to the tree does not ensure item_changed is called, so it's not clear if some updates required to happen are done if you manipulate the item off the tree and then add it, we'd need to verify this before we make this change

[jsjtxietian]

we'd need to verify this before we make this change

Indeed, you are right. Does anyone familiar with this part of code and can give me some suggestions?

[AThousandShips]

A basic test would be to try your own code and see if there's any noticeable difference, if there isn't studying the code in more detail should help

Edit: if it doesn't work I'd say each function that doesn't work should have an error fail saying something like "Manipulating TreeItem not in a tree is not supported."

[YuriSizov]

I think this is good as is right now. I don't particularly see a reason to check places that call these methods. Changing items should be fine out of Tree.

[AThousandShips]

I'll trust that the manipulation of these is safe off the tree, despite the aspects mentioned above 🙂, but won't approve myself as I can't confirm it

[akien-mga]

Thanks!