Make an option to have SyncVar hooks called on Server

[tufcode]

Please explain the suggested feature in detail.
[SyncVar(hook = nameof(HookFn))]
HookFn is never called on Server only mode so I have to either separate server and client logic or call the function manually, which will make me write more code and make the code less readable.

However, if there were an option to have SyncVar Hooks called on the server, it would speed up the development and make everything slightly more readable.
[SyncVar(hook = nameof(HookFn), callHookOnServer = true)]

How exactly does this keep you from releasing your game right now?
I made everything dependent on SyncVar Hooks on the server not knowing it would not work on server only mode. I have to rewrite most of the logic or call the function manually to get it to work now, which is extra work.

Can you suggest a possible solution?
Add something like callHookOnServer to SyncVar Hooks so we can have some hooks called on the server.

[JesusLuvsYooh]

This would be cool.
I made my game as Client Host - Clients, and then having to adjust it later on to also work for Headless servers was a pain.

[MrGadget1024]

Per the docs, hooks only fire on clients. If you need the server to do something when setting a syncvar do it right when you set it.
Changing this would break all projects using SyncVars, so really can't.

[miwarnec]

people requested this a lot over the years.
we can do it, would just need to remove 1 check in weaver.
people could still use [Server]/[Client] attributes on their hooks to be only called on server/client.
this way, people who want to use it can at least use it. more control.

this also makes SyncField easier, where SyncField wouldn't have to depend on NetworkClient.

let's leave this open for discussion.

[felipeggrod]

I think this is the way to go.
It is nice to have a method being called on the server and client from a single trigger.
Doing that rpc/cmd dance (#2421) is nice for specific functionalities. But it sucks when we just want to watch dozens of variables, for any changes.
Also, it is easier to have data as the source of truth and everything else just react to it (yeah, like react.js :) )

For example:
The player mesh processing system watching the equipment variables,
The particle effects system watching the equipment variables and health/damage/bonuses variables.
The combat system watching the health/armor/dmg/bonuses variables.
The equipment/skills system watching the health/armor/dmg/bonuses variables.

If anyone is interested, this is my current workaround:
It is not the best approach, since we gotta attach a bunch of those in a single gameobject

[Serializable]
public class SyncHookInt : NetworkBehavior
{   
    [SyncVar(hook = nameof(OnValueChangeTrigger))] private int _value;
    public UnityAction<int> OnValueChange; // I need a way to trigger an UnityEvent/Action on the server, and have it be invoked on the server AND client once.

    public void Set(int newValue) {
        _value= newValue;
        if (isServerOnly) OnValueChange.Invoke(_value);
    }
    public int Get() {
        return _value;
    }  
    private void OnValueChangeTrigger(int oldValue, int newValue){
        OnValueChange.Invoke(newValue);
    }
}

[MrGadget1024]

this is my current workaround

This seems simpler...

public UnityAction<int, int> OnValueChangedEvent;

[SyncVar(hook = nameof(OnValueChanged))]
int _value;

public int value
{
    get
    {
        return _value;
    }
    set
    {
        OnValueChangedEvent?.Invoke(_value, value);
        _value = value;
    }
}

// SyncVar hook and event handler
void OnValueChanged(int oldValue, int newValue)
{ 
    if (isServer) { /* do server stuff */ }
    if (isClientOnly) { /* do client stuff */ }
}

public override void OnStartServer()
{
    OnValueChangedEvent += OnValueChanged;
}

[felipeggrod]

This seems simpler...

This seems simpler-er...

[SyncVar(hook = nameof(OnValueChange), callHookOnServer = true)] int value;

public void OnValueChange(int old, int new) {
    if (isServer) { /* do server stuff */ }
    if (isClientOnly) { /* do client stuff */ }
}

[felipeggrod]

Also, please take a look at this #2421

[James-Frowen]

This should be a relatively easy change, mostly just needs someone who knows a bit of weaver stuff. This is how we did it in mirage: MirageNet/Mirage#1012

This should be the main part
https://github.com/MirageNet/Mirage/blob/1dc84e03ba76eb003129eea56e478327654754e5/Assets/Mirage/Weaver/Processors/SyncVarProcessor.cs#L238-L242

Getting an event to trigger instead of methods was a bit harder because it needs to have the null check on the event field. MirageNet/Mirage#991

[felipeggrod]

That's it! That hook event also looks amazing. This enables a data driven architecture to be done much more easily.

[felipeggrod]

Still thinking this would be amazing :)

[MrGadget1024]

This happens now, sort of. If the component has SyncDirection set to "Client To Server" the owner client can set the SyncVar directly instead of via Cmd, which sends it to server and other clients. Hook fires on server and the other clients, but not the owner (for now).

[tufcode]

This happens now, sort of. If the component has SyncDirection set to "Client To Server" the owner client can set the SyncVar directly instead of via Cmd, which sends it to server and other clients. Hook fires on server and the other clients, but not the owner (for now).

This sounds like the exact same SyncVar behavior with client authoritative logic.