I'm working on a 2D game engine using pygame. Right now I have drawing/sprites, collision detection/setting, and moving in the GameObject class (base class for all other objects). Is this bad practice? I've read some articles that discourage this, but I don't understand why such things should be separated if they all deal with the GameObject class.

class GameObject(pygame.Rect):
    def __init__(self, location, size, 
        tag, current_sprite=None, sprites=None):
        super().__init__(location, size)
        self.tag = tag
        self.current_sprite = current_sprite
        self.sprites = sprites
        self.collisions = {}

    def draw(self, surface):
        surface.blit(self.current_sprite, self)

    def move(self, x_distance, y_distance):
        self.x += x_distance
        self.y += y_distance

    def set_all_collisions(self, keys, *groups):
        """Map each key to a list of Rects who collided
           with the GameObject.
        """

        for key, group in zip(keys, groups):
            indexes = self.collidelistall(group)
            if indexes: 
                self.collisions[key] = [group[index] for index in indexes]
            else: 
                self.collisions[key] = [GameObject((1000, 1000), (32, 32), 'dummy')]

Ive come to realize it is not about what is "right". There is no "right" way to do anything, there are however design patterns and certain principles you can use to guide your development. You are currently setting yourself up to create a super class, which is generally frowned upon. Whether that is correct or not depends on your end goal.

General knowledge puts your current setup in an unfavorable state and most people will tell you that. Look into SOLID to get yourself started. You have already set yourself up at this point to break every single rule of good design theory.

It may very well work out in your particular case. If this works out for you, I wouldn't worry about it too much.

Still, I would personally discourage putting collision and drawing logic in the 'basic' GameObject class if you can avoid it. The reason being pretty simple: what if you want to have an object that is not involved in the collisions? Maybe there's purely aesthetical objects in your game, that shouldn't be involved in any of that.

What if you want some kind of invisible trigger that opens a door on the other side of the level? Having a 'Visible' or 'IsVisible' flag could solve this (or having no sprite set), but why does a GameObject that does not require this rendering logic at all have it in the first place? The same goes for objects that should not be involved in collisions: why does it have collision logic, if it is never participating in such? It kind of defies why you were using inheritance in the first place: so you can inherit all the properties and logic from the GameObject. I'm even leaving separation of concerns etc. out of this.

Preferably, your GameObject class is meant for the properties that all GameObjects have. Not just a part of them while having them inherit from GameObject anyway. Often, this breaks down to something like just a position, rotation and possibly a scale. Often, there isn't that much that is shared by literally all of the objects in your game.

It's still no easy thing to solve however, definitely not with using just inheritance, which is why I said that if it's not forming a problem for you right now, you probably shouldn't worry about it. If you start to struggle to 'get around everything the GameObject is doing', then you might want to reconsider your approach.

In Python there is no need for a single GameObject class, in fact, objects don't even need to be related through inheritance at all.

class GameObject:
    def __init__(self, location, size, 
        tag, current_sprite=None, sprites=None):
        self.location = pygame.Rect(location, size)
        ...

# this class only supplies an interface for other classes to implement
class Behavior:
    def __init__(self):
       ...
    def update(self, gameObject, deltaTime):
       ...
    # you may also add other functions to react to collision events

class EnemyBehavior(Behavior):
    def __init__(self):
       super().__init(self)
    def update(self, gameObject, deltaTime):
       ...

class GameObject:
    ...
    def update(self, deltaTime):
       this.behavior.update(self, deltaTime)

I don't know your game. All I can do is give you some general advice.

Do you have game objects that do not require all of these features? If not, then the features that are not universally necessary probably don't belong in that class.

It looks like this is becoming a "god class". It does everything. If your game is relatively complex, having a "god object" might become a pain to manage. It might help to separate these features into their own classes, and then have your "god class" inherit from these classes, or have each instance contain an instance (or pointer to an instance) of these classes. The latter version is generally the best; it helps with many common situations, like having multiple game objects with the exact same physical appearance then can share an instance of a "Visual" class.

As mentioned by nfries88, you have created a Higgs Boson god particle class.
Why do you have sprites on your base class? All objects must necessarily be drawn? What about hidden items that are invisible?
Why do they all have collision data? What about fake walls through which you can walk like in Perfect Dark™ and GoldenEye 007™?

Why would a single object be managing all these properties? Existence (GameObject), collision, and graphics are 3 unrelated features that should be managed by 3 separate classes.

A GameObject is simply a thing that exists. It has (not is—don’t inherit from pygame.Rect) a position/scale, and rotation. This is the only thing all objects in the game have in common. Everything else needs to be separated logically.

L. Spiro

As mentioned by nfries88, you have created a Higgs Boson god particle class.
Why do you have sprites on your base class? All objects must necessarily be drawn? What about hidden items that are invisible?
Why do they all have collision data? What about fake walls through which you can walk like in Perfect Dark™ and GoldenEye 007™?

Why would a single object be managing all these properties? Existence (GameObject), collision, and graphics are 3 unrelated features that should be managed by 3 separate classes.

A GameObject is simply a thing that exists. It has (not is—don’t inherit from pygame.Rect) a position/scale, and rotation. This is the only thing all objects in the game have in common. Everything else needs to be separated logically.

L. Spiro

All very good info, thanks everyone! So lets say I'm creating a new Player class. My current approach is now to have a few abstract base classes (Renderer, Collider, Mover) which I then use through inheritance to create PlayerRenderer, PlayerCollider, and PlayerMover classes. Then create an actual general Player class which inherits from GameObject and is composed of PlayerRenderer, PlayerCollider, and PlayerMover objects. Does that sound like a good approach? And I'm assuming any member functions of PlayerRenderer, PlayerCollider, and PlayerMover would need to receive a Player object as an argument or perhaps the objects themselves need references to the Player?

Then create an actual general Player class which inherits from GameObject and is composed of PlayerRenderer, PlayerCollider, and PlayerMover objects. Does that sound like a good approach?

And I'm assuming any member functions of PlayerRenderer, PlayerCollider, and PlayerMover would need to receive a Player object as an argument or perhaps the objects themselves need references to the Player?

And I'm assuming any member functions of PlayerRenderer, PlayerCollider, and PlayerMover would need to receive a Player object as an argument or perhaps the objects themselves need references to the Player?

Why should they need any data from the Player class? They should hold as much relevant data as necessary to perform their job within each instance of the class. If in some corner case you forsee them needing a reference to Player, maybe it would be appropriate to store a reference to Player as a class member too.

My initial thinking is that in order to draw, move, or detect collisions for the player you need access to the players location/size/rect.