Before a recent update to the code and docs, I had a radio button library set up that would, when you clicked a button, just loop through the buttons, turning that one on and the others off. A recent update has deprecated that and I’ve switched to the new paradigm, where you use the setRadioGroupId() function and let setToggleState() handle that loop my old code was doing.
There’s one problem, that I think is a bug. No matter what I do, the first button in the radio group that is graphically visible in the program–that is, addAndMakeVisible() has been used on it and it has set bounds that aren’t zero–refuses to ever not be toggled on, whether you click another button in the group (results in both of them being on) or try to click it off. I tried making a button that is visible but has its bounds set to all zeros and adding it to the group before all the other buttons. Even then, it was still the second button added–the first one that is graphically visible–that refused to turn off. The only change was that it started out off, but it wouldn’t go off again once it was toggled on.
I am using setRadioGroupId() without issue. A minimum code sample of the failure would be helpful.
I also call setClickingTogglesState(true) for all the buttons, I believe that was required for me to get things working correctly.
1 Like
setClickingTogglesState() was a helpful tip that I didn’t realize when I read through the documentation, but setting it to true on all the buttons had no effect on the behavior when I tried it just now. Here are the parts of my code that involve this issue:
—RadialButtons.h—
#ifndef RADIALBUTTONS_H_INCLUDED
#define RADIALBUTTONS_H_INCLUDED
#include "../JuceLibraryCode/JuceHeader.h"
class RadialButtons : public Component
{
public:
RadialButtons (int enum_start, int enum_size, int enum_default = -1);
~RadialButtons();
int getValue();
void setValue (int value);
int clicked (Button* button);
private:
static int radioGroupId;
int start;
int end;
int current;
// have to use vectors because Juce::Array does not have an "initialize with this amount of empty elements" constructor
std::vector<ToggleButton> buttons;
JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (RadialButtons)
};
#endif // RADIALBUTTONS_H_INCLUDED
—RadialButtons.cpp—
#include "RadialButtons.h"
int RadialButtons::radioGroupId = 0;
RadialButtons::RadialButtons (int enum_start, int enum_size, int enum_default)
: buttons (enum_size)
{
++radioGroupId;
if (enum_default == -1) enum_default = enum_start;
start = enum_start;
end = start + enum_size;
current = enum_default;
for (int i = 0; i < enum_size; ++i) {
addAndMakeVisible (buttons[i]);
buttons[i].setRadioGroupId (radioGroupId, dontSendNotification);
buttons[i].setClickingTogglesState (true);
buttons[i].setColour (ToggleButton::tickColourId, onColour);
buttons[i].setColour (ToggleButton::tickDisabledColourId, offColour);
if (i == enum_default - enum_start) buttons[i].setToggleState (true, dontSendNotification); // this line seems to be unnecessary now, but I've confirmed that its presence does not affect this issue's behavior
}
}
RadialButtons::~RadialButtons()
{
}
int RadialButtons::getValue()
{
int index = 0;
for (; index < buttons.size(); ++index)
if (buttons[index].getToggleState() == true) break;
if (index + start != current) setValue (current);
return current;
}
void RadialButtons::setValue (int value)
{
current = value;
int index = value - start;
for (int i = 0; i < buttons.size(); ++i) if (i == index) {
buttons[i].setToggleState (true, dontSendNotification);
break;
}
}
int RadialButtons::clicked (Button* button)
{
for (int i = 0; i < buttons.size(); ++i) {
if (button == &(buttons[i])) {
if (buttons[i].getToggleState() == false) setValue (i + start);
return i + start;
}
}
throw "Button not found";
}
Either my cold is causing me to be slow, or this code seems overly complicated. (do you come from a java background, by chance?)
Why is not just:
int RadialButtons::getValue()
{
for (int index = 0; index < buttons.size(); ++index)
if (buttons[index].getToggleState() == true)
return start + index;
return -1; // or something else to indicate not buttons are clicked (although I don't know if this is possible with a radio group)
}
void RadialButtons::setValue (int value)
{
buttons[value - start].setToggleState (true, dontSendNotification);
}
int RadialButtons::clicked (Button* button)
{
for (int i = 0; i < buttons.size(); ++i) {
if (button == &(buttons[i])) {
setValue (i + start);
return i + start;
}
}
throw "Button not found";
}
1 Like
The way the library gets used throughout the program is the reason for a lot of its complexity (example: networking is involved). I admit it looks needlessly complex without the context of some other files and even some functions I omitted when posting here because they didn’t have anything to do with the issue.
That said, the modified version of setValue() is actually a much better idea and I appreciate that tip (and don’t know how I was stupid enough to not come up with it myself). But I don’t see how any of these changes would fix this issue. I just tested it with the better version of setValue() and the issue remains.
For the record, yeah, technically my first language was Java, but the way I prefer to go about programming is very anti-Java. I’m a C coder through and through 
(who has had to adopt a couple Java-like attitudes to adapt to C++'s advantages)
No, the class itself makes sense, without any further context, it was the implementation that seemed overly complex. And I asked about java, because there was something about this code that felt like it came from someone used to java. Having said that, my changes weren’t meant to fix the issue, but just to suggest a more clean implementation. Although I suspect with further understanding of usage, this class could become even more clean. For example, I would implement a buttonListener, that sets current, which than makes getValue() simply a return current; And clicked() seemed redundant since the toggling of the button state is handled automatically by the framework.
I feel 99.9% sure that the underlying JUCE functionality is working. So all I can suggest is that you spend time debugging your code to find where it is going wrong. Write a small program that uses this class for one group, and use the debugger to look at things at various stages.
What you’re suggesting around the buttonListener is more similar to some higher-level code that uses this library than you realize. And said higher-level code is also the reason for the use of the clicked() function, which, yeah, I just realized is not necessary anymore. Most of why this library looks the way it does is I wrote it before the setRadioGroupId() function existed (or at least before it was in the docs), and the reason I think this is a JUCE bug is that this library worked as intended with only slightly different code, until the old version of setToggleState() was recently deprecated.
I’ll try paring it down to really only do what’s necessary now that we have the radio group ID paradigm (which probably means removing the clicked() function altogether, as you said) and see if that fixes it, but I’m not to hopeful.
EDIT: Yep. Even when this library, and the higher functions that use it, never actually call a function that changes a button’s value (such as setToggleState()), i.e. the clicked() function doesn’t (all it does now is return i + start, which is for networking reasons and shouldn’t have anything to do with this issue) and the setValue() function is never called (it’s only used in the networking case), AND I even tried commenting out that setToggleState() line in the constructor to be safe, this issue’s behavior is not changing.
Again, I suggest using the debugger to determine where the issue is happening. That’s one of the great things about JUCE, the source is all there. If it were me, I would write a small, minimal code, stand-alone app that shows the issue. Then other people (like myself) might even give a hand in debugging it. But for now, I’ve gone about as far as I can in helping you. And the JUCE team probably won’t help without a small running code sample the shows the issue is in JUCE.
Alright, I finally got some time to debug. I couldn’t go as far as I wanted to, because I don’t understand enough about the Value class to investigate Button::isOn::operator=(), but I’ve managed to actually confirm that JUCE’s own library code is failing to toggle this button off. You can see for yourself in my debug log, which I added comments (that start with //) in after the fact, explaining what I was doing/thinking. debug_log.txt (20.3 KB)
