koobas.hobune.stream
If statement is being ignored and else is run instead
Hey guys, I've been getting a certain issues with my code and I can't for the love of god figure out why this is happening. The code below is a part of a stringbuilder which shows the player how many of a certain item he has and how many he needs for crafting. The problem I have is that the if-statement is ignored completely and the else is run instead. I don't know why this is happening and it frustrates me a lot.
Without the else statement everything works as expected but when I add it, it breaks the whole damn code. It keeps showing 0/8 when it should be 15/8 because that is what I've set the item amount to. I tried throwing out Debug.Logs and for some reason it shows that the if statement is run once but it also shows that the else statement is run 34 times. A very specific number. Not sure why.
So I don't forget, this function is only run when the player clicks on an item he wants to craft. Its a OnClick event so it should only run once.
public void ShowItemInfo()
{
for (int i = 0; i < slots.Itemslots.Length; i++)
{
if (Reci.MaterialsNeeded == 3)
{
if (Reci.Materials[3].item.isequal(slots.Itemslots[i].Item))
{
sb3.Length = 0;
ShowItemInfo3(slots.Itemslots[i].Item.amount, Reci.Materials[3].amount);
ItemInfo[3].text = sb3.ToString();
}
else
{
sb3.Length = 0;
ShowItemInfo3(0, Reci.Materials[3].amount);
ItemInfo[3].text = sb3.ToString();
}
}
}
}
The reason I wanted an else there is because if I don't have the item it's checking with I want it to instead just show the player that he has zero of that item.
You do realise that your for loop runs through all items in your itemslots? If the item you're looking for is at index 3 out of 24 you will enter the if body when "i" is 3 but for all following iterations (4, 5, ...23) you will enter the else part. Unless the item you're looking for is the last one you will always have the else part running last.
Your general structure is quite confusing. Why do you check if (Reci.MaterialsNeeded == 3) every iteration? It doesn't change in between. If it actually is changed by one of your used methods this would be a very (very) bad design. What you usually would do is something like this:
public void ShowItemInfo()
{
if (Reci.MaterialsNeeded == 3)
{
sb3.Length = 0;
ShowItemInfo3(0, Reci.Materials[3].amount);
ItemInfo[3].text = sb3.ToString();
for (int i = 0; i < slots.Itemslots.Length; i++)
{
if (Reci.Materials[3].item.isequal(slots.Itemslots[i].Item))
{
sb3.Length = 0;
ShowItemInfo3(slots.Itemslots[i].Item.amount, Reci.Materials[3].amount);
ItemInfo[3].text = sb3.ToString();
break;
}
}
}
}
I guess sb3 is a StringBuilder? This is also bad design. It seems that ShowItemInfo3 does not actually show any information but actually fill the sb3 string builder with data. This is completely hidden and not obvious from the code. If you want to re-use the stringbuilder, just pass it as parameter to the method. Also you may want to find a better name for the method that actually describes what it does.
Also the name "ShowItemInfo3" suggests that there might be also a "ShowItemInfo2" and "ShowItemInfo1" method?!? This is also not very descriptive and also a general hint for a bad design decision.
$$anonymous$$aterialsNeeded checks how many materials the item I want to craft requires.
I only have one ShowItemInfo method that runs the code above. Bad na$$anonymous$$g convention, I know, will fix it as soon as I have nipped this issue in the bud. below the above method I have the parameters the stringbuilder looks for. I have four, one for each item.
public void ShowItemInfo0(int itemamount, int itemreq)
{
if (itemreq > 0)
{
sb0.Append(itemamount);
sb0.Append("/");
sb0.Append(itemreq);
}
}
public void ShowItemInfo1(int itemamount, int itemreq)
{
if (itemreq > 0 && itemamount > 0)
{
sb1.Append(itemamount);
sb1.Append("/");
sb1.Append(itemreq);
}
}
public void ShowItemInfo2(int itemamount, int itemreq)
{
if (itemreq > 0)
{
sb2.Append(itemamount);
sb2.Append("/");
sb2.Append(itemreq);
}
}
public void ShowItemInfo3(int itemamount, int itemreq)
{
if (itemreq > 0)
{
sb3.Append(itemamount);
sb3.Append("/");
sb3.Append(itemreq);
}
}
How would I go by fixing it if I had four items I needed to check. if I do it as you suggested above then I would need a for loop inside every if statement that checks $$anonymous$$aterialsNeeded. I would like to apologize for not pasting all the code from the beginning. Sorry.
I would also like to add that I can at most have an item require 4 separate materials in order for the player to be able to craft it. At the least it is 1 material needed for crafting. The above code I pasted shows only one item, one material....I'm making this more confusing than it is aren't I.
Yeah, The for loop didn't even cross my $$anonymous$$d to be honest. But I can see now why it was a bad idea to have the check inside of the loop and not vice versa. I'm far from being adept at program$$anonymous$$g but I hope I can get there someday. Your suggestion worked, though now I have one for loop inside every if statement that checks "$$anonymous$$aterialsNeeded". So four for loops in total. It works as I wanted and there are no issues with it anymore but how optimized is this? is it O$$anonymous$$ to have four for loops inside a single $$anonymous$$ethod?
