neuecc / UniRx

Reactive Extensions for Unity
MIT License
7.08k stars 891 forks source link

Model-View-(Reactive)Presenter Pattern Example need update #496

Open wodndb opened 3 years ago

wodndb commented 3 years ago

Hi,

I want to study about MV(R)P Pattern based UniRx. So I test MV(R)P Example in readme.

ToReactiveProperty method return IReadOnlyReactiveProperty<T>. However IsDead property in Enemy class is ReactiveProperty<T>. So source code in example is not working.

I change a type of IsDead to IReadOnlyReactiveProperty<bool> and the code works well without error.

Modified source code.

using UniRx;
using UnityEngine;
using UnityEngine.UI;

// Presenter for scene(canvas) root.
public class ReactivePresenter : MonoBehaviour
{
    // Presenter is aware of its View (binded in the inspector)
    public Button MyButton;
    public Toggle MyToggle;
    public Text MyText;

    Enemy enemy = new Enemy(1000);

    // Start is called before the first frame update
    void Start()
    {
        // Rx supplies user events from Views and Models in a reactive manner 
        MyButton.OnClickAsObservable().Subscribe(_ => enemy.CurrentHp.Value -= 99);
        MyToggle.OnValueChangedAsObservable().SubscribeToInteractable(MyButton);

        // Models notify Presenters via Rx, and Presenters update their views
        enemy.CurrentHp.SubscribeToText(MyText);
        enemy.IsDead.Where(isDead => isDead == true)
            .Subscribe(_ =>
            {
                MyToggle.interactable = MyButton.interactable = false;
            });
    }
}

// The Model. All property notify when their values change
public class Enemy
{
    public ReactiveProperty<long> CurrentHp { get; private set; }
    public IReadOnlyReactiveProperty<bool> IsDead { get; private set; }

    public Enemy(int initialHp)
    {
        // Declarative Property
        CurrentHp = new ReactiveProperty<long>(initialHp);
        IsDead = CurrentHp.Select(x => x <= 0).ToReactiveProperty();
    }
}

Actually I can't be sure that this source code is right...

Could anybody feedback this source code and update the example?

Thanks.

rus89 commented 3 years ago

Hi there,

I found some other example on github using .ToReactiveProperty() method over IntReactiveProperty field. The Rider IDE suggested 2 solutions:

  1. to change IntReactiveProperty to IReadOnlyReactiveProperty as you mentioned, and
  2. to safe cast entire statement with as ReactiveProperty<int>

And these 2 solutions make new 2 problems:

  1. there is a statement calling SetValueAndForceNotify(0) method over that IntReactiveProperty. And it's impossible to call it if I change IntReactiveProperty to IReadOnlyReactiveProperty
  2. safe casting is throwing a NullReferenceException: Object reference not set to an instance of an object when I start the game

I'm super confused 🤯😁

wodndb commented 2 years ago

I find example code (Examples/Sample12_ReactiveProperty.cs) in UniRx v7.1.0

In this example code, type of IsDead is IReadOnlyReactiveProperty<bool>.

I think that it's an answer for this issue.

Also Rider IDE 2021.2.1 suggested this. image