Why does this simple code not work as intended?

[Kasper_Kasper]

I have these buttons that when pressed become toggled, and at the same time untoggles the ohter 7 buttons to make only 1 of the toggled at all times. It works to some degree, but only if the old toggled button is lower on the list than the new toggled button.

Example, if AmazonianBtn is toggled, it does not untoggle when any of the below buttons are clicked and they are also not being toggled in this situation. But if toggled button is low on the list and the new toggled button is higher on this list it toggles+untoggles just fine.

What am i missing here? It seems such simple code and im really embarrased to ask as its probably something silly >.<

   public void ToggleHandler()
    {

        if (classAmazonianBtn.thisToggled)
        {
            classMagusBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classMagusBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classShadowspawnBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classMagusBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classBlackguardBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classMagusBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classSenjuMonkBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classMagusBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classDeadeyeBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classMagusBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classGladiatorBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classMagusBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classTemplarBtn.thisToggled = false;
        }

        if (classTemplarBtn.thisToggled)
        {
            classAmazonianBtn.thisToggled = false;
            classMagusBtn.thisToggled = false;
            classShadowspawnBtn.thisToggled = false;
            classBlackguardBtn.thisToggled = false;
            classSenjuMonkBtn.thisToggled = false;
            classDeadeyeBtn.thisToggled = false;
            classGladiatorBtn.thisToggled = false;
        }

    }

[Jjagg]

If this function geta executed after you click a button below the currently toggled button, the original button will still be toggled. So the if will succeed for that button and it will disable all buttons below it. Also if I were you I’d make a radio button group or something so you don’t have to copy paste all the button disabling.

[Kasper_Kasper]

Well i alrdy made a pretty advanced system for my buttons, would hate to trash it in favor for a new system. That being said, do you have any idea how i can solve this current issue with my current system? that would be preferable if possible

[monopalle]

How about this:

When you do your button iteration, dont do those changes at that time…

Throw them on a list of bools to BE toggled. And then when your button iteration is complete, it THEN sets all changes…

[lozzajp]

if (classAmazonianBtn.thisToggled)

Is true, then set all the others false. So when the if moves onto if (ClassMagusBtn.thisToggled) it is been set to false already.

Consider reading up on a switch statement and perhaps making a function that sets all to false and do something like this (pseudiddly code).

switch btnToggled:
case: classAmazonianBtn {
setAllFalse();
classAmazonianBtn.thisToggled = true; }

[KonajuGames]

I’m not 100% sure how your thisToggled property works, but you should be able to see that in the first if statement, you set classMagusBtn.thisToggled = false, and then the next if statement checks classMagusBtn.thisToggled. Your earlier if statements will directly affect following if statements.
Try adding an else before each if statement after the first. This means it will only go down so far as the first condition that returns true, then ignore the rest.

[willmotil]

This pattern you are using is inherently prone to error.
As others are saying you want to use a more logical pattern here.
This is essentially a simple state machine pattern or should be.

Do the following to clarify the problem exactly and see it.

Inside your method at the very top
Add a integer called result, int result = 0; This is the default state.
Add a int i = 0; this will be in this particular case the execution count, or more generally the error status.
Add the following in top down order
2)

if (classAmazonianBtn.thisToggled){ result =1; i++;}
if (classMagusBtn.thisToggled){ result =2; i++;}
if (classShadowspawnBtn.thisToggled){ result =3; i++;}
if (classBlackguardBtn.thisToggled){result =4; i++;}
if (classSenjuMonkBtn.thisToggled){result =5; i++;}
if (classDeadeyeBtn.thisToggled){result =6; i++;}
if (classGladiatorBtn.thisToggled){result =7; i++;}
if (classTemplarBtn.thisToggled){result =8; i++;}

Next after the above if statements, but still inside your method, set all of your buttons everyone of them to false.

        classAmazonianBtn.thisToggled = false;
        classMagusBtn.thisToggled = false;
        classShadowspawnBtn.thisToggled = false;
        classBlackguardBtn.thisToggled = false;
        classSenjuMonkBtn.thisToggled = false;
        classDeadeyeBtn.thisToggled = false;
        classGladiatorBtn.thisToggled = false;
        classTemplarBtn.thisToggled = false;

Then add this below, to set the last detected toggled button on only.

if (result == 1){ classAmazonianBtn.thisToggled = true;}
if (result == 2){ classMagusBtn.thisToggled = true;}
if (result == 3){ classShadowspawnBtn.thisToggled = true;}
if (result == 4){classBlackguardBtn.thisToggled = true;}
if (result == 5){classSenjuMonkBtn.thisToggled = true;}
if (result == 6){classDeadeyeBtn.thisToggled = true;}
if (result == 7){classGladiatorBtn.thisToggled = true; }
if (result == 8){classTemplarBtn.thisToggled = true;}

Turning only on the last detected toggled on result as it will have been the last one checked. The execution value i should be 1 or it is in a state of error by your own description. Note the above is better placed in a switch case method in normal conditions, though this is fine for now.

This is nearly equivalent to what you currently have written, but far easier to debug.

First of what did we do, we set the result depending on the toggle buttons, the result can only end up with one value, from 0 none, then from 1 to 8, your 7 buttons
Within each of the first if statements, we also increment i++;
This is to count the number of if statements that have actually executed.As you said only one should be executing. Then we turn off all the buttons ourselves. Then we turned on only the last toggled on button that the result value detected. At the end of the chain the i value should be no more then 1, by your description if that is correct your method is in a ok state otherwise more then one btn is toggled on which is in a state of error.

Now
If its more then that, then more then one of your if statements are evaluating to true;
Which is probably the cause of your problem.
From here you can step thru from the top down IF statement to find it by setting a breakpoint on it and running your code.

Think of the above pattern as ensuring final status, then that everything is cleared, then the last selected result is set to be in effect, or a bug finding exclusion pattern.
Excluding all possibilty’s before allowing, any single possibility in a controlled sharp clear order, enabling you to precisely control and determine the order of execution taking place.

You should throw a exception for yourself if( i > 1) at the very end, to ensure you see if this problem occurs again in the future with a little reminder note what it is detecting and why its bad.

[Kasper_Kasper]

Thank you for your very helpful answer, ive tried ur example and im getting closer to location the problem. However the problem do persist, as it seems for some reason my buttons stop activate their own toggles after the buttom/last button has been toggled, and im clueless as to why this happens since i can see the my code running trough the buttons update functions.

Here is what my button class if that could be any help:

using System;
using Microsoft.Xna.Framework;
using Microsoft.Xna.Framework.Content;
using Microsoft.Xna.Framework.Graphics;
using Microsoft.Xna.Framework.Input;

namespace EverMoreDeeper.MenuDesignPlaystyle.HeroClassesBtns
{

class AHeroClassBtn : AnimatedBtn
{
    private GameWorld gameWorld;
    private string heroClassTitle;

    Rectangle rectangle;
    public bool thisToggled = false;
    private bool clickBeganOnButton;

    public AHeroClassBtn(Texture2D newTexture, string newHeroClassTitle, Vector2 position, Vector2 newScale) : base(position)
    {
        heroClassTitle = newHeroClassTitle;
        sTexture = newTexture;
        sScale = newScale;

        FramesPerSecond = 9;

        AddAnimation(1, 0, 0, "Idle", 480, 360, new Vector2(0, 0));
        AddAnimation(1, 0, 1, "Hovered", 480, 360, new Vector2(0, 0));
        AddAnimation(1, 0, 2, "MouseDowned", 480, 360, new Vector2(0, 0));
        AddAnimation(1, 0, 3, "Toggled", 480, 360, new Vector2(0, 0));


        PlayAnimation("Idle");
    }

    public override void Update(GameTime gameTime, MouseState mouse) 
    {
        rectangle = new Rectangle((int)sPostion.X, (int)sPostion.Y, (int)192, (int)144);

        Rectangle mouseRectangle = new Rectangle(mouse.X, mouse.Y, 1, 1);

        if (!thisToggled)
        {

            if (!mouseRectangle.Intersects(rectangle))
            {
                PlayAnimation("Idle");
                clickBeganOnButton = false;
            }

            if (mouseRectangle.Intersects(rectangle) && mouse.LeftButton != ButtonState.Pressed)
            {
                PlayAnimation("Hovered");
            }

            if (mouseRectangle.Intersects(rectangle) && mouse.LeftButton == ButtonState.Pressed)
            {
                PlayAnimation("MouseDowned");
                clickBeganOnButton = true;
            }

            if (mouseRectangle.Intersects(rectangle) && mouse.LeftButton == ButtonState.Released && clickBeganOnButton == true)
            {
                thisToggled = true;
                PlayAnimation("Toggled");                                                         
            }

        }

    

        float deltaTime = (float)gameTime.ElapsedGameTime.TotalSeconds;

        base.Update(gameTime, mouse);
    }



    /// Runs every time an animation has finished playing
    public override void AnimationDone(string animation)
    {
      
    }
}

}

And my current togglehandler function in GameWorld:

        int result = 0;
        int i = 0;

        if (classAmazonianBtn.thisToggled) { result = 1; i++; }
        if (classMagusBtn.thisToggled) { result = 2; i++; }
        if (classShadowspawnBtn.thisToggled) { result = 3; i++; }
        if (classBlackguardBtn.thisToggled) { result = 4; i++; }
        if (classSenjuMonkBtn.thisToggled) { result = 5; i++; }
        if (classDeadeyeBtn.thisToggled) { result = 6; i++; }
        if (classGladiatorBtn.thisToggled) { result = 7; i++; }
        if (classTemplarBtn.thisToggled) { result = 8; i++; }


        classAmazonianBtn.thisToggled = false;
        classMagusBtn.thisToggled = false;
        classShadowspawnBtn.thisToggled = false;
        classBlackguardBtn.thisToggled = false;
        classSenjuMonkBtn.thisToggled = false;
        classDeadeyeBtn.thisToggled = false;
        classGladiatorBtn.thisToggled = false;
        classTemplarBtn.thisToggled = false;

        if (result == 1) { classAmazonianBtn.thisToggled = true; }
        if (result == 2) { classMagusBtn.thisToggled = true; }
        if (result == 3) { classShadowspawnBtn.thisToggled = true; }
        if (result == 4) { classBlackguardBtn.thisToggled = true; }
        if (result == 5) { classSenjuMonkBtn.thisToggled = true; }
        if (result == 6) { classDeadeyeBtn.thisToggled = true; }
        if (result == 7) { classGladiatorBtn.thisToggled = true; }
        if (result == 8) { classTemplarBtn.thisToggled = true; }

[Jjagg]

This just reverses your problem. result will be set to the index corresponding to the last button you checked that has thisToggled set to true. So if you click a button above the previously toggled, it will not work.

You can’t get it to work this way because your defining an explicit ordering on your buttons (the order you check them). Think about it like this. You enter the function with two buttons having thisToggled set to true, but you don’t know which one is toggled last. So inside this function you don’t have enough knowledge to pick the right button. So either you move this functionality to another function that does have enough knowledge (1) or you pass extra information to know which was toggled last (2).

I’d once again recommend to do (1), so group buttons and disable the others in the same group when one is clicked (at the time that a button is clicked you know that is is the last one clicked). If you really want to do it this way, you can add a timestamp (time when the button was last toggled on) or even easier a boolean that flags if the button was toggled on this frame to see which button was clicked last (2) and enable only that one.

if (mouseRectangle.Intersects(rectangle) && mouse.LeftButton == ButtonState.Released && clickBeganOnButton == true)
{
    justToggled = true;
    PlayAnimation("Toggled");                                                         
}

Then in your togglehandler

AHeroClassBtn justToggledBtn = null
// buttons contains all your buttons
foreach (var button : buttons) {
    if (button.justToggled) {
        justToggledBtn = button;
        // don't forget to reset justToggled
        button.justToggled = false;
    }
}

if (justToggledBtn == null) return;

foreach (var button : buttons) {
    button.thisToggled = false;
}
justToggledBtn.thisToggled = true;

[willmotil]

The reason i had him change his code to that form is for the purpose of debugging with limited information he originally posted, for him to logically find the problem.

According to his statements.

Example, if AmazonianBtn is toggled, it does not untoggle when any of
the below buttons are clicked

The first button in your original code turned off all others below it immediately.

and they are also not being toggled in this situation.

What would happen if two of your button rectangles were overlapping ?
When you clicked on a lower one in the list when a previous one was still toggled on above it ?

But if a toggled button is low on the list and the new
toggled button is higher on this list it toggles+untoggles just fine.

What would happen in the reverse case where to occur, the lower was toggled on then the one above it was toggled on afterwards ?

What am i missing here?

// place this in SomeClass moving it out of and place it below your toggle handler

public static void ToggleAllOff()
{
classAmazonianBtn.thisToggled = false;
classMagusBtn.thisToggled = false;
classShadowspawnBtn.thisToggled = false;
classBlackguardBtn.thisToggled = false;
classSenjuMonkBtn.thisToggled = false;
classDeadeyeBtn.thisToggled = false;
classGladiatorBtn.thisToggled = false;
classTemplarBtn.thisToggled = false;
}

change the code in this if statement, in each of your classes.

if (mouseRectangle.Intersects(rectangle) && mouse.LeftButton == ButtonState.Released && clickBeganOnButton == true)
{
//thisToggled = true;
if(thisToggled)
{
// ToggleAllOff(); //depending on the behavior you want
// thisToggle = false; //depending on the behavior you want
}
else
{
SomeClass.ToggleAllOff();
thisToggled = true;
PlayAnimation(“Toggled”);
}
}

If that executes correctly you can then remove the entire toggle handler method.
Or leave the part in it that increments i for a later assert debug or exception throw if you wanted it to notify yourself of overlapping rectangles.
Or you can drop in jjags solution to the problem above.

if not.

Do you draw the same rectangle to the screen that you are using to check for clicks.
To verify that these hardcoded clicking rectangles are actually were the buttons are drawn on screen ?
or
Add a line to the end of that toggle handler method like so.

if(i > 1)
{
int dummyvar =0; // place a breakpoint on this line, run your code, click some buttons
}

if your code breaks on this line above, two of your clicking rectangles are overlapping

[Jjagg]

@willmotil

The code should turn off all toggles then only turns on the last one clicked.

The whole point of my last comment is that his toggledchanged function does not know which button is clicked last, since when a button is clicked the thisToggled property is just set to true and the thisToggled property for other buttons is not modified so the previously toggled button will stay toggled.[quote=“Jjagg, post:9, topic:7683”]
You enter the function with two buttons having thisToggled set to true, but you don’t know which one is toggled last.
[/quote]
The code you posted cannot work for the same reason.

[willmotil]

Ya i saw that a minute later. I’ll change that.

I was guessing thisToggled was a property not a bool and i overlooked that line of code. Either way, im trying to point out as well, that it could also be the simple fact that he has hard coded his rectangles click positions, into each class.
If he then copy pasted a class, as a template, or this method but did not change the rectangles hard coded value. This problem will persist regardless of the toggle function being on or off proper or not.

Normally i do something like this with my buttons.
This way you can basically just do whatever you want.
Actions are a special kind of delegate for simple method calling they are nice for buttons.
They let your game class and your individual classes use buttons however they want

public class MyGame1
{
    List<butn> button_list = new List<butn>();
    myclass a = new myclass();
    myclass b = new myclass();

    public MyGame1()
    {
        // add some buttons and link them to whatever class you want
        button_list.Add(butn.NewButton(new Rectangle(0,0,100,100),a.OnClick, a.OnMouseOver ) );
        button_list.Add(butn.NewButton(new Rectangle(0, 200, 100, 100), b.OnClick, b.OnMouseOver, true, false));
    
    }
    
    public void CheckButns(int x, int y, bool isleftclicked)
    {
        // loop the buttons
        foreach (butn n in button_list)
        {
            // check something
            if (n.posRectangle.Contains(x,y) && isleftclicked)
            {
                n.FireOnClick(); // do something
            }
            if (n.isToggleable)
            {
                if(n.isToggled)
                {
                     // you get the idea
                }
            }
            if (n.isAnimating)
            {
                n.Animating();
            }
        }
    }
}
public class myclass
{
    public myclass()
    {

    }
    
    public void OnClick()
    {
        // called action to do on click or mouse over
    }
    public void OnMouseOver()
    {
        // called action to do on click or mouse over
    }
    public void DrawToggled(){}
}


 // you can just put this in some corner and forget about it
//
public class butn
{
    public Rectangle posRectangle;
    public Action click;
    public Action mouseOver;

    public bool isAnimating = false;
    public bool isToggleable = false;
    public bool isToggled = false;

    public butn(){}
    public static butn NewButton(Rectangle rectangle, Action _onclick, Action _mouseover)
    {
        butn n = new butn();
        n.posRectangle = rectangle;
        n.click = _onclick;
        n.mouseOver = _mouseover;
        return n;
    }
    public static butn NewButton(Rectangle rectangle, Action _onclick, Action _mouseover, bool isToggleable, bool isToggledon)
    {
        butn n = new butn();
        n.posRectangle = rectangle;
        n.click = _onclick;
        n.mouseOver = _mouseover;
        n.isToggleable =  isToggleable;
        n.isToggled = isToggled;
        return n;
    }
    public void FireOnClick()
    {
        click();
    }
    public void FireOnMouseOver()
    {
        mouseOver();
    }
    public void Animating()
    {
    }
    public void DefaultDraw(){}
}

[Kasper_Kasper]

Took ur advise on putting all the buttons in a list and finally after some quite a bit of refactoring got it to work, yay!^^ Thanks alot all of you, you were a great help and i learned alot from ur advises! Cheers!

• Kasper