Cysharp / R3

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

Zip operator functionality issue #162

Open gSpidy opened 4 months ago

gSpidy commented 4 months ago

It seems like Zip operator not raising onComplete until all its underlying observables are completed This behaviour is greatly differs from previous rx implementations

For example: Zip observable array with N elements and infinite timer. In current implementation zip will never raise onCompleted (but it should've when timer fires N+1st event, according to rxmarbles)

Is this behaviour intentional in R3's implementation?

neuecc commented 4 months ago

It seems that the Zip operator in RxNet behaves in such a way that it completes when OnNext is emitted if all other observables except itself are in the OnCompleted state. For example, if two infinite timers are included in the Zip, it is unlikely to complete. That in itself seems like a strange behavior...

gSpidy commented 4 months ago

Thanks for your answer!

I was just wondering if it's intentional behaviour in new implementation because i noticed that some of my observables using zip were broken after migrating from UniRx

something like that: Screenshot 2024-03-08 181019 now it will never call oncompleted (but previously (in unirx) oncomplete was called when the source observable ends and the number of timer ticks matched the number of elements)

If it is intentional new behaviour, that's ok. Maybe mention this changes in docs?

neuecc commented 4 months ago

I think it's good to complete Zip somewhere, as it will never move and will only continue to accumulate in the Queue forever. I think it's particularly beneficial in the case of the left-right pattern, so I'll revert some of the behavior (in your pattern, it will behave the same as UniRx). Waiting for OnCompleted in Nth-Zip doesn't seem to be very appropriate behavior for UniRx either, so I may make changes. I will report and implement after a little more thought on the behavior. Thank you!