View Single Post
Old Oct 28th, 2005, 3:37 PM   #4
Arevos
Programming Guru
 
Arevos's Avatar
 
Join Date: Aug 2005
Location: England
Posts: 1,499
Rep Power: 5 Arevos is on a distinguished road
That's some pretty fine programming - but don't take it the wrong way if I offer some constructive criticism.

In check_collision(), you have a for-loop like this:
for collision in range( 0, len(collisions) ):
   if collisions[collision][0] == "rectangle":
      ...
The 'range' function appears to be redundant. A for-loop iterates over each item in a list or generator. A more "pythonic" approach could be:
for collision in collisions:
   if collision[0] == "rectangle":
      ...
Another point of redundancy are the "rectangle" and "point" strings. Why not filter on length? You know that a point has two values (x and y), and that a rectangle has four (x1, y1, x2 and y2):
for collision in collisions:
   if len(collision) == 4:
      # rectangle stuff
   elif len(collision) == 2:
      # point stuff
There's also a minor bug in your program:
for collision in range( 0, len(collisions) ):
   if collisions[collision][0] == "rectangle":
      ...
   elif collisions[collision][0] == "point":
      ...
   else:
      if collision == len(collisions)-1:
         c_o_l_l_i_s_i_o_n = 0
         return c_o_l_l_i_s_i_o_n ##0 means no collision
The function will only return 0 if and only if the last element of the sequence is not a rectangle or a point. What you probably want to do is place the c_o_l_l_i_s_i_o_n after the for-loop:
for collision in range( 0, len(collisions) ):
   if collisions[collision][0] == "rectangle":
      ...
   elif collisions[collision][0] == "point":
      ...
c_o_l_l_i_s_i_o_n = 0
return c_o_l_l_i_s_i_o_n ##0 means no collision
This ensures that the function will return 0 if there are no collisions.

Another redundancy is your use of the c_o_l_l_i_s_i_o_n variable. You don't need it. Instead of:
c_o_l_l_i_s_i_o_n = 0
return c_o_l_l_i_s_i_o_n
Instead, try:
return 0
Or, better yet, instead of using 0 and 1, use False and True:
return False
Also, if statements; you don't need to embed them. You can use logical and:
if collisions[collision][0] == "rectangle":
   if character_location[0] > (collisions[collision][1]) and \
         character_location[0] < (collisions[collision][3]) and \
         character_location[1] > (collisions[collision][2]) and \
         character_location[1] < (collisions[collision][4]):
      c_o_l_l_i_s_i_o_n = 1
      return c_o_l_l_i_s_i_o_n
So, after we done and removed these bits of redundancy in check_collision, we get this:
def check_collision(collisions, character_location):
   for collision in collisions:
      if len(collision) == 4 and \
         if character_location[0] > collision[0] and \
               character_location[0] < collision[2] and \
               character_location[1] > collision[1] and \
               character_location[1] < collision[3]:
            return True
      elif len(collision) == 2 and \
         if character_location[0] > (collision[0] - 5) and \
               character_location[0] < (collision[0] + 5) and \
               character_location[1] > (collision[1] - 5) and \
               character_location[1] < (collision[1] + 5):
            return True
   return False
In the above function, collisions are either points of two numbers, ie. [x, y], or rectangles of three numbers, ie. [x1, y1, x2, y2].

But we can get it even smaller still, by using pygame's Rect class. The code below defines collisions in a slightly different fashion. (x, y) is a point, whilst [x, y, width, height] is a rectangle:
from pygame import Rect

def check_collision(collisions, (x, y)):
   character_rect = Rect(x - 5, y - 5, 10, 10)
   
   for collision in collisions:
      if len(collision) == 4:
         if character_rect.colliderect(Rect(*collision)):
            return True
      if len(collision) == 2:
         if character_rect.collidepoint(*collision):
            return True

   return False
Rect objects can be created like so: Rect(x, y, width, height). Once created, they have several handy collision functions. Why slave away when you can let pygame do the hard work for you?

Indeed, if you wanted to take a more object-orientated approach, you could wrap your character in a Sprite class - but if I go on this way, we'll be here forever, so I'll cut my post short.
Arevos is offline   Reply With Quote