Repository navigation
A NaN constructor option is the option left out; CabinetSim keeps an echoed macro - #247
Merged
Merged
Conversation
…im keeps a macro echoed back from get_macro (#145)
…ant CtorNan before the base cleans
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #145.
A NaN passed for a numeric constructor option reached
_buildas it was, so 32 effect classes rendered something that was neither the default nor zero. The baseComponentconstructor now drops a NaN option (keyword, or positional where the interpreter can name the arguments) before_buildruns, so the option takes its default as each class's documentation says. One rule covers every class.Four options keep the NaN meaning their class already documents and tests: Glide on DigitalDelay and TapeDelay (the jump), damping on ConvolutionReverb (out of circuit) and interval on ShimmerHall (refused). A class names such an option in
_NAN_OWN.CabinetSim.set_macrosnaps a float to the nearest of 128 positions on purpose. A constructor option off the grid is not on that grid, so echoingget_macro()back moved it (46.99 read back as 47). The numberget_macro()itself returned is now kept; any other float still snaps.SlapbackDelayalready echoes exactly on current main, and the new test holds it there.Tests:
tests/test_nan_options.pyrenders every effect with each numeric option set to NaN against the option left out, checks the get/set round trip and the audio on both classes, and checks that a chosen float still snaps.