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

Poll: fix initialization to fire on first-sample trigger #5965

Merged
merged 1 commit into from
Feb 9, 2023

Conversation

mtmccrea
Copy link
Member

@mtmccrea mtmccrea commented Feb 9, 2023

Purpose and Motivation

Poll misses a trigger on the first sample... but not anymore!

This bug was revealed only with the updated behavior of Impulse (as of 3.13). There was no issue filed, but addresses a discussion that uncovered the bug.

Types of changes

  • Bug fix

To-do list

  • Code is tested
  • All tests are passing
  • [N/A] Updated documentation
  • This PR is ready for review

@dyfer
Copy link
Member

dyfer commented Feb 9, 2023

Thanks! This seems to work as expected:

(
{
	var trig = Impulse.kr(1);
	Sweep.kr(1).poll(trig);
	FreeSelf.kr(PulseCount.kr(trig) >= 3);
	Silent.ar(1)
}.play;
)

results in

-> Synth('temp__1' : 1001)
UGen(Sweep): 0.00133333
UGen(Sweep): 1.00267
UGen(Sweep): 2.00267

While this looks fine to me, it does not produce the double posting at the beginning of the synth that you showed here. I just wanted to double check whether the behavior in this PR is exactly what you'd expect.

@dyfer dyfer added this to the 3.13.0 milestone Feb 9, 2023
@mtmccrea
Copy link
Member Author

mtmccrea commented Feb 9, 2023

it does not produce the double posting

Yes, that's the correct behavior. The double posting from before was just the quick "patch" for the previous thread, to make sure the issue wasn't with Impulse.

Thanks for your attention to detail :)

Copy link
Member

@dyfer dyfer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@dyfer dyfer merged commit 4acbee7 into supercollider:3.13 Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants