Giter VIP home page Giter VIP logo

Comments (10)

soraphis avatar soraphis commented on June 20, 2024

what are your settings about fast enter playmode?

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

what are your settings about fast enter playmode?

Reload domain and reload scene set to false

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

I might have a found the source for some of the reset, in AtomBaseVariableInstancer.cs there's this extremely weird bit:

        private void OnEnable()
        {
            if (Base == null)
            {
                _inMemoryCopy = ScriptableObject.CreateInstance<V>();
            }
            else if(_inMemoryCopy == null)
            {
                _inMemoryCopy = Instantiate(Base);
            }
            ImplSpecificSetup();
        }

So if the instancer has no base var, it's always reset on OnEnable
(and too bad for anyone listening to the old instance)
but if it DOES have a base var, then it checks that it actually needs a reset?

Why would a base var change the behavior so much?

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

The others are simply called through OnEnable

I don't think any part of the code takes in account that OnEnable may be called multiple times on a SO.

Why don't Variables init on Awake instead ?

        protected virtual void Awake()
        {
            SetInitialValues();
        }

        protected virtual void OnEnable()
        {
            TriggerInitialEvents();

#if UNITY_EDITOR
            if (EditorSettings.enterPlayModeOptionsEnabled)
            {
                _instances.Add(this);

                EditorApplication.playModeStateChanged -= HandlePlayModeStateChange;
                EditorApplication.playModeStateChanged += HandlePlayModeStateChange;
            }
#endif
        }

        private void OnDestroy()
        {
            // NOTE: This will not be called when deleting the Atom from the editor.
            // Therefore, there might still be null instances, but even though not ideal,
            // it should not cause any problems.
            // More info: https://issuetracker.unity3d.com/issues/ondisable-and-ondestroy-methods-are-not-called-when-a-scriptableobject-is-deleted-manually-in-project-window 
#if UNITY_EDITOR
            _instances.Remove(this);
#endif
        }

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

I've pushed my version that tries to avoid these problems here:

https://github.com/lgarczyn/unity-atoms/tree/master

from unity-atoms.

soraphis avatar soraphis commented on June 20, 2024

I don't think Awake is the correct approach here.

first, Awake seems to be somewhat broken... It Also seems to be more like a "OnCreated" callback, which is NOT the moment to call "SetInitialValues" (as it wouldn't do anything then).

We might not need the call at all - but I'm not certain about that - as the playModeStateChange events might be good enough.

There are ultimatively a couple of different cases to check:

IntVariable

  • must be set to it's initialValue when leaving playmode
  • must Trigger InitialEvents when entering playmode
  • must keep it's value when unity hot reloads
  • should not trigger Initial events when hot reloading

Furthermore:

  • Variables in an IntVariableInstancer must behave exactly the same.
  • All tests need to be run (and pass) with and without Domain Reload.

sadly only 2(×2) of those can be checked with unit tests. As we can't really change the domain reload thing with unit tests and can't rly check the hot reload with unit tests ...

I feel there is still a lot more to be checked and taken into account, but I don't remember exactly all the issues we had with this in the past...

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

You should be able to check domain reload through unit tests by using this:

https://docs.unity.cn/Packages/[email protected]/api/UnityEngine.TestTools.WaitForDomainReload.html

And calling https://docs.unity3d.com/ScriptReference/EditorUtility.RequestScriptReload.html

SetInitialValues is required to be called on Awake or OnEnable to ensure that the serialized current and previous value are not mismatched with the initial value, which maybe could happen ?

It's also required to ensure that the callback for EnterPlayMode is registered, but that could be handled using many other systems.

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

I also fixed the instancer bug: lgarczyn@267eaeb

from unity-atoms.

soraphis avatar soraphis commented on June 20, 2024

SetInitialValues is required to be called on Awake or OnEnable to ensure that the serialized current and previous value are not mismatched with the initial value, which maybe could happen ?

SetInitialValues in Awake has no effect, since it is only called at asset creation, _value, _oldValue and InitialValue will all be default.
It is required in OnEnable, to make sure that the (hidden) _value field is correctly InitialValue for the start of the game. Also _oldValue has to be the same here because for the initial events that are triggered it make no sense to have _oldValue at a value of the previous playmode session.

while HandlePlayModeStateChange will also call instance.SetInitialValues() it might not be necessary to have that call in OnEnable ... #if UNITY_EDITOR but within a build, I think it has to be in OnEnable ...

It's also required to ensure that the callback for EnterPlayMode is registered, but that could be handled using many other systems

If it is called in Awake, it will not register the EnterPlayMode callbacks frequent enough, that part HAS to be in OnEnable

I also fixed the instancer bug: lgarczyn@267eaeb

🤔 I guess this is okay. At least i can't think of an case where this would break something.

(but for real, imho the Instancers require a rewrite (#370)

from unity-atoms.

lgarczyn avatar lgarczyn commented on June 20, 2024

SetInitialValues in Awake has no effect, since it is only called at asset creation, _value, _oldValue and InitialValue will all be default.
It is required in OnEnable, to make sure that the (hidden) _value field is correctly InitialValue for the start of the game. Also _oldValue has to be the same here because for the initial events that are triggered it make no sense to have _oldValue at a value of the previous playmode session.

I can think of two effects:

  • Copy-pasting an atom
  • Deserializing an atom from JSON

In both those case, awake would not be called with the default value.

from unity-atoms.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.