1
\$\begingroup\$

I'm trying to dynamically create Unity 5 UI buttons that each set a global variable decision_id to a different value when clicked.

The problem I'm having with this implementation is that they all set decision_id to the exact same value right now. That value being the last one in my foreach loop.

Here's my code:

IEnumerator make_decision(XmlNode options_node) {
    // reset decision_id
    decision_id = "nothing";

    // spawn a button for each option_node in options_node
    int i = 1;
    var new_option_1 = Instantiate(option_1);
    var new_option_2 = Instantiate(option_2);
    var new_option_3 = Instantiate(option_3);
    var new_option_4 = Instantiate(option_4);
    foreach (XmlNode option_node in options_node) {
        XmlNode temp_option_node = option_node;
        string option_id = temp_option_node.Attributes["id"].Value;
        Debug.Log(option_id);
        switch (i) {
            case 1:
                new_option_1.GetComponentsInChildren<Text>()[0].text = option_node.InnerText;
                new_option_1.GetComponent<Button>().onClick.AddListener(() => set_decision_id(option_id));
                new_option_1.transform.SetParent(canvas.transform, false);
                break;
            case 2:
                new_option_2.GetComponentsInChildren<Text>()[0].text = option_node.InnerText;
                new_option_2.GetComponent<Button>().onClick.AddListener(() => set_decision_id(option_id));
                new_option_2.transform.SetParent(canvas.transform, false);
                break;
            case 3:
                new_option_3.GetComponentsInChildren<Text>()[0].text = option_node.InnerText;
                new_option_3.GetComponent<Button>().onClick.AddListener(() => set_decision_id(option_id));
                new_option_3.transform.SetParent(canvas.transform, false);
                break;
            case 4:
                new_option_4.GetComponentsInChildren<Text>()[0].text = option_node.InnerText;
                new_option_4.GetComponent<Button>().onClick.AddListener(() => set_decision_id(option_id));
                new_option_4.transform.SetParent(canvas.transform, false);
                break;
            default:
                Debug.Log("Too many option nodes! Should only be four per options section");
                break;
        }
        i++;
    }

    while (decision_id == "nothing") {
        yield return new WaitForSeconds(.5f);
        // the button will change decision_id for us eventually
        // decision_id = "monarchy";
    }

    // despawn each button spawned above
    Destroy(new_option_1);
    Destroy(new_option_2);
    Destroy(new_option_3);
    Destroy(new_option_4);

}

I've searched this issue and have found lots of talk about the lambda's capturing instances instead of values... but I've already gone out of my way to prevent this by capturing both the XmlNode itself AND the id string in local variables.

Is there something super obvious that I'm missing here?

\$\endgroup\$
  • \$\begingroup\$ You could avoid this whatsoever, just add a string property onto new_option_X objects and set it instead - you would not need lambdas at all. new_option_1.GetComponent<OptionProperty>().option = option_id ; \$\endgroup\$ – wondra Oct 8 '15 at 17:09
  • \$\begingroup\$ How would that change the decision_id variable when clicked? \$\endgroup\$ – Ortsac Oct 8 '15 at 18:10
  • \$\begingroup\$ in start() method subscribe to set_decision_id(), which will be will be fetching the string value from the property, rather than argument. In case set_decision_id() is static function, just add a method on the same script as property is, which will just call set_decision_id() with the string property as argument. \$\endgroup\$ – wondra Oct 8 '15 at 21:56
  • \$\begingroup\$ That's a weird one. This seems to be the same problem you're having: stackoverflow.com/questions/28922083/… Maybe see if one of those answers helps? I have no idea why they would, but I guess it worked for those people. \$\endgroup\$ – tandersen Oct 8 '15 at 22:05
1
\$\begingroup\$

You could avoid this switch entirely. Just add a string property onto new_option_X objects and set it instead; you would not need lambdas at all.

switch (i) {
  case 1:
    new_option_1.GetComponent<OptionProperty>().Option = option_id;
    ...

where OptionProperty is simple script:

public class OptionProperty : MonoBehaviour {
  public string Option { get; set; }

  void Start()
  {
    /*"same" handler for all buttons, fetch the argument from property instead*/
    GetComponent<Button>().onClick.AddListener(set_decision()); 
  }

  /*not using anonymous functions allows you to unsubscribe later, preventing potential memory leaks*/
  private void set_decision() 
  {
    set_decision_id(Option); /*or whatever function wherever it is*/
  }
}
|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.