Cysharp / R3

The new future of dotnet/reactive and UniRx.
MIT License
1.72k stars 70 forks source link

OnNext is not called for new subscriptions after this. #163

Closed kimk-s closed 4 months ago

kimk-s commented 4 months ago

Thank you so much for your library.

I installed R3 on the new Unity project and wrote the four scripts below.

I created the scene hierarchy as shown below and set references to each other in the inspector. (BarItem is not activeSelf false on scene.)

Untitled

I wrote the four scripts below.

// Script 1
public class PropertyHolder : MonoBehaviour
{
    public ReactiveProperty<int> property;

    void Awake()
    {
        property = new ReactiveProperty<int>().AddTo(this);

        property
            .Skip(1)
            .Subscribe(x => Debug.Log($"[PropertyHelper] {x}"));
    }
}

// Script 2
public class Foo : MonoBehaviour
{
    public PropertyHolder propertyHolder;
    public Bar bar;

    void Start()
    {
        propertyHolder.property
            .Skip(1)
            .Subscribe(x => Debug.Log($"[Foo] {x}"))
            .AddTo(this);

        Observable.Return(Unit.Default)
            .Delay(TimeSpan.FromSeconds(1))
            .Take(1)
            .Subscribe(_ =>
            {
                bar.gameObject.SetActive(true);
                Destroy(this.gameObject);
            });
    }
}

// Script 3
public class Bar : MonoBehaviour
{
    public BarItem item;

    void Start()
    {
        item.gameObject.SetActive(true);
    }
}

// Script 4
public class BarItem : MonoBehaviour
{
    public PropertyHolder propertyHolder;

    void Start()
    {
        propertyHolder.property
            .Skip(1)
            .Subscribe(x => Debug.Log($"[BarItem] {x}"))
            .AddTo(this);

        propertyHolder.property.Value = 1;
        propertyHolder.property.Value = 2;
    }
}

And I pressed the Play button in UnityEditor.

I expected log output like the following.

[PropertyHelper] 1
[Foo] 1
[BarItem] 1
[PropertyHelper] 2
[Foo] 2
[BarItem] 2

In reality, the log below was output.

[PropertyHelper] 1
[PropertyHelper] 2

After this, I no longer received OnNext when I created subscriptions to PropertyHolder.property(ReactiveProperty).

When a BarItem is activeSelf in the scene, the expected log is output.

Is there something I'm doing wrong?

I used

Content added.

(BarItem's inspector)

Untitled2

I Replace namespace R3 with UniRx. And I got the log as below

[PropertyHelper] 1
[BarItem] 1
[PropertyHelper] 2
[BarItem] 2
neuecc commented 4 months ago

Since it is being destroyed, [Foo]... will probably not be called (it is being canceled by AddTo(this)). [BarItem] might be called, but...?

kimk-s commented 4 months ago

Thank you for taking the time to review.

I understand that [Foo] is not working. I would like to ask about [Bar] and the situation after this.

So, could the [property] variable (ReactiveProperty<>) in [PropertyHolder] be broken after this?

After this, I additionally subscribed to the [property] variable of [PropertyHolder] and set the value, but OnNext could not be received for only additionally subscription. OnNext occurs for subscriptions that already existed before this (in [PropertyHolder]'s Awake() method).

For example, if I additionally wrote the [Baz] class script below and placed it as active in the scene:

public class Baz : MonoBehaviour
{
    public PropertyHolder propertyHolder;

    void Start()
    {
        Observable.Return(Unit.Default)
            .Delay(TimeSpan.FromSeconds(3))
            .Take(1)
            .Subscribe(_ =>
            {
                propertyHolder.property.Skip(1).Subscribe(x => Debug.Log($"[Baz] {x}"));
                propertyHolder.property.Value = 3;
            });
    }
}

When this part of [Baz]'s Start() method is executed after a 3-second delay, I expected the following log output.

[PropertyHelper] 3
[Baz] 3

In reality it was like this

[PropertyHelper] 3

If I use UniRx instead of R3, the log output is as expected.

The full log of the program after adding [Baz] is as follows. (One line is one log)

With R3 ( [BarItem]'s activeSelf is false in scene. )

[PropertyHelper] 1
[PropertyHelper] 2
[PropertyHelper] 3

With R3 ( [BarItem]'s activeSelf is true in scene. )

[PropertyHelper] 1
[Foo] 1
[BarItem] 1
[PropertyHelper] 2
[Foo] 2
[BarItem] 2
[PropertyHelper] 3
[BarItem] 3
[Baz] 3

With UniRx

[PropertyHelper] 1
[BarItem] 1
[PropertyHelper] 2
[BarItem] 2
[PropertyHelper] 3
[BarItem] 3
[Baz] 3

(I'm migrating from UniRx to R3) (Sorry for the typo. [PropertyHolder] and [PropertyHelper] refer to the same thing.) (Sorry for my poor English.)

neuecc commented 4 months ago

My environment in R3 shows correct result, so can't reproduce your issue.

image

kimk-s commented 4 months ago

In my case, I newly installed the latest Unity 2023.2.13f1, created a new project, installed R3, and ran it. It keeps happening on my laptop.

I uploaded my project to GitHub. If you don't mind, I would appreciate it if you could check the status of the project.

image

https://github.com/kimk-s/R3TestUnity

neuecc commented 4 months ago

I seem to have created a bug in a recent update. We have reproduced it in a simple console application. It appears that the internal observer management is corrupted. I will fix it immediately! Thank you!

var p1 = new ReactiveProperty<int>();
p1.Skip(1).Subscribe(x => Debug.Log("[P1]" + x));

var d = p1.Skip(1).Subscribe(x => Debug.Log("[P2]" + x));

d.Dispose();

p1.Skip(1).Subscribe(x => Debug.Log("[P3]" + x)); // P3 is not raised

p1.Value = 1;
p1.Value = 2;

public static class Debug
{
    public static void Log(string x)
    {
        Console.WriteLine(x);
    }
}
neuecc commented 4 months ago

I've released fixed release on 1.1.8. Thanks to your persistent research, I was able to realize this. I really appreciate it.

kimk-s commented 4 months ago

Thank you very much!!!