Skip to content

Fix: ThrottleFirstLastFrame emits default(T) after a trailing value was published - #389

Open
gajk1034 wants to merge 1 commit into
Cysharp:mainfrom
gajk1034:fix/throttle-first-last-frame-hasvalue
Open

Fix: ThrottleFirstLastFrame emits default(T) after a trailing value was published#389
gajk1034 wants to merge 1 commit into
Cysharp:mainfrom
gajk1034:fix/throttle-first-last-frame-hasvalue

Conversation

@gajk1034

Copy link
Copy Markdown

_ThrottleFirstLastFrame.MoveNext clears lastValue on window close but does not reset hasValue, unlike the TimeSpan-based _ThrottleFirstLast variants which reset both. Once a window has published a trailing value, hasValue stays true forever, so every subsequent window that received only one value re-emits the already-cleared lastValue — i.e. default(T) — when it closes.

  var frameProvider = new FakeFrameProvider();
  var subject = new Subject<int>();
  var results = new List<int>();

  subject.ThrottleFirstLastFrame(5, frameProvider).Subscribe(results.Add);

  subject.OnNext(1);          // opens window, emits 1                                                                                                                           
  subject.OnNext(2);          // stored as trailing value                                                                                                                        
  frameProvider.Advance(5);   // window closes, emits 2 (OK)                                                                                                                     

  subject.OnNext(3);          // opens a new window, emits 3                                                                                                                     
  frameProvider.Advance(5);   // window closes, emits 0 = default(int)                                                                                                           

  // results: [1, 2, 3, 0] — expected [1, 2, 3] 

This PR resets hasValue on window close, matching the TimeSpan-based implementation, and adds a regression test.

The added test fails on current main with [1, 2, 3, 0] (an extra default(int) emission) and passes with this fix.

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

Successfully merging this pull request may close these issues.

1 participant