feat: SyncVar hooks can have 2, 1 or no params#4077
Conversation
| void OnValueChanged(int oldValue) | ||
| { | ||
| HookCalled.Invoke(oldValue); | ||
| } |
There was a problem hiding this comment.
| void OnValueChanged(int oldValue) | |
| { | |
| HookCalled.Invoke(oldValue); | |
| } | |
| void OnValueChanged(int oldValue) => HookCalled.Invoke(oldValue); |
| void OnValueChanged() | ||
| { | ||
| HookCalled.Invoke(); | ||
| } |
There was a problem hiding this comment.
| void OnValueChanged() | |
| { | |
| HookCalled.Invoke(); | |
| } | |
| void OnValueChanged() => HookCalled.Invoke(); |
| public int value = 0; | ||
|
|
||
| public event Action<int> HookCalled; | ||
|
|
There was a problem hiding this comment.
Remove blank line
| void OnValueChanged(int oldValue) | ||
| { | ||
| HookCalled.Invoke(1); | ||
| } |
There was a problem hiding this comment.
| void OnValueChanged(int oldValue) | |
| { | |
| HookCalled.Invoke(1); | |
| } | |
| void OnValueChanged(int oldValue) => HookCalled.Invoke(1); |
| void OnValueChanged(int oldValue, int newValue) | ||
| { | ||
| HookCalled.Invoke(2); | ||
| } |
There was a problem hiding this comment.
| void OnValueChanged(int oldValue, int newValue) | |
| { | |
| HookCalled.Invoke(2); | |
| } | |
| void OnValueChanged(int oldValue, int newValue) => HookCalled.Invoke(2); |
| void onChangeHealth() | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
| void onChangeHealth() | |
| { | |
| } | |
| void onChangeHealth() { } |
| void onChangeHealth(int oldValue) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
| void onChangeHealth(int oldValue) | |
| { | |
| } | |
| void onChangeHealth(int oldValue) { } |
| void onChangeHealth(int oldValue, int newValue) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
| void onChangeHealth(int oldValue, int newValue) | |
| { | |
| } | |
| void onChangeHealth(int oldValue, int newValue) { } |
| void onChangeHealth(int someOtherValue, int moreValue, bool anotherValue) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
| void onChangeHealth(int someOtherValue, int moreValue, bool anotherValue) | |
| { | |
| } | |
| void onChangeHealth(int someOtherValue, int moreValue, bool anotherValue) { } |
| void onChangeHealth(int someOtherValue, int moreValue, int anotherValue, int moreValues) | ||
| { | ||
|
|
||
| } |
There was a problem hiding this comment.
| void onChangeHealth(int someOtherValue, int moreValue, int anotherValue, int moreValues) | |
| { | |
| } | |
| void onChangeHealth(int someOtherValue, int moreValue, int anotherValue, int moreValues) { } |
It's only ~100 new loc excluding tests (which are great), and if we can deprecate the 2-param signature that will eventually remove almost as much code. Most cases don't need Ideally, we should warn for 2-param hooks to advise users to have one param or none, while still allowing two params to compile and work as they do today so there's a transition window for existing projects. |
The weaver does this by generating a shim method with 2 params that calls the actual hook method
Not 100% sure if we actually want this for an extra 200 lines of weaver code
Tested with both mono&il2cpp builds (not tested virtual/static hook calls yet, will do if we plan on merging this)