3

I'm new to stackoverflow, but was hoping for a little insight from more advanced programmers. I am switching majors to Computer Science next semester and am taking an intro class learning some beginner's Python programming. I have already finished the program below (the assignment was to make a program that draws ovals on the window surface by filling in some of the professor's code, not too bad at all) but I wanted to add a little something extra: As you can see, I have the color of the ovals set to be random, but it stays the same until the program is restarted entirely i.e. all of the ovals are that particular color for the length of the program. With the code written the way it is, I can't figure out a way to get the color to change for each oval. Keep in mind, this is all for kicks, but if anyone's feeling especially helpful or creative, I'm curious to see what you have to say. Let me know if I can expound on anything. Thanks!


import pygame, random, sys

WINDOWWIDTH = 700
WINDOWHEIGHT = 700
BACKGROUNDCOLOR = (150,160,100)
#A different color every run
OVAL_COLOR = (random.randint (0,255),random.randint (0,255),
                    random.randint (0,255))

pygame.init()
windowSurface = pygame.display.set_mode((WINDOWWIDTH, WINDOWHEIGHT))
pygame.display.set_caption("Mobile Ovals")

#The draw variable is used later to indicate the mouse is still pressed
ovals = []
completedOvals = []
finished = False
draw = False
startXY = (-1, -1)

while not finished:
    for event in pygame.event.get():
        if event.type == pygame.QUIT or (event.type == pygame.KEYUP and
                    event.key == pygame.K_ESCAPE):
            finished = True

        elif event.type == pygame.KEYDOWN:
            pressed = pygame.key.get_pressed()
            if pressed[pygame.K_F4] and (pressed[pygame.K_LALT] or
                        pressed[pygame.K_RALT]):
                finished  = True

        elif event.type == pygame.MOUSEBUTTONDOWN:
            startXY = event.pos
            draw = True

        elif event.type == pygame.MOUSEBUTTONUP:
            draw = False
            for oval in ovals:
                completedOvals.append (oval)

        if draw == True:
            del ovals [:]

            #The above function ensures only one oval is onscreen at any given time
            endXY = event.pos
            width = (abs(endXY[0]-startXY[0]))
            height = (abs(endXY[1]-startXY[1]))

            #The code below allows the user to drag any direction
            if endXY[0] < startXY[0]:
                left = endXY[0]
            else:
                left = startXY[0]
            if endXY[1] < startXY[1]:
                top = endXY[1]
            else:
                top = startXY[1]

            ovals.append (pygame.Rect (left, top, width, height))

            windowSurface.fill(BACKGROUNDCOLOR)

            for oval in ovals:
                pygame.draw.ellipse(windowSurface, OVAL_COLOR, oval)

            for completedOval in completedOvals:
                pygame.draw.ellipse(windowSurface, OVAL_COLOR, completedOval)

            pygame.display.update()

pygame.quit()
David Cain
  • 16,484
  • 14
  • 65
  • 75

2 Answers2

2

Your problem is quite simple. You set OVAL_COLOR once. But every time you make reference to the variable OVAL_COLOR, you're not creating a new random color, you're re-using the RGB color that was randomly generated when you created the variable.

Now, the way your program is structured, you maintain a list of all complete ovals that you're re-drawing every time the draw variable is set to true. If you place the OVAL_COLOR variable inside the for loop, you will update the color with every mouse movement, changing the color of the oval being drawn, as well as the color of all the old ovals being re-drawn.

The solution to have a new random oval color is to set the variable OVAL_COLOR when the mouse button goes down. That way, the oval color won't change as you drag the mouse to adjust the oval. But, given the current structure of the program, you'll need to save the oval colors assigned to completed ovals, or you'll still have the oval color change each time.


When the mouse button is pressed down, we want a new random color for our circle. Generate a random value, which will be used every time the circle is re-drawn.

    elif event.type == pygame.MOUSEBUTTONDOWN:
        startXY = event.pos
        OVAL_COLOR = (random.randint (0,255),random.randint (0,255),
                            random.randint (0,255))
        draw = True

When the mouse button is released, save the coordinates for the oval, along with the color that it was drawn with.

    elif event.type == pygame.MOUSEBUTTONUP:
        draw = False
        # print len(ovals) # (always ==1)
        completedOvals.append ((ovals[-1], OVAL_COLOR)) 

When we iterate through these completed ovals, draw them with the same color each time.

        for (completedOval, color) in completedOvals:
            pygame.draw.ellipse(windowSurface, color, completedOval)
David Cain
  • 16,484
  • 14
  • 65
  • 75
  • 1
    Exactly. The problem is I can't figure out how to fix that problem. I tried including the OVAL_COLOR constant within the loop, but then it just constantly changes the colors of the ovals enough to merit a seizure. I can't figure out where to place the color code so that instead of "for all completedOvals" it only changes the color of each oval as it is drawn. –  Apr 14 '12 at 23:34
  • I guess it would have to involve modifying or removing the "for all ovals" because with that code, every oval, drawn or being drawn, changes colors. –  Apr 14 '12 at 23:39
  • Ah, you beat me to it. I tried that, but again that just changes the color constantly. Hm... –  Apr 14 '12 at 23:40
  • Well, this is your problem, @JeffArcher: You're using a while loop, which will indefinitely repeat until the variable `finished` is true. So it'll re-generate a random oval color each time they're drawn. Which is not what you're aiming for, I gather? *edit:* that may not actually be what's going on- your indentation obscures how the loop's actually functioning. – David Cain Apr 14 '12 at 23:42
  • Right. So if I wanted to change the color, it would have to involve re-writing the code entirely so that instead of running on the while loop it runs on some other method. I just wanted to make sure that there wasn't some way to do it with the code as-is. Looks like I have a good challenge in front of me :-). –  Apr 14 '12 at 23:44
  • No that is what's going on. While finished = False, the entire program runs. So, I may have to just rewrite the way the program carries out. –  Apr 14 '12 at 23:48
  • I updated my response- it should do what you're looking for. A tip for the future- Next time you post, please be sure that the indentation is correct so others don't need to correct it. As you know, indentation is incredibly important in Python; I had to re-indent your code just to test it and run it. – David Cain Apr 15 '12 at 00:07
  • My apologies, I'll edit it right now. Thanks for all the help! –  Apr 15 '12 at 14:14
1

Create a simple Oval() class, that contains it's color, and size.

import pygame
from pygame.locals import * 

class Oval(object):
    """handle, and draw basic ovals. stores Rect() and Color()"""
    def __init__(self, startXY, endXY):
        self.color = Color(random.randint(0,255), random.randint(0,255), random.randint(0,255))
        self.rect = Rect(0,0,1,1)
        self.coord_to_oval(startXY, endXY)

    def draw(self):
        pygame.draw.ellipse(windowSurface, self.color, self.rect)

    def coord_to_oval(self, startXY, endXY):
        width = (abs(endXY[0]-startXY[0]))
        height = (abs(endXY[1]-startXY[1]))

        #The code below allows the user to drag any direction
        if endXY[0] < startXY[0]:
            left = endXY[0]
        else:
            left = startXY[0]
        if endXY[1] < startXY[1]:
            top = endXY[1]
        else:
            top = startXY[1]

        self.rect = Rect(left, top, width, height)

# main loop
while not finished:
    for event in pygame.event.get():
       # events, and creation:
       # ... your other events here ...

       elif event.type == MOUSEBUTTONDOWN:
            startXY = event.pos
            draw = True

        elif event.type ==MOUSEBUTTONUP:
            # on mouseup, create instance.
            endXY = event.pos
            oval_new = Oval(startXY, endXY)
            completedOvals.append(oval_new)

        # draw them:
        for oval in ovals:
                oval.draw()
        for oval in completedOvals:
                oval.draw()

I mostly left out your non-completed ovals. Was that to show the size before clicking?

ninMonkey
  • 7,211
  • 8
  • 37
  • 66
  • Your code has a number of syntax errors (the Oval class does not extend the class 'self', for instance). Also, this approach only draws the oval when you release the mouse button -the original purpose was to be able to scale an oval on-screen before adding it to the canvas. – David Cain Apr 15 '12 at 23:21
  • Thanks for the additional option, monkey! An interesting take, too, though I like the simplicity of David's code. If I had to rewrite the code I might use yours. –  Apr 16 '12 at 00:35
  • @David: edited to fixed Oval(object). You may add the preview, by having a single oval instance: set `topleft` on MOUSEDOWN, and width,height on MOUSEMOTION events. delete/set to none on MOUSEUP. – ninMonkey Apr 16 '12 at 02:04
  • @JeffA., I'd like to note that my approach was a simple solution that worked with the code you had supplied to you (and didn't rely on any programming concepts you haven't covered yet). Monkey's approach is much better design - what would happen if you wanted to draw many different kinds of shapes, or have objects drawn in different ways? My code would become unreadable very quickly, whereas an object-oriented approach like monkey's would be clean and readable. – David Cain Apr 16 '12 at 05:12