GrandOrgue / grandorgue

GrandOrgue software
Other
166 stars 40 forks source link

Divisional Setter Error for Tremulant >009 #1773

Closed hnb2907 closed 8 months ago

hnb2907 commented 8 months ago

Hi,

Thanks for the work to implement https://github.com/GrandOrgue/grandorgue/pull/1757 on v3.13.3-1.

I have been able to add 2 more tremulants (total now 12 in my organ), and they are working as expected. Previously, "[Tremulant010]" was defined, but not included in the divisional piston setter.

Next I added the tremulants 010/011/012 to the Divisional piston setter, and it possibly has a bug. This error is thrown while loading the organ: "Out of range value at section 'Divisional101' entry 'Tremulant010':-10"

After a bit of investigation, it is complaining about the -010 which relates to the following lines in my config: Tremulant010=-010 It is accepting the "Tremulant010=" but not the -10 For example changing it to Tremulant010=-009 is ok, the error is not thrown.
Similar behaviour for "Tremulant011=" and "Tremulant012="

Here is a cut down part of my organ, hopefully that's enough information for it to be investigated. I had a quick look through the GO code, it looks ok to me, so I don't understand why only the divisional piston config reader is limited to <10.

FYI, I also added the new tremulants to the General piston setters, no errors, and it is working correctly.

[Organ]
NumberOfTremulants=12

[Tremulant001]
Name=Main
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant002]
Name=Solo
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant003]
Name=Tuba Diapason
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant004]
Name=Tibia Clausa
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant005]
Name=Vox Humana
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant006]
Name=Clarinet
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant007]
Name=Traps Reit
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant008]
Name=Perc Reit 
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant009]
Name=Vibra- phone 
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
;TremulantType=Wave
TremulantType=Synth
Period=240
StartRate=20
StopRate=20
AmpModDepth=55

[Tremulant010]
Name=Piano Sustain 
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
TremulantType=Wave

[Tremulant011]
Name=Horn
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
;TremulantType=Wave
TremulantType=Synth
Period=230
StartRate=8
StopRate=5
AmpModDepth=18

[Tremulant012]
Name=Sax
DefaultToEngaged=N
DisplayInInvertedState=N
ShortcutKey=0
Displayed=N
;TremulantType=Wave
TremulantType=Synth
Period=130
;Period=230
StartRate=8
StopRate=5
AmpModDepth=38

[Divisional101]
; Accomp 1
Name=1
ShortcutKey=0
NumberOfStops=33
Stop001=-001
Stop002=-002
Stop003=-003
Stop004=-004
Stop005=-005
Stop006=-006
Stop007=-007
Stop008=-008
Stop009=-009
Stop010=-010
Stop011=-011
Stop012=-012
Stop013=-013
Stop014=-014
Stop015=-015
Stop016=-016
Stop017=-017
Stop018=-018
Stop019=-019
Stop020=-020
Stop021=-021
Stop022=-022
Stop023=-023
Stop024=-024
Stop025=-025
Stop026=-026
Stop027=-027
Stop028=-028
Stop029=-029
Stop030=-030
Stop031=-031
Stop032=-032
Stop033=-033
NumberOfSwitches=0
NumberOfCouplers=2
Coupler001=-001
Coupler002=-002
NumberOfTremulants=12
Tremulant001=-001
Tremulant002=-002
Tremulant003=-003
Tremulant004=-004
Tremulant005=-005
Tremulant006=-006
Tremulant007=-007
Tremulant008=-008
Tremulant009=-009
Tremulant010=-010
Tremulant011=-011
Tremulant012=-012
Displayed=N

Many thanks, Chris.

larspalo commented 8 months ago

@hnb2907 Quick question: Did you also add the extra tremulants to the [Manual999] that carries the divisional? That is, you're sure that the NumberOfTremulants=12 exist in the relevant manual section and the Tremulant011 and Tremulant012 are there too?

Reason is that for a general all elements will be ok to just add to the combination, but for a (manual) divisional the "global" tremulants must be listed as available under the manual to be possible to use in a divisional.

larspalo commented 8 months ago

@hnb2907 I've just tested adding 12 tremulants to an organ, including all in a manual correctly referenced. I could use the setter panel divisionals just fine when the tremulants was defined under the manual too.

It's still a bug that GO allows storing a "global" tremulant in a divisional when it's not listed under the manual (at least without using the Full option in such cases).

hnb2907 commented 8 months ago

Hi @larspalo,

Thankyou again for the quick reply. You are correct, I hadn't added Tremulants 010/011/012 in the [Manual999] section; I hadn't realised they also feature here, and how the configurations are different between General/Divisional.

It also explains that I didn't understand how the GO source code derived the maximum allowed value. I was incorrectly expecting it to use [Organ] NumberOfTremulants=nnn as the maximum.

It is now working, and I have closed the issue :)

Many thanks, Chris.