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

param remove #887

Closed
wants to merge 1 commit into from
Closed

param remove #887

wants to merge 1 commit into from

Conversation

@pq
Copy link
Collaborator

@pq pq commented Oct 4, 2019

this almost works but inexplicably falls over for some but not all param ids...

params:print()
paramset [arcadia]
1 enc3 = timbre
2 shape = 0.0 
3 timbre = 0.5 
4 sub = 0.0 
5 noise = 0.0 
6 stereo width = 0.5 
7 pitch lag = 0.0 
8 filter gain = 0.0 
9 cut = 8.0 
10 cut env amt = 0.0 
11 cut attack = 0.05 
12 cut sustain = 0.9 
13 cut release = 1.0 
14 level = 0.15 
15 amp attack = 0.05 
16 amp decay = 0.1 
17 amp sustain = 0.9 
18 amp release = 1.0 
19 unnamed = ---
20 output = crow ii JF
<ok>
params:remove('enc3')
lua: /home/we/norns/lua/core/paramset.lua:219: invalid paramset index: enc3
stack traceback:
	[C]: in function 'error'
	/home/we/norns/lua/core/paramset.lua:219: in function 'core/paramset.lookup_param'
	/home/we/norns/lua/core/paramset.lua:142: in function 'core/paramset.remove'
	(...tail calls...)

😬

but then:

params:remove('level')
<ok>

or maybe i'm going about this in entirely the wrong way?

/cc @tehn

@tehn
Copy link
Member

@tehn tehn commented Oct 5, 2019

this seems reasonable, but i have to stare at the existing params code a bit to remember how it all works ;)

@tehn
Copy link
Member

@tehn tehn commented Oct 6, 2019

the issue is that table.remove decrements all the indexes above the removal, so the reverse lookup table params.lookup basically gets trashed.

so if you remove something, then remove something later in the list it will hit the wrong param.

so the removal needs to do a sort of rebuild of the lookup table

@pq
Copy link
Collaborator Author

@pq pq commented Oct 6, 2019

ah; that makes sense. thanks for taking a look!

@pq
Copy link
Collaborator Author

@pq pq commented Oct 7, 2019

after thinking about this some more, i'm not convinced remove(id) is really the desired API and really it would be better to just clear and re-add.

@pq pq closed this Oct 7, 2019
@tehn tehn deleted the param_remove branch Dec 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants