SetValue action but with Variable's InitialValue about unity-atoms HOT 14 CLOSED

Ok, let's summarize things:

1. We all agree that it should be added ResetValue() method on Variable.
2. It's reasonable to add non-generic class which will be inherited by Variables to provide that functionality above and other.
3. It's also good to add parametless Events (to provide i.e. a mechanism to store different Events without arguments in a list).
   That's why I asked in other issue #51 will it be added. I don't have an idea to do it in other way (resetting Variables not by util MonoBehaviour), many well-paid assets do it in that way providing a "AssetName"Manager living in a scene. I think if there will be another flexible, good solution they would do it.
   So my opinion, I think that should be added (what is in the points above) and then we could think about other solutions (which I think it would be difficult to find)
   Edit: Read the other issue, no problem @adam I just make a game with your framework that's why I'm following your repo and for me it's also good to look how it's developed. If I will understand the project and code structure in the future I would also try to pull requests.

from unity-atoms.

My biggest issue though with having a reset variable MonoScript util is that resetting variables / data should not be something belonging to a util script inside a scene, but should rather be something that should be triggered outside of a scene / during the loading of a scene.

Its sadly the way unity works. And when using the ECS this gets promoted even more.

Since the resetting logic is scene dependent, there needs to be scene specific information. those information belong into the scene.

yes: there is an overhead resulting from the transform component. but we can't do anything about it. that's unity's architecture.

from unity-atoms.

Make some GameAction where we can store a List of Variables to reset to initialValue and a method with doing that job. We can use a Hook OnStart, Void Listener and use this GameAction on callback.

currently available workaround:

you can do this by yourself. Have a "Setup" script in your level, which runs on level load / level reset / whatever. And in this setup method iterate through a list of ScriptableObjectVariable and call on everyone: OnAfterDeserialize(). it will reset the currentValue to the initialValue.

@AdamRamberg Its probably a good Idea to add a public void Reset() method (or ResetValue()) to ScriptableObjectVariable which resets the value to the initialValue. While it would actually do the same as AfterDeserialize it would be semantically better to have this redundancy

I don't think it would be wise to create some kind of event on our site, because this can be very game dependent.

note to myself: some more investigation when scriptable variables are resetted (from AfterDeserialize) and how this differs between editor and builds

from unity-atoms.

currently available workaround:

you can do this by yourself. Have a "Setup" script in your level, which runs on level load / level reset / whatever. And in this setup method iterate through a list of ScriptableObjectVariable and call on everyone: OnAfterDeserialize(). it will reset the currentValue to the initialValue.

I checked that there is no non-generic base class :/ How can I store ScriptableObjectVariable in a List?
It's not possible to do List<ScriptableObjectVariable < T > >, so I can't store IntVariable/FloatVariable/GameObjectVariable in one list. I would have to use one list for every type of [sth]Variable, right? I can make IntVariableResetter.cs, FloatVariableResetter and etc. Uhh, it's a solution, but a dirty one.
What do you think?

from unity-atoms.

Its just a workaround! its not meant as permanent solution.

you could use a List<ISerializationCallbackReceiver> (that makes no sense semantically and could totally be abused, but it should work - if used correctly - as intended)

edit: argh ... not sure if you can feed this list in the inspector, but a List<ScriptableObject> and in the loop where you try to reset, you can cast to ISerializationCallbackReceiver

somewhere like that:

public List<ScriptableObject> variablesToReset = new List<ScriptableObject>();

void Setup(){
    foreach (var variable in variablesToReset)
    {
        var _resettable = variable as ISerializationCallbackReceiver;
        if(_resettable == null) continue;
        _resettable.OnAfterDeserialize();
    }
}

from unity-atoms.

For now, works as expected. Great. Hope to see not-workaround ;)
Because I think it's a common problem to reset Variables in a secure way (I mean not using SetVariable with value field, but with InitialValue) when a level starts.

from unity-atoms.

Great idea @IceTrooper ! FYI: MonoHooks are actually going away in the nest release of Unity Atoms so that solution won't suffice.

@soraphis a public ´Reset()´ or ´ResetValue()´ function would work great.

In addition to the above, I have also been thinking about creating some static manager class for Unity Atoms. I know what you are thinking, static classes are bad and all that, but I actually think that if used "internally" in the library it won't be that bad. This manager will be created with DontDestroyOnLoad and will give Atoms (that registers for it) triggers on Unity callbacks (like on Start / Awake / Scene Start for example). In this case with could just add a "Reset on Scene Start" checkbox on the Variable. Thoughts?

from unity-atoms.

I think IceTrooper showed a real problem here, that we have no non-generic base class.

we should also take a look in similar projects. mainly: SmartData, ScriptableObject-Architecture and this asset

how would Atoms register on the manager? if they need to be added manually (in the inspector) to a list, there is no real benefit.
otherwise, how would the manager know which variables to reset?

wouldn't it be easier to just provide a utility MonoBehaviour, that takes a list of variables and resets them on Awake(), so that everyone who needs this functionality would just use it, and everyone who does not need is, does not have a unused singleton (or singleton-like).

from unity-atoms.

Hi guys
@AdamRamberg Thank you for saying that MonoHooks are going away. In that project, I haven't used it except for that case we are talking on this issue topic (resetting OnStart).
I think ResetValue() would be great.

In addition to the above, I have also been thinking about creating some static manager class for Unity Atoms. I know what you are thinking, static classes are bad and all that, but I actually think that if used "internally" in the library it won't be that bad. This manager will be created with DontDestroyOnLoad and will give Atoms (that registers for it) triggers on Unity callbacks (like on Start / Awake / Scene Start for example). In this case with could just add a "Reset on Scene Start" checkbox on the Variable. Thoughts?

I don't think it's a good idea (or I just don't understand the real benefit). As @soraphis said, I also prefer to give more flexibility with MonoBehaviour we could create on our own or just use that 'utility' to reset variables stored in a list.

And one more reason I'm writing this comment:
Do you think it would be good idea to provide some mechanism/interface to listen to an event, but no matter of the type of that event. So we could store in a variable something like GameEvent (without < T >) and it could listen any of GameEvent < T >. It doesn't know the passed parameter, only just receives that event is raised. I don't know it's a good or bad idea, but like in Variable< T >, it requires that GameEvent < T > will inherit GameEvent class.
BTW: I know we got VoidEvent, but it receives only VoidEvent, not something more general like GameEvent we could store VoidEvent/IntEvent/FloatEvent in and still don't care about passed parameter with that event.
Just an idea of the base non-generic class (or interface) for GameEvent too. We could make a IGameEvent and IGameEvent < T > both to provide functionality like a List < T > implements IList and IList < T > both (I think a List < T > is a good example how to do it in your framework with GameEvent < T > EDIT: Hmm, I don't know if that solution to compare it to List is good...).

For example, I got a Generator which creates new points every time PointCollected(float valueForPoint) raises, but as you could imagine I don't care about valueForPoint in a generator. I just want to know that it raised. In addition, I want to give my designer flexibility to change an event Generator is registered to another event (still it doesn't care about what type, what parameter it got) like BonusCollected(GameObject bonus) or LifeLost(). I don't have that possibility, because GameEvent is generic.

from unity-atoms.

Great, we all agree to add a ResetValue method!

how would Atoms register on the manager? if they need to be added manually (in the inspector) to a list, there is no real benefit.
otherwise, how would the manager know which variables to reset?

wouldn't it be easier to just provide a utility MonoBehaviour, that takes a list of variables and resets them on Awake(), so that everyone who needs this functionality would just use it, and everyone who does not need is, does not have a unused singleton (or singleton-like).

The benefit is that we want to reset data and to me it feels unintuitive / clumsy to add a GameObject with a MonoBehaviour to a scene and then drag in the Variables just for resetting a Variable on level start. This could also be really tedious and error prone, especially if you want to do this to a bunch of levels.

On a second thought, it actually might be overkill to create singletons for this when the SceneManager in Unity got SceneManager.sceneLoaded . Created a new branch with the (in my mind) simplest solution to this specific problem that @IceTrooper has. Take a look here. @soraphis @IceTrooper Thoughts? Could merge to canary if you think it looks good.

However, adding some kind of singleton to hook up atoms to Unity's lifecycle methods is possible and I think it could be a nice thing to add further down the road. This doesn't necessarily has to bloat the core functionality if we start to use partial classes and add the functionality under a new package which defines some part of an atom. The basic concept would look like this:

using System;
using System.Collections.Generic;
using UnityEngine;
using UnityEngine.Serialization;

namespace UnityAtoms
{
    public class UpdateNotifier : MonoBehaviour
    {
        public static UpdateNotifier Instance
        {
            get
            {
                if (_instance == null)
                {
                    var gameObject = new GameObject();
                    gameObject.name = "UpdateNotifier";
                    gameObject.AddComponent<UpdateNotifier>();
                    _instance = gameObject.GetComponent<UpdateNotifier>();
                }
                return _instance;
            }
        }
        private static UpdateNotifier _instance = null;
        private List<Action> _registeredAtoms = new List<Action>();

        public UpdateNotifier()
        {
            _registeredAtoms.Clear();
        }

        private void Update()
        {
            _registeredAtoms.ForEach((action) =>
            {
                action();
            });
        }

        public void Register(Action action)
        {
            _registeredAtoms.Add(action);
        }
    }

    public abstract partial class ScriptableObjectVariable<T, E1, E2> : ScriptableObjectVariableBase<T>,
        ISerializationCallbackReceiver, IVariableIcon
        where E1 : GameEvent<T>
        where E2 : GameEvent<T, T>
    {
        [SerializeField]
        private bool _doStuffOnUpdate;

        static partial void DoStuffOnUpdate(ScriptableObjectVariable<T, E1, E2> self)
        {
            if (self._doStuffOnUpdate)
            {
                UpdateNotifier.Instance.Register(() =>
                {
                    // DO STUFF
                });
            }
        }
    }
}

The "main" implementation of ScriptableObjectVariable would need to contain just a definition of DoStuffOnUpdate like this:
static partial void DoStuffOnUpdate(ScriptableObjectVariable<T, E1, E2> self)
and then call that method on Awake.

Just something to think about if there is a use case further down the road.

I don't think it's a good idea (or I just don't understand the real benefit). As @soraphis said, I also prefer to give more flexibility with MonoBehaviour we could create on our own or just use that 'utility' to reset variables stored in a list.

See my answer to @soraphis above.

Do you think it would be good idea to provide some mechanism/interface to listen to an event, but no matter of the type of that event.

I could see that it being useful to add an interface to the GameEvent class. Created an issue for it (#51 ).

I think IceTrooper showed a real problem here, that we have no non-generic base class.

Is it important to derive from a base class (to be able to be serializable by Unity) or would it suffice with creating interfaces like @IceTrooper is mentioning above? What use cases do you see?

from unity-atoms.

The benefit is that we want to reset data and to me it feels unintuitive / clumsy to add a GameObject with a MonoBehaviour to a scene and then drag in the Variables just for resetting a Variable on level start. This could also be really tedious and error prone, especially if you want to do this to a bunch of levels.

you would probably have a setup gameobject with a few setup components in your game.

Take a look here. @soraphis @IceTrooper Thoughts? Could merge to canary if you think it looks good.

no you cannot subscribe to the scene manager in the scriptable object and just reset for gods sake.
what if you additive load levels part by part?

the problem is that: when and which variables needs to be resetted is highly depending on the game you make. what if you have scenes where a variable needs to be resetted and one where not?

what if you reset the variable on a "reset level" action that is custom made and does not use the scenemanager?

this is game logic and not library logic. we should provide the functionality but not the logic.

Is it important to derive from a base class (to be able to be serializable by Unity) or would it suffice with creating interfaces like @IceTrooper is mentioning above? What use cases do you see?

depends. if you want a list that you can fill in the inspector it has to be a base class and cannot be an interface

However, adding some kind of singleton to hook up atoms to Unity's lifecycle methods is possible and I think it could be a nice thing to add further down the road.

For most people, this ScriptableVariable-Architecture is a tool to get rid of the Singleton chaos, that most projects had a few years ago. I don't think we should add our own.

I don't see the benefit of DoStuffOnUpdate. why would one use it? why is it static? what goal does it archive, which can't be archived right now?

I could see that it being useful to add an interface to the GameEvent class. Created an issue for it (#51 ).

will look into this and answer in that issue.

from unity-atoms.

@AdamRamberg I don't like the idea with SceneManager. As @soraphis said, it shouldn't be tied with a Variable and scene because of that sometimes we want to reset value, sometimes we don't want to. The solution should just provide a mechanism to reset.

from unity-atoms.

no you cannot subscribe to the scene manager in the scriptable object and just reset for gods sake.

This is seriously not an ok comment and provides no value to the discussion. Comments like this will most likely keep others away from contributing to the project. Please think of this going forward.

what if you additive load levels part by part?

the problem is that: when and which variables needs to be resetted is highly depending on the game you make. what if you have scenes where a variable needs to be resetted and one where not?

what if you reset the variable on a "reset level" action that is custom made and does not use the scenemanager?

Didn't know of additive loading levels. Good point. The "custom reset level action" is a good argument too against this.

My biggest issue though with having a reset variable MonoScript util is that resetting variables / data should not be something belonging to a util script inside a scene, but should rather be something that should be triggered outside of a scene / during the loading of a scene. It's however pretty common practice in Unity that there are util scripts inside scenes that does similar things like this, but why should there be a GameObject in a scene containing that functionality (the GameObject only acts as a container, but still has for example a Transform and is in the scene)? We could add a utility class though, in order to provide a solution right now, but think that it would be nice to provide some other alternative further down the road.

As @soraphis said, it shouldn't be tied with a Variable and scene because of that sometimes we want to reset value, sometimes we don't want to. The solution should just provide a mechanism to reset.

Just to explain my thinking. I was thinking that if this was the case that we could have added a GameFunction to the Variable, containing logic if the Variable should be reseted or not.

from unity-atoms.

This should have be solved in 778acce (added ResetValue method) and in e732dad (added none generic base classes to all atoms).

Also an example of ResetValue was added in 8c8bc5f.

from unity-atoms.