Problem
I made my own class that inherits from button and has an OnHold
event. It seems to work ok but I’m wondering if I can improve it or if you can spot any glaring mistakes that I made.
Here is the code:
using UnityEngine;
using UnityEngine.UI;
using UnityEngine.EventSystems;
using UnityEngine.Events;
[AddComponentMenu("UI/Hold Button")]
public class HoldButton : Button
{
private bool isBeingPressed;
private float currTime;
private static float TimeMax = 2;
private bool startTimer;
public ButtonHoldEvent onHold { get; set; }
public override void OnPointerDown(PointerEventData eventData)
{
startTimer = true;
currTime = 0;
}
public override void OnPointerUp(PointerEventData eventData)
{
startTimer = false;
currTime = 0;
isBeingPressed = false;
}
void Update()
{
if(startTimer)
{
currTime += Time.deltaTime;
if(currTime >= TimeMax)
{
isBeingPressed = true;
}else
{
isBeingPressed = false;
}
}
if (isBeingPressed)
{
onHold.Invoke();
}
}
public class ButtonHoldEvent : UnityEvent
{
public ButtonHoldEvent() : base()
{
}
}
}
Solution
There is a few thing you can improve :
TimeMax
is used like a constant here, but is not given the access modifierconst
.-
Try to group your class members in a meaningful way. It could depend on their function(member of same interface), access-modifier, or what makes sense to you.
public ButtonHoldEvent onHold { get; set; } private static float TimeMax = 2; private bool isBeingPressed; private float currTime; private bool startTimer;
-
The name
currTime
doesn’t reflect what it contains.heldDuration
is more appropriate. The same applies forTimeMax
and should be renamedTriggerTime
. isPressed
is fine, butisHeldDown
fits more the theme here.
Full code :
[AddComponentMenu("UI/Hold Button")]
public class HoldButton : Button
{
public ButtonHoldEvent onHold { get; set; }
private const float TriggerTime = 2;
private bool isHeldDown;
private float heldDuration;
public override void OnPointerDown(PointerEventData eventData)
{
isHeldDown = true;
heldDuration = 0;
}
public override void OnPointerUp(PointerEventData eventData)
{
isHeldDown = false;
heldDuration = 0;
}
void Update()
{
if (!isHeldDown)
{
heldDuration += Time.deltaTime;
if (heldDuration >= TriggerTime)
{
onHold.Invoke();
// uncomment next line, to make `onHold` trigger only once per mouse press-hold
//isHeldDown = false;
}
}
}
public class ButtonHoldEvent : UnityEvent
{
public ButtonHoldEvent() : base()
{
}
}
}