Closed lumeriith closed 1 year ago
Jengal34
commented
3 years ago Same problem here. trying to sync (with hook) a var from a type derived class from NetworkBehaviour the host change value before the client join then the client join, it will receive a SyncHooked(null,null) instead of SyncHooked(null,newValue) (no error just wrong SyncFunction params)
lumeriith
commented
3 years ago At the time I wrote this issue about two bugs:
NetworkBehaviour SyncVars work fine, but Tester SyncVars (which inherits from NetworkBehaviour) show different value on clients.I did further testing and found out that the first bug is purely editor bug.
[SyncVar]
public MonsterBase recentMonster;
[SyncVar]
public NetworkBehaviour recentBehaviour;
...
private IEnumerator SetRandomMonsterRoutine()
{
while (true)
{
var monsters = FindObjectsOfType<MonsterBase>();
var mon = monsters[Random.Range(0, monsters.Length)];
recentMonster = mon;
recentBehaviour = mon;
yield return new WaitForSeconds(1f);
}
}
recentBehaviour field shows the correct value, but recentMonster field doesn't, even though they're set to same value on server.
In fact, recentMonster field is delayed by exactly one SyncVar set, showing what the value was the last time.
However as soon as I add a code that reads from recentMonster, you can see it starts to show correct values on editor.
The first bug became a really trivial one, the focus should be on the second bug. I will edit the title accordingly to this find.
Jengal34
commented
3 years ago Since I add some info to your bug, I think I figured out "my" problem. At the really start, the var is synched but I guess the other player component is not created for the new client. So the hook occur saying "value change from null (a real null) to null (I have a netID but no gameComponent with it)" And as soon as the game component is created, the hook is no more invoked. I feel like it s a bug. but not sure it s yours.
MrGadget1024
commented
3 years ago Since I add some info to your bug, I think I figured out "my" problem.
You're issue is a race condition if you're trying to ref something on a different object that may not yet exist on the client, which is why we caution strongly against game objects / NB's in SyncVars because we can't guarantee spawn order (or if the other thing will ever spawn due to network visibility). That's not the same as this bug report.
imerr
commented
1 year ago NB SyncVar Serialization uses writer.GetWriteFunc which looks up the right method by type: https://github.com/MirrorNetworking/Mirror/blob/master/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs#L446 or defaults to the standard Write method for NetworkBehaviour, which should be NetworkWriterExtensions.WriteNetworkBehaviour if I'm seeing this right.
NB SyncVar Deserialization uses a deserializer for the generated backing field of type "NetworkBehaviourSyncVar" to serialize it: https://github.com/MirrorNetworking/Mirror/blob/master/Assets/Mirror/Core/NetworkBehaviour.cs#L913
Thats where the mismatch is coming from with custom serializers
The fix seems to be to not use the convenient reader, but to manually call .Read
Next issue: ordering 🎉 It seem, when a NB syncvar references itself, the NB isn't actually registered as spawned until after the syncvars are populated, so the NB lookup fails Otherwise it seems to work great https://share.dl.je/2023/02/2023-02-17_19-44-50_xDvKrVo66G.mp4 see how if it spawns in with it set to itself the lookup doesnt work, but NI does
// we need to use reader.Read<T> here, since people may define custom
// readers for their network behaviours
T readNB = reader.Read<T>();
netIdField = new NetworkBehaviourSyncVar();
if (readNB)
{
netIdField.netId = readNB.netId;
netIdField.componentIndex = readNB.ComponentIndex;
}
instead of the old
netIdField = reader.ReadNetworkBehaviourSyncVar();Weaver patch to force generation of typed reader: Goes here https://github.com/MirrorNetworking/Mirror/blob/master/Assets/Mirror/Editor/Weaver/Processors/NetworkBehaviourProcessor.cs#L589
// force generation of a reader, if it doesn't exist for this type yet
// we need to do this in order to be able to use Read<T> in the deserialization helper
// We need to use Read<T> in case a custom reader/writer pair is defined for the type
// Previously the serialization code would always use the custom writer,
// while the deserialization would only use a fixed NB reader in the deserialization
// helper method we call below
// https://github.com/MirrorNetworking/Mirror/issues/2680
MethodReference readFunc = readers.GetReadFunc(syncVar.FieldType, ref WeavingFailed);
if (readFunc == null)
{
Log.Error($"Failed to generate a read method for a NetworkBehaviour subclass.", syncVar);
WeavingFailed = true;
return;
}
Describe the bug SyncVars of type deriving from NetworkBehaviours point to wrong NetworkBehaviour on clients, while RPCs work fine.
How to reproduce the issue, step by step
Let Tester.cs have following code:
Testerattached on the scene.Tester'ssyncvarandrpc.syncvarandrpcshould point to a sameTesterthat the server has set, butsyncvarpoints to wrongTester, whilerpcone works fine.Repro Project Don't mind the project name, I thought this was caused by something else before. CustomReaderWriterRepro.zip
Expected behavior The default reader/writers Mirror implements supports serializing/deserializing NetworkBehaviours. Hence both variables points to the same, correct Tester instance.
Screenshots Left: Host, Right: Client
Desktop:
Additional context Writing a custom reader/writer extension causes the Deserialization of
Testerto fail, specifically SyncVarsyncvar. When syncing SyncVars, server uses the custom writer correctly, but the client uses the default one implemented for NetworkBehaviour. This leads to EndOfStream error or a warning from Mirror telling you that it just read wrong number of bytes, and of course wrong data is synced.Left: Host, Right: Client