CODEWITHSUNDEEP
pythonpython-3.x
John C. Jameson
John C. Jameson

Reputation: 13

Index of for loop not iterating correctly

I have a for loop that is supposed to iterate a number of times equal to the length of an array. The loop runs the correct number of times, but the index does not properly increment.

I tried manually incrementing i with i = i + 1, but that did not seem to change anything.

room = [['x','x','x','x','x'],['x','.','.','.','x'],['x','.','.','.','x'],['x','.','.','.','x'],['x','x','x','x','x']]
entities = []

class Entity:
    def __init__(self, x_pos, y_pos):
        self.x_pos = x_pos
        self.y_pos = y_pos

        self.new_x_pos = x_pos
        self.new_y_pos = y_pos
    def Move(self, x_move, y_move):
        self.new_y_pos = self.y_pos + y_move
        self.new_x_pos = self.x_pos + x_move
        if self.CheckCollision(self) is True:
            print("collision")
        if self.CheckCollision(self) is False:
            self.new_x_pos = self.x_pos
            self.new_y_pos = self.y_pos
    def CheckCollision(self, entity1):
        for i in range(len(entities)-1):
            #this loop here. It runs twice, but the value of i is zero for both iterations
            entity = entities[i]
            print(i)
            if entity1.new_y_pos == entity.y_pos:
                if entity1.new_x_pos == entity.x_pos:
                    return True
                    break
            else:
                return False
calipso = Entity(1, 1)
entities.append(calipso)
joseph = Entity(3,2)
entities.append(joseph)
print(entities)
calipso.Move(1,1)

print(calipso.x_pos, calipso.y_pos, sep=' ')

I want i to increment each iteration of the for loop, thus i === 0 for the first iteration and i === 1 for the second iteration. Currently, i === 0 for both iterations and I don't know why.

Upvotes: 0

Views: 150

Answers (2)

Calvin Godfrey
Calvin Godfrey

Reputation: 2359

There's a couple things wrong with your for loop. First of all, if the y positions match but the x values don't, nothing will happen. Barring this case, however, the for loop will always exit after the first iteration because if the positions coincide, return True is called (and by the way, because of the return True, you don't need break after it). And otherwise, it returns False.

So inside Move, CheckCollision is called twice, once for each if statement, which is why i is printed twice, 0 both times.

To fix it, you would have to have CheckCollision return False outside the for loop, so that it checks ALL the entities to make sure it doesn't collide with anything.

The last thing to consider is that you never check that entity1 and entity don't refer to the same object. There are some cases that would result in an entity colliding with itself! Without completely revising the method itself to be like the other answer, the only option to fix that would be some sort of unique id attached to each entity, but revising the method (as in the other answer) is DEFINITELY the better option.

Also, stylistically, method names for classes should always start with a lower case letter (so your methods should be move, checkCollision, etc.).

EDIT: In this specific case, i will never go to 1 anyway. This is because len(entities) is 2, so the for loop will go from 0 to 1, NOT including the end, so no matter what it will stop after the first iteration. But if you had more entities, you would run into the problem described above.

Upvotes: 1

Endyd
Endyd

Reputation: 1279

I've refactored your code so that the checkCollision method works the way I think you meant it. See comments within the code. I changed the name of the methods to start with lower case. 

class Entity:
    def __init__(self, x_pos, y_pos):
        self.x_pos = x_pos
        self.y_pos = y_pos
        self.new_x_pos = x_pos
        self.new_y_pos = y_pos

    def move(self, x_move, y_move):
        self.new_x_pos = self.x_pos + x_move
        self.new_y_pos = self.y_pos + y_move
        if self.checkCollision() is True:
            # We don't update x_pos and y_pos here since collision means entity shouldn't move
            print("collision")
            return False
        else: # You need to use ELSE here so that you don't end up calling checkCollision twice
            # if checkCollision returned a False, then we can update the position of entity
            self.x_pos = self.new_x_pos
            self.y_pos = self.new_y_pos
            return True

    def checkCollision(self):
        for entity in entities:
            # Iterate through every entity other than self and see if their position is the same as self's possible new position
            if entity != self and entity.x_pos == self.new_x_pos and entity.y_pos == self.new_y_pos:
                return True
        # Return false if no collision occurs after checking through every entity     
        return False

calipso = Entity(1, 1)
entities.append(calipso)
joseph = Entity(3,2)
entities.append(joseph)

calipso.move(1,1) # successful move
print(calipso.x_pos, calipso.y_pos, sep=' ') # prints 2 2 after succesful move
calipso.move(1,0) # collision with joseph, fails
print(calipso.x_pos, calipso.y_pos, sep=' ') # prints 2 2, calipso did not move

Upvotes: 1

Related Questions