MultiplayerSynchronizer.add_visibility_filter seems not work

[dicarne]

Tested versions

• Reproduced in 4.3.beta1

System information

Windows11

Issue description

MultiplayerSynchronizer.add_visibility_filter does not seem to change the visibility of the corresponding peer, even though the filter returns true. I thought that if the filter returns true, it means it is visible, but it is not. Is there something wrong with my understanding?

Steps to reproduce

1. nodes like this: MultiplayerSynchronizer setting:

2. add filter:

3. check peer's visibility, I think if the visibility filter returns true, then visibility should be true.

however, visibility is still false.

4. However, when using set_visibility_for to set visibility directly, it takes effect

code overview

Minimal reproduction project (MRP)

add_visibility_filter.zip

[alpapaydin]

also having similar problems, synchronizer visibility needs better documentation

[dicarne]

bool MultiplayerSynchronizer::is_visible_to(int p_peer) {
	if (visibility_filters.size()) {
		Variant arg = p_peer;
		const Variant *argv[1] = { &arg };
		for (Callable filter : visibility_filters) {
			Variant ret;
			Callable::CallError err;
			filter.callp(argv, 1, ret, err);
			ERR_FAIL_COND_V(err.error != Callable::CallError::CALL_OK || ret.get_type() != Variant::BOOL, false);
			if (!ret.operator bool()) {
				return false;
			}
		}
	}
	return peer_visibility.has(0) || peer_visibility.has(p_peer);
}

The relevant source code is as above. It seems that if any filter returns false, it is not visible. If it is public or has been explicitly specified as visible to p_peer, then it is visible. That is, it is visible only when:

1. The node is public and passes all filters.
2. The node has been specified as visible to a peer, and all filters can allow this peer to be visible.

Therefore: When the node is specified as not public, but the filter allows a peer to be visible, and has not been explicitly specified as visible to this peer through set_visibility_for, this node will not be seen by this peer.

[voidexp]

This genuinely looks like a bug. If the filter returns true, isn't that sufficient to make a decision about the visibility? What's the usefulness of needing to pass the filter AND set the visibility explicitly for the node? Especially if the function is called add_visibility_filter()

[dginovker]

Based on the code snippet provided by dicarne, it looks like the filter runs ON TOP of the public visibility. So if public visibility is false, and you only use a filter, NO ONE will see your syncs. You have to set public visibility to TRUE (or give a particular peer visibility to the object). Yikes that is not intuitive. I spent 2 days debugging this.

Note I would LOVE if we had "visibility groups", i.e. if you're part of the node visibility group, you get the sync changes, otherwise you don't. This would simplify everything.

Much love Godot devs. +1 to whoever documents this better and +1000 if anyone can add visibility groups 🥹