CODEWITHSUNDEEP
c#unity-game-engine
Fan4i
Fan4i

Reputation: 115

for loop is executed always just ONCE

Brief information image, what my code do: enter image description here

And this is my code:

private void CheckObjectAttackPoints(Point AttackPoint){
    Point ObjectAttackPoint = AttackPoint;
    ObjectAttackPoint.X -= 1;
    int count=0; //This variable for reading how many tiles are false
    //Check tiles active and ObjectAttackPoint is exist in List
    for (int i=0; i < 1;i++) {
        if (GameManager.AllPoints.Contains (ObjectAttackPoint)) {
            if (!GameManager.TileColliders [ObjectAttackPoint.X, ObjectAttackPoint.Y].activeSelf) {
                count++;
                ObjectAttackPoints [i] = ObjectAttackPoint;
                Debug.Log (ObjectAttackPoints [i].X +", " + ObjectAttackPoints [i].Y);
            }
        }
        if (i == 1) {
            break;
        }
        ObjectAttackPoint.X += 2;
    }

    if (count > 0) {
        Debug.Log ("Object can attack " + count + " points");
    }

}

So, this method need AttackPoint which already has AttackPoint.Y-1 value, we need only to check if AttackPoint.X exists in List and check if object at this point is active. At the method start, AttackPoint.X decreases its value by 1.

My problem is that console returns only one point even if two tiles are not active (Example image: 0,0 and 0,2 tiles are not active, console returns only count=1 and tile with Point 0,0), this means that method checks only one tile and my code have a mistake, but I can't understand where it is. Can someone help me?

Upvotes: 3

Views: 1238

Answers (3)

Fabjan
Fabjan

Reputation: 13676

Welcome to StackOverflow. There are a few problems with this code:

  1. The i < 1 condition won't allow the for loop to execute more than once.

  2. This code can be considered as a fairly good example of 'spaghetti code', it's considered a bad practice to use break operator in a loop unless it's absolutely necessary.

  3. There is no need to use a for loop to perform a limited number of operations that are known in advance

  4. Check for each attack point could be and imho should be done in a separate method.

You can do something along the lines of:

private bool CheckObjectAttackPoint(Point AttackTo)
{
   return GameManager.AllPoints.Contains(AttackTo) && !GameManager.TileColliders[AttackTo.X, AttackTo.Y].activeSelf
}

private bool CheckObjectAttackPoint(Point AttackFrom, int xDiff, yDiff)
{
   var pointToAttack = new Point(AttackFrom.X + xDiff, AttackFrom.Y + yDiff);
   return CheckObjectAttackPoint(pointToAttack);
}

Now you can use this method by providing it with attack points:

Point objCurrentPoint = ...; // Currect position is (1;1)
CheckObjectAttackPoint(objCurrentPoint, -1, -1);
CheckObjectAttackPoint(objCurrentPoint, +1, -1);

Upvotes: 3

Galandil
Galandil

Reputation: 4249

Your for loop is executed always exactly just ONCE, your method's code is equivalent to this:

private void CheckObjectAttackPoints(Point AttackPoint) {
    Point ObjectAttackPoint = AttackPoint;
    ObjectAttackPoint.X -= 1;
    int count = 0; //This variable for reading how many tiles are false
                   //Check tiles active and ObjectAttackPoint is exist in List
    if (GameManager.AllPoints.Contains(ObjectAttackPoint)) {
        if (!GameManager.TileColliders[ObjectAttackPoint.X, ObjectAttackPoint.Y].activeSelf) {
            count++;
            ObjectAttackPoints[0] = ObjectAttackPoint;
            Debug.Log(ObjectAttackPoints[0].X + ", " + ObjectAttackPoints[0].Y);
        }
    }
    ObjectAttackPoint.X += 2;

    if (count > 0) {
        Debug.Log("Object can attack " + count + " points");
    }
}

If you want to cycle through all elements of ObjectAttackPoint, you should use

If it's a List:

for (int i=0; i < ObjectAttackPoint.Count;i++)

If it's an array:

for (int i=0; i < ObjectAttackPoint.Length;i++)

EDIT: what's the use of the break condition when i==1 anyway? Even if you extend the for loop, that if will break the loop exactly after two loops (i.e. after it's executed for i == 0 and i == 1). You should explain more in depth what's ObjectAttackPoints.

Upvotes: 4

Madruel
Madruel

Reputation: 61

The error is in the cycle definition:

for (int i=0; i < 1;i++)

This means the code will be executed only once, for i=0. You could fix it like this:

for (int i=0;i<=1;i++)

Upvotes: 2

Related Questions