Closed nomakewan closed 1 year ago
I have added precompiled binaries that reflect the changes present in this PR per the discussion in #168 and also checked through the code for the Proton Pack and Attenuator more carefully, finding two more compiler errors that are now corrected.
As the base repository does not have precompiled binaries for the Attenuator and I do not know what version numbering scheme it should use, I have not included precompiled binaries for it. If they are desired I'd be more than happy to include them.
Good catches.
Just waiting for the diy neutrona wand code revamp request to be merged first, then will test this one out and merge after.
While messing around with my bench setup I found that the code for the Neutrona Wand was not using the correct sound files for each of the 5 power levels (Levels 1-2 used Level 1's sound, Levels 3-4 used Level 2's sound, and Level 5 used Level 5's sound). I've added that correction to this PR.
While messing around with my bench setup I found that the code for the Neutrona Wand was not using the correct sound files for each of the 5 power levels (Levels 1-2 used Level 1's sound, Levels 3-4 used Level 2's sound, and Level 5 used Level 5's sound). I've added that correction to this PR.
Is was actually done on purpose, I found the transition jarring between the different idle sounds. It is something I'm meaning to go back to one day and try to smooth things out.
While messing around with my bench setup I found that the code for the Neutrona Wand was not using the correct sound files for each of the 5 power levels (Levels 1-2 used Level 1's sound, Levels 3-4 used Level 2's sound, and Level 5 used Level 5's sound). I've added that correction to this PR.
Is was actually done on purpose, I found the transition jarring between the different idle sounds. It is something I'm meaning to go back to one day and try to smooth things out.
Got it. I've reverted the changes.
One more fix since I now have a working bench setup that I really wanted to iron things out with before getting hardware into the wand more permanently. I changed the playEffect(P_PACK_BOOTUP) call to playEffect(S_BOOTUP) since that is the correct enum to use (though the evaluated integer is identical, we should only use sound effect enums with playEffect). I also added a check to soundIdleStop() in 1984/1989 mode to prevent an overlapping shutdown sound effect if the user switches off the Activate switch while the lower toggle switch is turned on.
This should be the last one. I don't think I spotted anything else in my testing that caught my attention.
EDIT: Just kidding, found one more bug where a check specific to Cross The Streams Mix was instead being run on everything except Cross The Streams Mix.
Marking this as a draft as I believe I've stumbled onto a rather major bug relating to firing control, though at present it only seems to affect CTS Mix. It's going to take me a little bit to walk the code for each of the firing modes and button combinations to be certain. Apologies for the delay.
Marking this as a draft as I believe I've stumbled onto a rather major bug relating to firing control, though at present it only seems to affect CTS Mix. It's going to take me a little bit to walk the code for each of the firing modes and button combinations to be certain. Apologies for the delay.
What did you stumble upon?
What did you stumble upon?
CTS Mix seems to have some overlapping audio and doesn't seem to return to normal after releasing a trigger the way I would expect it to. When I checked the code, I think it's a sequencing issue that could be related to b_sound_firing_alt_trigger/b_sound_firing_intensify_trigger but I'm not positive just yet.
At present I'm walking the code for regular CTS mode. I've finished tracing CTS Intensify, and working on CTS Alt. Then I'll do CTS Both. That'll give me a baseline design so I can move on to taking a look at how CTS Mix works.
Preliminarily, it appears that the branch of code in modeFiring() right at the top which checks against b_sound_firing_intensify_trigger = true
won't ever actually get touched since that variable is set to true first through modeFireStart(), then again in modeFireStartSounds(), before being checked in modeFiring().
I was getting some weird overlap in my headphones when bench testing CTS Mix, which is why I started investigating. I could be way off but that's why I'm starting with the basics and working forward to be absolutely certain this is an actual issue and not just me misunderstanding the code flow and making a big deal out of a nothingburger.
What did you stumble upon?
CTS Mix seems to have some overlapping audio and doesn't seem to return to normal after releasing a trigger the way I would expect it to. When I checked the code, I think it's a sequencing issue that could be related to b_sound_firing_alt_trigger/b_sound_firing_intensify_trigger but I'm not positive just yet.
At present I'm walking the code for regular CTS mode. I've finished tracing CTS Intensify, and working on CTS Alt. Then I'll do CTS Both. That'll give me a baseline design so I can move on to taking a look at how CTS Mix works.
Preliminarily, it appears that the branch of code in modeFiring() right at the top which checks against
b_sound_firing_intensify_trigger = true
won't ever actually get touched since that variable is set to true first through modeFireStart(), then again in modeFireStartSounds(), before being checked in modeFiring().I was getting some weird overlap in my headphones when bench testing CTS Mix, which is why I started investigating. I could be way off but that's why I'm starting with the basics and working forward to be absolutely certain this is an actual issue and not just me misunderstanding the code flow and making a big deal out of a nothingburger.
b_sound_firing_intensify_trigger does get set to false if firing in CTS mix and releasing the intensify button but still firing from the wand tip button. I've been testing things on my bench test setup and haven't noticed anything strange. With CTS Mix, if you release 1 trigger but have another trigger still going, it just transitions back to the regular firing without ever stopping.
b_sound_firing_intensify_trigger does get set to false if firing in CTS mix and releasing the intensify button but still firing from the wand tip button. I've been testing things on my bench test setup and haven't noticed anything strange. With CTS Mix, if you release 1 trigger but have another trigger still going, it just transitions back to the regular firing without ever stopping.
Indeed; now that I'm done walking the code, I see that it in fact has nothing to do with those variables per se. Instead, the culprit lies squarely in modeFiring() when CTS Mix is enabled and a second fire button is pressed/released.
Say we're already holding down Intensify in Power Mode 1 (default). That means S_GB1_FIRE_LOOP
is currently looping. Then we press down the Mode/Alt button to go into CTS Mix. First thing that happens is modeFiring() detects that we have a previously-unpressed Mode/Alt in CTS Mix and calls S_FIRING_LOOP_GB1
to loop. Then it goes into the actual CTS code and calls S_FIRING_LOOP_GB1
a second time. Then, since we're at Power Level 1 (which is less than i_power_mode_max
of 5) it additionally calls S_GB1_FIRE_HIGH_POWER_LOOP
. Finally, it stops the original Intensify S_GB1_FIRE_LOOP
sound.
So right off the bat we've called two instances of the same sound, plus an extra which isn't accounted for that was done intentionally to make this magic work. If we then release the Mode/Alt switch, modeFiring() calls stopEffect(S_FIRING_LOOP_GB1)
once, leaving S_GB1_FIRE_HIGH_POWER_LOOP
playing. I'm not sure why CTS Mix has three sound effects looped on top of one another while active, but in theory this should be inaudible as all three are essentially the same (S_GB1_FIRE_HIGH_POWER_LOOP
is just S_FIRING_LOOP_GB1
but louder). The problem is what happens when we stop CTS Mix and then restart it.
In Power Level 5 (which starts out playing S_GB1_FIRE_HIGH_POWER_LOOP
before any CTS shenanigans) it's almost impossible for me to hear a difference, so I'm going to posit here that the real issue is that in Power Level 1~4, every time you let go of an alternate trigger and then push it down again, yet another instance of S_GB1_FIRE_HIGH_POWER_LOOP
is being asked to be played, over and over again.
Unfortunately it appears that the update() function which is critical for reading data back from the WAV Trigger does not function correctly. I tested it myself and wavTrigger::isTrackPlaying always returns false, which coincides with an open issue on the WAV Trigger github.
So while the most sound (heh) solution would just be to ask the WAV Trigger if the sound is already playing and then not call it again if so, it would appear that in this case, the solution will need to be a new boolean flag. I've made that change to the branch in this PR accordingly.
Once more, I apologize for all the delays.
b_sound_firing_intensify_trigger does get set to false if firing in CTS mix and releasing the intensify button but still firing from the wand tip button. I've been testing things on my bench test setup and haven't noticed anything strange. With CTS Mix, if you release 1 trigger but have another trigger still going, it just transitions back to the regular firing without ever stopping.
Indeed; now that I'm done walking the code, I see that it in fact has nothing to do with those variables per se. Instead, the culprit lies squarely in modeFiring() when CTS Mix is enabled and a second fire button is pressed/released.
Say we're already holding down Intensify in Power Mode 1 (default). That means
S_GB1_FIRE_LOOP
is currently looping. Then we press down the Mode/Alt button to go into CTS Mix. First thing that happens is modeFiring() detects that we have a previously-unpressed Mode/Alt in CTS Mix and callsS_FIRING_LOOP_GB1
to loop. Then it goes into the actual CTS code and callsS_FIRING_LOOP_GB1
a second time. Then, since we're at Power Level 1 (which is less thani_power_mode_max
of 5) it additionally callsS_GB1_FIRE_HIGH_POWER_LOOP
. Finally, it stops the original IntensifyS_GB1_FIRE_LOOP
sound.So right off the bat we've called two instances of the same sound, plus an extra which isn't accounted for that was done intentionally to make this magic work. If we then release the Mode/Alt switch, modeFiring() calls
stopEffect(S_FIRING_LOOP_GB1)
once, leavingS_GB1_FIRE_HIGH_POWER_LOOP
playing. I'm not sure why CTS Mix has three sound effects looped on top of one another while active, but in theory this should be inaudible as all three are essentially the same (S_GB1_FIRE_HIGH_POWER_LOOP
is justS_FIRING_LOOP_GB1
but louder). The problem is what happens when we stop CTS Mix and then restart it.In Power Level 5 (which starts out playing
S_GB1_FIRE_HIGH_POWER_LOOP
before any CTS shenanigans) it's almost impossible for me to hear a difference, so I'm going to posit here that the real issue is that in Power Level 1~4, every time you let go of an alternate trigger and then push it down again, yet another instance ofS_GB1_FIRE_HIGH_POWER_LOOP
is being asked to be played, over and over again.Unfortunately it appears that the update() function which is critical for reading data back from the WAV Trigger does not function correctly. I tested it myself and wavTrigger::isTrackPlaying always returns false, which coincides with an open issue on the WAV Trigger github.
So while the most sound (heh) solution would just be to ask the WAV Trigger if the sound is already playing and then not call it again if so, it would appear that in this case, the solution will need to be a new boolean flag. I've made that change to the branch in this PR accordingly.
Once more, I apologize for all the delays.
Actually it was intented to play multiple times to take advantage of the Polyphonic capabilities of the audio board, to give the effect of a more powerful stream and crazyness going on. (multiple packs crossing together).
the isTrackPlaying() will be disabled when track reporting is turned off during the setup, which is done on purpose as it is not needed. Also the I found the Wav Trigger can spam the incoming serial connection with track status information which I found was the problem with the pack crashing back in September. The problem is when updating the addressable LEDs, interrupts are turned off very quickly for timing issues and turned on back which can result in lost serial data from the wav trigger if lots of sound are triggered on/off while addressable LEDs are updated. Interrupts are used to pull the data out of the serial buffer. In the case of the wav trigger, serial data was becoming corrupted and incorrect data was written to wrong variables causing a crash.
the isTrackPlaying() will be disabled when track reporting is turned off during the setup, which is done on purpose as it is not needed. Also the I found the Wav Trigger can spam the incoming serial connection with track status information which I found was the problem with the pack crashing back in September. The problem is when updating the addressable LEDs, interrupts are turned off very quickly for timing issues and turned on back which can result in lost serial data from the wav trigger if lots of sound are triggered on/off while addressable LEDs are updated. Interrupts are used to pull the data out of the serial buffer. In the case of the wav trigger, serial data was becoming corrupted and incorrect data was written to wrong variables causing a crash.
Ah, yes, I forgot about track reporting being disabled. Good point.
As for the multiple streams that's fine while firing in full-on CTS Mix (though curiously that effect is not present in CTS, only CTS Mix), the issue is just that in Power Level 1 through 4, releasing one of the two triggers will cause the normal Proton Stream sound effect to replicate itself out-of-phase, not the CTS stream. That's what's fixed by the addition of that new boolean flag.
the isTrackPlaying() will be disabled when track reporting is turned off during the setup, which is done on purpose as it is not needed. Also the I found the Wav Trigger can spam the incoming serial connection with track status information which I found was the problem with the pack crashing back in September. The problem is when updating the addressable LEDs, interrupts are turned off very quickly for timing issues and turned on back which can result in lost serial data from the wav trigger if lots of sound are triggered on/off while addressable LEDs are updated. Interrupts are used to pull the data out of the serial buffer. In the case of the wav trigger, serial data was becoming corrupted and incorrect data was written to wrong variables causing a crash.
Ah, yes, I forgot about track reporting being disabled. Good point.
As for the multiple streams that's fine while firing in full-on CTS Mix (though curiously that effect is not present in CTS, only CTS Mix), the issue is just that in Power Level 1 through 4, releasing one of the two triggers will cause the normal Proton Stream sound effect to replicate itself out-of-phase, not the CTS stream. That's what's fixed by the addition of that new boolean flag.
Ah ok I know now what you are referring to.
Instead of merging directly into the main branch, I merged your changes into a wip wand branch i'm working on at the moment. I will close this request.
Refer here or push to this one if you do more wand changes. https://github.com/gpstar81/haslab-proton-pack/tree/wandupdates
These changes mirror the proposed changes to the original WavTrigger library which resolve several compiler error messages when compiling code for the Neutrona Wand or Proton Pack.
Apologies with having to open a new PR for this; apparently the changes recently pushed caused enough changes that it automatically closed the previous PR with these changes, so I had to remake it.