Problem
I’m currently working on a live keyboard demo on unity. I’ve animated every key but my script spans over 2000 lines. I’m fairly happy with my script but I know theres better ways to do it.
I have defined functions and if statements for each and every key.
I’ll post my code below
private Animator Alpha1Press;
private Animator Alpha2Press;
private Animator Alpha3Press;
private Animator Alpha4Press;
private Animator Alpha5Press;
private Animator Alpha6Press;
private Animator Alpha7Press;
private Animator Alpha8Press;
private Animator Alpha9Press;
private Animator Alpha0Press;
public void playAlpha1()
{
Alpha1Press.SetBool("Alpha1Pressed", true);
}
public void stopAlpha1()
{
Alpha1Press.SetBool("Alpha1Released", true);
}
public void playAlpha2()
{
Alpha2Press.SetBool("Alpha2Pressed", true);
}
public void stopAlpha2()
{
Alpha2Press.SetBool("Alpha2Released", true);
}
public void playAlpha3()
{
Alpha3Press.SetBool("Alpha3Pressed", true);
}
public void stopAlpha3()
{
Alpha3Press.SetBool("Alpha3Released", true);
}
public void playAlpha4()
{
Alpha4Press.SetBool("Alpha4Pressed", true);
}
public void stopAlpha4()
{
Alpha4Press.SetBool("Alpha4Released", true);
}
public void playAlpha5()
{
Alpha5Press.SetBool("Alpha5Pressed", true);
}
public void stopAlpha5()
{
Alpha5Press.SetBool("Alpha5Released", true);
}
public void playAlpha6()
{
Alpha6Press.SetBool("Alpha6Pressed", true);
}
public void stopAlpha6()
{
Alpha6Press.SetBool("Alpha6Released", true);
}
public void playAlpha7()
{
Alpha7Press.SetBool("Alpha7Pressed", true);
}
public void stopAlpha7()
{
Alpha7Press.SetBool("Alpha7Released", true);
}
public void playAlpha8()
{
Alpha8Press.SetBool("Alpha8Pressed", true);
}
public void stopAlpha8()
{
Alpha8Press.SetBool("Alpha8Released", true);
}
public void playAlpha9()
{
Alpha9Press.SetBool("Alpha9Pressed", true);
}
public void stopAlpha9()
{
Alpha9Press.SetBool("Alpha9Released", true);
}
public void playAlpha0()
{
Alpha0Press.SetBool("Alpha0Pressed", true);
}
public void stopAlpha0()
{
Alpha0Press.SetBool("Alpha0Released", true);
}
public void animations()
{
// ...
if (Input.GetKeyDown(KeyCode.Alpha1))
{
Alpha1Press.SetBool("Alpha1Released", false);
playAlpha1();
}
if (Input.GetKeyUp(KeyCode.Alpha1))
{
Alpha1Press.SetBool("Alpha1Pressed", false);
stopAlpha1();
}
if (Input.GetKeyDown(KeyCode.Alpha2))
{
Alpha2Press.SetBool("Alpha2Released", false);
playAlpha2();
}
if (Input.GetKeyUp(KeyCode.Alpha2))
{
Alpha2Press.SetBool("Alpha2Pressed", false);
stopAlpha2();
}
if (Input.GetKeyDown(KeyCode.Alpha3))
{
Alpha3Press.SetBool("Alpha3Released", false);
playAlpha3();
}
if (Input.GetKeyUp(KeyCode.Alpha3))
{
Alpha3Press.SetBool("Alpha3Pressed", false);
stopAlpha3();
}
if (Input.GetKeyDown(KeyCode.Alpha4))
{
Alpha4Press.SetBool("Alpha4Released", false);
playAlpha4();
}
if (Input.GetKeyUp(KeyCode.Alpha4))
{
Alpha4Press.SetBool("Alpha4Pressed", false);
stopAlpha4();
}
if (Input.GetKeyDown(KeyCode.Alpha5))
{
Alpha5Press.SetBool("Alpha5Released", false);
playAlpha5();
}
if (Input.GetKeyUp(KeyCode.Alpha5))
{
Alpha5Press.SetBool("Alpha5Pressed", false);
stopAlpha5();
}
if (Input.GetKeyDown(KeyCode.Alpha6))
{
Alpha6Press.SetBool("Alpha6Released", false);
playAlpha6();
}
if (Input.GetKeyUp(KeyCode.Alpha6))
{
Alpha6Press.SetBool("Alpha6Pressed", false);
stopAlpha6();
}
if (Input.GetKeyDown(KeyCode.Alpha7))
{
Alpha7Press.SetBool("Alpha7Released", false);
playAlpha7();
}
if (Input.GetKeyUp(KeyCode.Alpha7))
{
Alpha7Press.SetBool("Alpha7Pressed", false);
stopAlpha7();
}
if (Input.GetKeyDown(KeyCode.Alpha8))
{
Alpha8Press.SetBool("Alpha8Released", false);
playAlpha8();
}
if (Input.GetKeyUp(KeyCode.Alpha8))
{
Alpha8Press.SetBool("Alpha8Pressed", false);
stopAlpha8();
}
if (Input.GetKeyDown(KeyCode.Alpha9))
{
Alpha9Press.SetBool("Alpha9Released", false);
playAlpha9();
}
if (Input.GetKeyUp(KeyCode.Alpha9))
{
Alpha9Press.SetBool("Alpha9Pressed", false);
stopAlpha9();
}
if (Input.GetKeyDown(KeyCode.Alpha0))
{
Alpha0Press.SetBool("Alpha0Released", false);
playAlpha0();
}
if (Input.GetKeyUp(KeyCode.Alpha0))
{
Alpha0Press.SetBool("Alpha0Pressed", false);
stopAlpha0();
}
// ...
}
What’s a more effective way to write this? I’m a firm believer in the DRY principle and this is simply TOO much
Solution
That moment when you know Unity better than C#. I’m on other side: I know nothing about Unity (except docs that I’ve read while writing this review) but know C# well. Let’s merge our knowledge.
-
Animator
is a single animation engine, in other wordsGetComponent<Animator>()
always returns the same value, thus you need only one variable to store it. -
KeyCode
isenum
(link) thus you can iterate it with a loop. -
string in format
<KeyCode>+<Pressed/Released>
is good to concatenate these two parts.
The refactored copy of your whole script:
public class keyboard : MonoBehaviour
{
private Animator animator;
void Start()
{
animator = GetComponent<Animator>();
}
private string BoolToPressed(bool state)
{
return state ? "Pressed" : "Released";
}
private void PlayKey(KeyCode keyCode, bool state)
{
animator.SetBool(keyCode + BoolToPressed(!state), false);
animator.SetBool(keyCode + BoolToPressed(state), true);
}
private void animations()
{
for (KeyCode k = KeyCode.Backspace; k <= KeyCode.Menu; k++)
{
if (Input.GetKeyDown(k))
PlayKey(k, true);
else
if (Input.GetKeyUp(k))
PlayKey(k, false);
}
}
void Update()
{
animations();
}
}