Option to select protection level of Unity Event/Lifecycle functions

[lordmortis]

Currently when generating event functions, Rider generates them all as private, I was wondering if an option could be added to set the default protection level of generated event functions?

(related to: #1493 )

[citizenmatt]

Could you provide some reasons for changing this? These event functions are only supposed to be called by Unity, so there is little reason to make them visible to user code

[lordmortis]

I've seen editor testing where they are called manually. You could use reflection for this, but it's quite messy. I've also seen overrides on sub-classes Even though they are only called by the unity engine via C++ reflection, there's an argument to be made that because they are being called from outside the class/assembly, the protection level is essentially public.

If you look at the settings window of rider itself, in Editor -> Color Sceme -> Unity, the update function in the example is also declared public:

[citizenmatt]

That settings code is just a plain block of handwritten text 😄 I wouldn't put too much stock in that one.

It looks like there are two different issues here - calling directly and overriding. Calling directly would require generating at either public, protected or even internal level, but overriding would require protected virtual. We can investigate adding an option to set the access rights, but in the meantime, especially if the change in visibility is ad-hoc, it's possible to quickly change access rights with Alt+Enter on the private keyword, or from usage if you try to use a private method in another class.

[lordmortis]

The overriding appears to work fine if there's a subclass: if i have:

public class MyFunkyBase : MonoBehaviour
{
    protected virtual void Update()
    {
    }
}

and I make:

public class Subclass : MyFunkyBase
{
}

Asking for a unity event override does the right thing and adds a protected override void Update(), so that works.

alt-enter is pretty quick to do, so that's a good workaround, just be handy for those that declare them public-by-default to not have to do that step :D

[benblo]

It's a common policy in many studios to have all Unity messages protected by default. It's also part of the Microsoft/UnityVS analyzers (albeit opt-in): https://github.com/microsoft/Microsoft.Unity.Analyzers/blob/main/doc/UNT0021.md

Silently erasing a base message in a derived class without realizing it is a very common source of bug. I've several times found lot of bugs "for free" when landing in a new codebase by mass replacing "void Update" by "protected void Update()"... suddenly the compiler does its job, warning that things look fishy! Then you can fix it (add virtual, or indeed erase with new if that was the intention), but at least you're aware of it, it's not silent anymore.

there is little reason to make them visible to user code

IMO this is the wrong way to look at it; it's not about making them visible, it's about respecting the OO paradigm.

Unity made a horrible mistake years ago with those magic messages, they clearly should have just been virtual methods (somehow it seemed more Flash-like and friendly at the time...). It's too late for them to change MonoBehaviour now, but clearly all their new APIs now use proper OO (either base classes with virtuals, or interfaces).