gilzoide / unity-update-manager

Simple to use Update Manager pattern for Unity + Jobified Update for MonoBehaviours and pure C# classes alike
The Unlicense
72 stars 8 forks source link

[ExecuteAlways] fights with AManagedBehaviour #17

Closed rever96 closed 9 months ago

rever96 commented 9 months ago

I would like to have a script that executes both in editormode and in playmode because it applies changes to material rend. but with AManagedBehaviour, IUpdatable it gives me tons of error when exiting the playmode because he try to call dontdestroyonload in editormode.

I don't know if u would support it. But if you do, an easy fix would be to have the UpdateManager as a singleton prefab to add in the scene

gilzoide commented 9 months ago

Hey there, thanks for the report! Do you need for the object to be updated in edit mode, or is it just about not getting the errors? Because not getting errors is super easy to fix, we just need to only register/unregister objects or calling DontDestroyOnLoad if (Application.isPlaying). But making managed updates work in editor is not so much, although it should be possible (there are nuances around DontDestroyOnLoad that makes it a little difficult to get it right in edit mode).

Not exactly related to the issue, which I'm conviced should be dealt with, but maybe using OnValidate works for your use case, instead of relying on [ExecuteAlways]. Just a thought.

rever96 commented 9 months ago

OnValidate gets called only when values changes, my goal is to replicate the environment like in playmode so it's easier to create the leveldesign (light insigna that turn on and off for example wouldn't be updated with OnValidate).

I'm just about to not get the error. So i can manually add the update in editmode. ie.

[ExecuteAlways]
public class Neon : AManagedBehaviour, IUpdatable
{
    public static readonly int NEON_INTENSITY = Shader.PropertyToID("_NeonIntensity");

    [SerializeField] private float speed = 1;
    [SerializeField] private float intensity = 1;
    [SerializeField] private CompositeAnimationCurveSo animationCurve;
    private float _animationTimePosition;
    [SerializeField] private Renderer rend;

    void Start()
    {
        rend = gameObject.GetComponent<Renderer>();
    }

    public void ManagedUpdate()
    {
        if (animationCurve != null)
        {
            _animationTimePosition += speed * Time.deltaTime;

            float oscillatingValue = Mathf.PingPong(_animationTimePosition, 1.0f);

            float animation_progress = Mathf.Lerp(0, 1, animationCurve.evaluate(oscillatingValue));

            rend.SetShaderValue(NEON_INTENSITY, animation_progress * intensity);
        }
    }

#if UNITY_EDITOR
    public void Update()
    {
        if (!EditorApplication.isPlayingOrWillChangePlaymode)
        {
            ManagedUpdate();
        }
    }
#endif
}

I think an approach like this is good because doesn't affect whole editor performance as it would do applying [ExecuteAlways] on the manager. The script that would run always will be very few so it will be good to keep that away from the managed update optimization

gilzoide commented 9 months ago

Ok, got it.

I'm just about to not get the error. So i can manually add the update in edit mode. ie.

Well, that means you do want the object to be updated in edit mode. To be honest, I'd rather make UpdateManager work 100% in edit mode so that you and other people can avoid these kinds of editor-only Update hacks, which are potential sources of bug. I'll let you know when I come up with something.

In the mean time, if you want a hacky way out of the errors, you can override OnEnable and OnDisable, possibly inside a #if UNITY_EDITOR block, and only call the base implementations if the game is playing, something in the lines of:

protected override void OnEnable()
{
    if (Application.isPlaying)
    {
        base.OnEnable();
    }
}

protected override void OnDisable()
{
    if (Application.isPlaying)
    {
        base.OnDisable();
    }
}

This should work, since they are the methods that try to instantiate the UpdateManager, which in turn calls DontDestroyOnLoad.

gilzoide commented 9 months ago

I think an approach like this is good because doesn't affect whole editor performance as it would do applying [ExecuteAlways] on the manager.

I really don't think it would make that much of a difference, since the UpdateManager only updates objects that register themselves, such as active and enabled AManagedBehaviours. In edit mode, only objects that are also marked as [ExecuteInEditMode] or [ExecuteAlways] would be registered for updates, so it shouldn't be any worse than letting Unity update them in edit mode.

gilzoide commented 9 months ago

Hey @rever96, I made the UpdateManager work in edit mode, it is in the branch rc-5.1.2. I also released the 1.5.2-preview1 tag, I think OpenUPM will release this preview version automatically at some point.

I made it so that ManagedUpdate methods will be called in edit mode, so that after the fix you can drop the Update workaround. Please try it out and tell me if it works for your case.

gilzoide commented 9 months ago

I'm closing this as fixed by #18, feel free to reopen the issue in case the problem persists.

rever96 commented 9 months ago

Hi,

sry I didn't had time to check the other branch.

why not use the main branch for this update?

Kind regards, Loris Toia

Message ID: @.*** com>

gilzoide commented 9 months ago

No problem! I hope it works for you, like it worked in my tests.

why not use the main branch for this update?

Making separate branches for new unreleased versions is just a way to organize the workflow. Preview releases of the new version go into the branch. When the new branch/version is considered stable, I merge into main and bump the version to a non-preview stable version and release it. If there were fixes to make on top of it, main would still be stable code, while we fixed the new version. This is just one of the many ways to organize Git workflows, each one has its pros and cons.