Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setter for Value property in FloatReference #12

Open
Brandon-Gui123 opened this issue Mar 7, 2020 · 3 comments
Open

Setter for Value property in FloatReference #12

Brandon-Gui123 opened this issue Mar 7, 2020 · 3 comments

Comments

@Brandon-Gui123
Copy link

Brandon-Gui123 commented Mar 7, 2020

Hello Ryan! I'm probably very late but amazing sharing; it completely changed the way on how I worked with Unity and this is so beneficial. Thank you!

I am thinking of adding a set to the Value property so that when other scripts assign a value, the Value property can receive it, then modify, based on whether UseConstant is true or false, ConstantValue or Variable.Value.

The property will look like this:

public float Value
{
    get { return UseConstant ? ConstantValue : Variable.Value; }
    set
    {
        if (UseConstant)
            ConstantValue = value;
        else
            Variable.Value = value;
    }
}

And in other scripts, one can simply do something like this:

public FloatReference hp;
.
.
.
hp.Value = 10;

So far, the only implication I have seen was that setting via a FloatReference, you may be expecting a variable to get updated, but if UseConstant is true, the constant is updated and not the variable. So, for example, if you decrement the player's HP and the HP is of type FloatReference with UseConstant set to true, then this may not update the UI for the HP (e.g. say a HP bar) which depends on a FloatVariable to determine the fill amount, making others think that the player's HP is not decreasing.

But then if such a case happens, FloatVariable should be used to adjust HP instead of FloatReference, which will make it clear to the team that modifications made to it will be clearly shown.

@Brandon-Gui123
Copy link
Author

On second thought, maybe instead we don't allow Variable.Value to get modified via the Value property, forcing users to modify the value via a FloatVariable.
This will allow other scripts that use the FloatVariable to see the changes.

The property can be changed to this:

public float Value
{
    get { return UseConstant ? ConstantValue : Variable.Value; }
    set
    {
        if (!UseConstant)
            // maybe Debug.LogError here to warn devs?
        else
            ConstantValue = value;
    }
}

@baratgabor
Copy link

Sorry, I'm not the maintainer; but I got a notification, and it occurred to me to mention that I also extended the solution in this repo with differentiated read-only and writeable classes, and a lot of other features. :) If you want, you can look into it as a possible implementation.

For example this is a writeable float property, which extends the basic concrete read-only class, which is further derived from a generic abstract base class GameProperty<T> that exposes a protected setter method which takes care of propagating the changed value to subscribers:

https://github.com/baratgabor/Unity3D-ReactiveScriptables/blob/master/ScriptableObjects/Properties/FloatProperty_Writeable.cs

I also ended up differentiating 'properties' and 'events', because in my use I realized that a lot of times all I want is to push a notification to a component with a certain payload, without having an initial value set. But possibly there were other reasons too; I can't quite remember because it was ages ago. :)

@Brandon-Gui123
Copy link
Author

@baratgabor I see. I do think that creating separate read-only and writeable classes is a good solution because you have a choice on whether modifications are allowed. Thanks for showing me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants