- Home /
Question is off-topic or not relevant
whats wrong with this script
The array amount has 3 element element0 - 100, element1 - 200 and element2 - 300. when i click the claim button for the first time it add 100 coins which is correct but when i click second time it add 300 coins instead of 200 coins and when i click the third time it add 800 coins instead of 300 coins . And after adding 300 coins it shows index out of the range exception. why does it add previous index amount. for the first time it add 100 which is correct. for the second time it add 300 which is (element0 + element1) 100 + 200 and for the third time it add 800 which is (element0 + elemnent1 + element1 + element2) 100 + 200 + 200 + 300
[System.Serializable]
public class AchievementData
{
public int[] itemCount, amount;
public Button claimButton;
}
public class AchievementsUI : MonoBehaviour
{
public CashSystem cashSystem;
public AchievementData[] achievementData;
public List<string> savedName = new List<string>();
public List<int> savedCount = new List<int>();
void Start()
{
string path = Application.persistentDataPath + "/AchievementItemName.dat";
if (File.Exists(path))
{
DataManager8 data = SaveData.LoadAchievementItemName();
savedName = data.itemName;
DataManager9 count = SaveData.LoadAchievementItemCount();
savedCount = count.itemCount;
}
for(int i = 0; i < savedName.Count; i++)
{
switch(savedName[i])
{
case "Gold":
Inititalize(0, savedCount[i], savedName[i]);
break;
case "Silver":
Inititalize(1, savedCount[i], savedName[i]);
break;
}
}
}
void Inititalize(int id, int count)
{
for(int i = 0; i < achievementData[id].itemCount.Length; i++)
{
if(count >= achievementData[id].itemCount[i])
{
achievementData[id].claimButton.interactable = true;
achievementData[id].claimButton.onClick.AddListener(() => ClaimButtonClick(id, i, count));
break;
}
}
}
void ClaimButtonClick(int id, int index,int count)
{
cashSystem.AddCash(achievementData[id].amount[index]);
achievementData[id].claimButton.interactable = false;
AchievementStatus(id, index, count);
}
void AchievementStatus(int id, int index, int count)
{
index += 1;
if(count >= achievementData[id].itemCount[index])
{
achievementData[id].claimButton.interactable = true;
achievementData[id].claimButton.onClick.AddListener(() => ClaimButtonClick(id, index, count));
}
}
}
Answer by GeroNL · Aug 14, 2021 at 02:24 PM
i suggest do debug call in initialize and in AchievementStatus, who actually repeated call ClaimButtonClick->addcash,
Hope it help.
Answer by Bunny83 · Aug 14, 2021 at 02:34 PM
Well, "AddListener" does what it's supposed to do: It adds the method as a listener to the event. You never remove the listener. So when you add the second listener, the first one is still registered so both methods will fire the second time. You may want to call RemoveAllListeners before you add a new one if you want to "replace" the method.
Also be careful with closures around for loop variables. In your case you do break out of the loop, however I would generally avoid using a for loop variable in a closure. If for some reason you restructure your code and the for loop may continue the closure would use the wrong index, possibly even one that is out of bounds since when a for loop runs to the end, the loop variable is generally one larger than the largest allowed index. So you should always copy the loop variable into a local variable when you use it in a closure.
We don't have the full picture of your code, however I would highly suggest to refactor more often. Class names like "DataManager9" looks like heavy code smell. Also an instance of that class called "count" is even more confusing ^^.
Follow this Question
Related Questions
Index out of range inside IEnumerator 0 Answers
Audio loop half working 0 Answers
Resizable Array of Classes? 2 Answers
Array Is Out Of Range 2 Answers