Simple Button - works sometimes, doesn't work sometimes

I’ve got a class defined for a button. I define this button globally as b1 and in setup define the size and values. Then in draw I have b1.update() and b1.display() and if(b1.press()) {functionon = !functionon}; where functionon essentially carries out what I want the button to do when pressed.

It works sometimes, but sometimes when I click the button - the colour doesn’t change nor does the function work - what could be the reason?

Code of my class below:

class Buttons {
  int rectX, rectY;
  int circleX, circleY;
  int rectSize;
  int circleSize;
  color rectColor, circleColor;
  color oC;
  color rectHighlight, circleHighlight;
  boolean rectOver;
  boolean circleOver;
  boolean on = true;
  int shape;

  Buttons (int shaper, int x, int y, int size, color cl, int hl) {
    shape = shaper;
    if (shape == 1){
      rectX = x;
      rectY = y;
      rectSize = size;
      rectColor = cl;
      rectHighlight = hl;
    }else if (shape == 0){
      circleX = x;
      circleY = y;
      circleSize = size;
      circleColor = cl;
      circleHighlight = hl;
    }
    oC = cl;
  }

  void update() {
  if ( overCircle(circleX, circleY, circleSize) ) {
    circleOver = true;
    rectOver = false;
  } else if ( overRect(rectX, rectY, rectSize, rectSize) ) {
    rectOver = true;
    circleOver = false;
  } else {
    circleOver = rectOver = false;
  }
  }
  
  boolean press(){
    if(shape ==1){
  if (mousePressed & rectOver){
     on = !on;
    if (on == false){
      rectColor = rectHighlight;
    } else{
      rectColor = oC;
    }
    return true;
    } else{
    return false;
  }
    }else{
      if (mousePressed & circleOver){
     on = !on;
    if (on == false){
      circleColor = circleHighlight;
    } else{
      circleColor = oC;
    }
    return true;
    } else{
    return false;
  }
    }
  }

boolean overRect(int x, int y, int width, int height)  {
  if (mouseX >= x && mouseX <= x+width && 
      mouseY >= y && mouseY <= y+height) {
    return true;
  } else {
    return false;
  }
}

boolean overCircle(int x, int y, int diameter) {
  float disX = x - mouseX;
  float disY = y - mouseY;
  if (sqrt(sq(disX) + sq(disY)) < diameter/2 ) {
    return true;
  } else {
    return false;
  }
}
  void display() {
  
  if (rectOver) {
    fill(rectHighlight);
  } else {
    fill(rectColor);
  }
  stroke(255);
  rect(rectX, rectY, rectSize, rectSize);
  
  if (circleOver) {
    if(on){
    fill(circleHighlight);
    }else{
      fill(oC);
    }
  } else {
    fill(circleColor);
  }
  stroke(0);
  ellipse(circleX, circleY, circleSize, circleSize);
  }
}

Hi,

I just tested your code and the mouse/ button intersection seems to work fine.

Here some advice to improve your code :

  • You can press Ctrl+T to auto format your code in the Processing IDE

  • You shouldn’t name your class Buttons as it represent a single button not a collection. Instead name it Button.

  • You can see that your if statements are too complicated :

    1. Instead of saying “if x is true then return true; else return false” just say “return x” because it’s basically the same

    2. You shouldn’t encapsulate two types of button inside your Button class. Instead use heritage to create two classes RectangleButton and CircleButton that redefine the functions display() and over().

  • For a more “modular” approach, you can define an interface ActionEvent with one single method execute() for example that represent an action to do when you press your button. Then the Button class is going to hold an instance of ActionEvent that you can call when the mouse is pressed inside the button.

This is a potential solution :

// Out list of buttons
ArrayList<Button> buttons;

void setup() {
  size(500, 500);

  buttons = new ArrayList<Button>();
  
  // Add a rectangle button
  buttons.add(new RectangleButton(50, 50, 150, 50, new ActionEvent() {
    public void execute() {
      println("Rectangle button pressed");
    }
  }
  ));

  buttons.add(new CircleButton(50, 200, 50, new ActionEvent() {
    public void execute() {
      println("Circle button pressed");
    }
  }
  ));
}

void draw() {
  background(255);
  
  // Display every buttons
  for (Button b : buttons) {
    b.display();
  }
}

void mousePressed() {
  // For every buttons, execute the action if mouse is over
  for (Button b : buttons) {
    if (b.over(mouseX, mouseY)) {
      b.execute();
    }
  }
}

// Interface to represent an action
public interface ActionEvent {
  public void execute();
}

public abstract class Button {
  protected int x, y, w, h;
  private ActionEvent action;

  public Button(int x, int y, int w, int h, ActionEvent action) {
    this.x = x;
    this.y = y;
    this.w = w;
    this.h = h;
    this.action = action;
  }

  public void execute() {
    action.execute();
  }
  
  // Change the fill color depending on the action
  // Red : not pressed
  // Green : over
  // Blue : over and pressed
  public void setFill() {
    if (over(mouseX, mouseY)) {
      if (mousePressed) {
        fill(0, 0, 255);
      } else {
        fill(0, 255, 0);
      }
    } else {
      fill(255, 0, 0);
    }
  }
  
  // Abstract method display to be redefined
  public abstract void display();
  
  // Abstract method over to be redefined
  public abstract boolean over(int x, int y);
}

public class RectangleButton extends Button {
  public RectangleButton(int x, int y, int w, int h, ActionEvent action) {
    super(x, y, w, h, action);
  }

  public void display() {
    setFill();
    rect(x, y, w, h);
  }

  public boolean over(int x, int y) {
    return (x >= this.x && x <= this.x + w) &&
      (y >= this.y && y <= this.y + h);
  }
}

public class CircleButton extends Button {
  public CircleButton(int x, int y, int diameter, ActionEvent action) {
    super(x, y, diameter, diameter, action);
  }

  public void display() {
    setFill();

    ellipseMode(CORNER);
    ellipse(x, y, w, h);
  }

  public boolean over(int x, int y) {
    return dist(x, y, this.x + w / 2, this.y + h/2) <= w/2;
  }
}

2 Likes

Thanks for your reply Joseph. Please see the screenshot gif below, it still doesn’t quite work - I got rid of a circular button and just using rectangles now as well. The top button controls the eye blinking (on or off) and should turn grey when pressed once (turn off blinking), white when pressed a second time (turn on blinking). Instead at times I have to click 3-4 times before it changes colour and the blinking state.
Untitled 2