Document that accessing Multiplayer from Node is not thread-safe

[AThousandShips]

Unsure about the exact wording so took that from the error message of SceneTree

Only added to Node as the API isn't necessarily restricted but the access to it from SceneTree is, and SceneTree is stated to be non-safe

Closes: #79323

[AThousandShips]

An additional option to reduce crashes in some contexts would be to add null checks in some methods on Node to catch cases, will investigate

Edit: done, #79332

[AThousandShips]

Ping @Faless

[mhilbrunner]

I'm torn on this. It is useful info in the edge case that you are calling them from another thread, but it also adds a lot of clutter if we add this bit in all functions that are not thread safe everywhere. The default assumption should be that things are not thread-safe unless stated otherwise, especially anything interacting with nodes or the scene/SceneTree, IMO.

OTOH, if you are new, you may not expect "multiplayer.function()" to be interacting with the Node/SceneTree like it is and think it is some kind of global thing, or something.

I'm undecided. :/

[Mickeon]

Wouldn't it make sense to add it to the @rpc documentation, instead of all this? It would catch basically everyone wanting to mess with multiplayer, without needing to be needlessly repetitive.

It could be a warning instead of a note, as well.

[AThousandShips]

Not sure about the location (as not all who use the multiplayer use @rpc), but warning is a good detail thank you, will add when I next update

[Mickeon]

I feel very strongly for the warning in @rpc, but it doesn't have to be only there, I suppose. Perhaps it only really needs to be in multiplayer's description. That's the second place users would look when messing with... multiplayer.

[YuriSizov]

It is useful info in the edge case that you are calling them from another thread, but it also adds a lot of clutter if we add this bit in all functions that are not thread safe everywhere. The default assumption should be that things are not thread-safe unless stated otherwise, especially anything interacting with nodes or the scene/SceneTree, IMO.

I agree with this POV. With the crashes fixed by #79332, I think the original issue should be closed and we don't need to add warnings specifically in these few methods. Instead it should be made clear in the multiplayer documentation in the manual.

[AThousandShips]

Closing as per above, will look at the manual side 🙂