Implement makeSafeForNet more thoroughly for avatar-related messages
#191
Conversation
AvBrainGeneric can contain arbitrary messages, and so can (indirectly) many avatar-related messages that contain a brain and/or task. This change ensures that such nested messages are checked for net-safety.
| // These fields can contain arbitrary messages, | ||
| // so we must check them before sending the brain to other clients. | ||
| // The client never uses these fields for brains sent over the network though, | ||
| // so for simplicity, don't allow them over the network at all. | ||
| // If this causes issues, we could specifically allow NotifyMsg | ||
| // (the only message type that the client puts in these fields) | ||
| // or recursively call makeSafeForNet() on the messages. | ||
| return m_startMessage == nullptr && m_endMessage == nullptr; |
There was a problem hiding this comment.
This check is more restrictive than strictly necessary (see comment). It hasn't caused any issues in my (limited) testing, but this can be loosened if necessary.
PlasMOUL/Avatar/CoopCoordinator.cpp
Outdated
| // This can only be LinkToAgeMsg. | ||
| && (m_acceptMsg == nullptr || (m_acceptMsg->type() == ID_LinkToAgeMsg && m_acceptMsg->makeSafeForNet())); |
There was a problem hiding this comment.
I think I would prefer to see us only run makeSafeForNet() on the m_acceptMsg and not constrain on its type. That way, we're free to add more coop coordinators in the future. IIRC, TOC has something bespoke that uses coop coordinator. While they don't use DS, I think that's a good indicator that this is a useful avenue for future features.
There was a problem hiding this comment.
Sure, can do. I wanted to be safe here and keep DIRTSAND's existing behavior - before my changes here, a similar check was done in MOUL::AvCoopMsg::makeSafeForNet().
Only LinkToAgeMsg is currently used here, but we might want to use other types in the future.
AvBrainGenericcan contain arbitrary messages, and so can (indirectly) many avatar-related messages that contain a brain and/or task. This change ensures that such nested messages are checked for net-safety.H-uru/Plasma#1550 implements similar checks on the receiving end in the client.