Rearranging Code

I have some classes that communicate with each other. Some of these are:
-PC (player character)
-NPC (non player character)
-GUI
-Combat Manager

The PC class plays all the animations, sound effects, visual effects (particles, lights, shaders), the look of the player avatar (shows/hides armor and weapons) but also keeps track of in-game variables (health, armor, items, action points, position, current map, etc).
The NPC class is basically the same, only it’s controlled by AI not a player (they inherit from a common, abstract Creature class).
The GUI class handles all buttons, icons, items, indicators, options, dialogs, etc.
The Combat Manager controls all NPC moves in combat, it also ‘rolls the dice’ for hits and damage.

The GUI needs to keep a reference to the PC (when say a sword is dragged to the hand slot, the PC class has to update the look of the 3d avatar/actor).
The PC needs to keep a reference to the GUI (when some stats change - like a health drop after getting damaged- the PC needs to call the GUI to update the on screen health indicator)
The Combat Manager needs to keep reference to both the GUI (eg. to show the player that it’s his turn to move) and the PC (for hits, attacks and all that jazz) and all the NPC.
Luckily the NPC don’t need to track anything, they communicate only via the Combat Manager.

When I initiate the GUI I don’t even have a reference to my PC because it’s not initiated yet, so later when I do initiate the PC I have to pass it to the GUI.

I actually have more of these - a Time Manager (updates in-game clock, moves the sun/moon/stars/clouds, handles time of day changes), Music Manager (changes music once a track ends, changes music when entering or leaving combat, changes music on day2night-night2day transitions), Quest Manager (quest status can change via dialog or picked up/dropped items -in GUI, via combat -Combat Manager or when some time passes - Time Manager). Basically most of my classes need to have a handle on most of my other classes…
It works but it’s a conundrum with circular references all the way.

I’m looking for any tips that could make all this more elegant.

I’m not sure that it would work well for a system as complex as yours, but I had a similar problem with a project of mine and found in the end that a single, global “common” class seemed to work well and be at the least fairly elegant.

The basic idea would be that the “common” class keeps references to various important objects as called for – the “world”, the GUI, the player perhaps, and so on – but has no idea what they are, and thus doesn’t have to import their classes. You simply have the relevant classes assign themselves to the relevant members of the “common” class and then access them as called for, perhaps with some checks for both assignment and retrieval.

(I tend to make the references and relevant methods static members of the class, and access the class statically, rather than relying on something instantiating it.)

This allows any given file to access any of these systems without having to worry about circular imports, passing objects as parameters or circular references, as long as you take care to perform whatever validity checks are called for (such as checking that the member isn’t None).

Admittedly, it’s commonly advised that one stay away from globals, but I feel that this is one situation in which a global static object is a reasonable and fairly elegant solution.

Example:

Common.py

class Common():
    world = None

    @staticmethod
    def setWorld(newWorld):
        # Perform whatever checks may be called for...
        Common.world = newWorld

    @staticmethod
    def getWorld():
        return Common.world

    @staticmethod
    def cleanup():
        Common.setWorld(None)

World.py

from Common import Common

class World(object):
    def __init__(self):
        Common.setWorld(self)
        # Further initialisation...

    def doSomething(self):
        # Do something

Player.py

from Common import Common

class Player(object):
    def __init__(self):
        # Initialisation...

    def someAction(self, someParam):
        # This is some action that calls for the "world" to do something.
        Common.getWorld().doSomething()

Hi Wezu, have you done any top level design, like domain modelling, for this project? From your description it sounds like you have a lot unnecessary coupling between objects and/or bloated objects that do too many things. For example, should a “Time_Manager” object really be responsible for moving the sun and moon, etc? Maybe it should just do that one thing that its name implies and simply let other objects handle the time that it supplies them. I know that’s just one example, but there are definitely ways to create systems that are not full of “circular references”. While there’s no one right way something like the following has worked for me with panda3d projects (sorry for the novel but there is no short version):

1. Have a Main Controller obj called something like “GameCtrl” which is a global object that represents the root of your object hierarchy, attach to it the major game objects. Give it a “main_loop” method setup in a task that calls it once per frame. Initialize this object to begin the game.

class GameCtrl:

    def __init__(self):
        self.World = World()
        self.Player = Player()
        self.GUI = Gui()
        self.User_Events_Handler = UEH()
        self.Game_Clock = GlobalClock()
        taskMgr.add(self._main_loop_, "main_loop", extraArgs=[], appendTask=True)

    def _main_loop_(self):
        ...

if __name__ == "__main__":
    game = GameCtrl()
    run()  # Assuming DirectStart has been imported.

2. Each major object (GUI, World, etc.) is responsible for creating the objects that it contains or uses most. Each can also supply its own “main_loop” method if necessary. So for “World” something like:

class World:

    def __init__(self):
        self.Planet = Planet()
        self.Moon = Moon()
        self.NP_Chars = []
        for c in range(50):
            npc = NonPlayerChar()
            self.NP_Chars.append(npc)
            
    def _main_loop_(self):
        ...

3. Each frame “game.main_loop” gets called and initiates a traversal through the object hierarchy with each object responding to input as necessary and giving whatever output is expected of it:

class GameCtrl:
    ...
    def _main_loop_(self):
        dt = self.Game_Clock.getDt()
        user_events = self.GUI.get_user_events()                 # i
        commands = self.User_Event_Handler.get_commands()       # ii
        self.Player._main_loop_(user_events, dt)                # iii
        self.World._main_loop(user_events, dt)                  # iv
        ...

So one frame would roughly entail:

i) Rather than having your program respond specifically to each user event have your GUI object (or another object) listen for user events and gather them into a list, which is then passed on to GameCtrl when called from its main_loop:

class Gui(DirectObject):

    def __init__(self):
        self.current_events = []
        self.accept("w", self.collect_events, ["a"])
        self.accept("s", self.collect_events, ["s"])
        self.accept("esc", self.collect_events, ["esc"])
        
    def collect_events(self, evt):
        self.current_events.append(evt)
        
    def get_user_events(self):    # called every frame from GameCtrl._main_loop_.
        ue = self.current_events
        self.current_events = []
        return ue

ii) GameCtrl then passes these events to the User_Event_Handler object which simply reinterprets these events into command strings based on a key map; this lets you sub in different key maps while maintaining a generic command set.

class User_Events_Handler:

    key_map = {
        'a':"move_forward",
        's':"move_backward",
        'esc':"quit_game"
    }

    def get_commands(self, user_events):
        commands = []
        for evt in user_events:
            if evt in self.key_map:
                commands.append(self.key_map[evt])
        return commands

iii and iv) Now pass the time “dt” and the commands list to each major object in turn (through their “main_loop” method) to be handled directly or passed down to the objects they contain (characters, planets, etc.):

class Player:
    ...
    def _main_loop_(self, commands, dt):
        if "move_forward" in commands:
            self.setPos(self, 0, 1, 0)
        elif "move_backward" in commands:
            self.setPos(self, 0, -1, 0)


class World:
    ...
    def _main_loop_(self, commands, dt):    # Might not need commands, but can't hurt.
        self.Planet._main_loop_(commands, dt)
        self.Moon._main_loop_(commands, dt):
        for npc in self.NP_Chars:
            npc._main_loop_(commands, dt)  

class Planet:
    ...
    def _main_loop_(self, commands, dt):
        increment = self.vector * dt
        old_pos = self.getPos()
        new_pos = old_pos + increment
        self.setPos(new_pos)

This is only a rough outline and you could vary it any way you want (i.e. skip the User_Events_Handler step and just handle events directly) and it might not be possible or desirable to translate your project to such an outline but maybe you can glean some ways of hiding objects from each other. In this case the GUI never needs to reference any other object, it simply provides a way of getting all the current user events, likewise only the World object needs to reference the planets and npcs. Also you can add and remove objects from the main_loop cycle with very few alterations to code in other places. Notice how most objects don’t even call any methods from other objects because the controller object handles most of the messaging.

This is obviously an idealized example and there are many instances in which you’ll need to directly couple at least some objects together, but as a basic rule in an OO system if you start with the most generalized means of communication and then work down to more specific coupling only in necessary cases you can spare yourself a lot of the complexity that has seemed to crop up in your case.

Cslos77 >> if I’d start anew, then that could work. But it’s not a Big Plan Up Front type of project - I have no idea what I could need next monday. The solution seems more problematic then the problem.

Thaumaturge >> I think I’ll have to do it like that. I’ll have all the common elements in one place and accessible in the same way everywhere.