Welcome, Guest. Please login or register.

Pages: [1]
Print
Author Topic: engine.py and world.py  (Read 11526 times)
tZee
Community member

Posts: 190


View Profile Email
« on: August 19, 2009, 10:08:17 AM »

I am very unhappy about those two classes. They two big garbage dumping places under the pretense to be something like a MVC approach.

They're too big, too messy, are wickedly cross-referenced and there is no structure in terms of separating parts that belong together from the rest. They're a mix of data, input handling, graphics display and object loading/handling.
It's horrible! ! !

I want to sort this out. GUI parts belong in a class that handles GUI stuff. Input has to be handled at some point, so it might be this. Data handling should be delegated to a class that is solely responsible for doing that.

And the names... an example:

Code:
def handleMouseClick(self,position):
        """Code called when user left clicks the screen.
           @type position: fife.ScreenPoint
           @param position: Screen position of click
           @return: None"""
        self.gameState.PC.run(position)

The name of that function has nothing to do with what it actually does. When we chose names they should at least hint at what is happening in it - or have a related name.
Same goes for class names. World and Engine are not what their names suggest.

Usually you would have one "master" class which holds all the references to the "delegate" classes, which do the actual work. The master class only delegates/organizes the work, maintains the overview.
E.g. we would have a master class "Game", which is an input listener and holds the delegate classes "GUI", "Loader", "GameState"/"ObjectManager", "Map" etc.
According to the input it would trigger things/delegate work.

What do you think about this? What is your opinion of world.py and engine.py?
Logged

tZee
Community member

Posts: 190


View Profile Email
« Reply #1 on: August 19, 2009, 10:27:48 AM »

Just saw the Trac tickets about the GUI. That's a step in the direction I suggested here, but needs to be done for more than just the GUI stuff. Smiley
Logged

saritor
Guest
« Reply #2 on: August 19, 2009, 02:07:24 PM »

I agree fully with you. This would not only drastically improve our ability to discuss the code itself, it would make learning it much easier for newcomers like myself. I still don't have a very strong grasp on what these guys are doing, and its because they try to do so much.
Logged
Kaydeth
Community member

Posts: 185



View Profile Email
« Reply #3 on: August 19, 2009, 02:11:32 PM »

Function naming is a bit of a tricky thing. Basically it's a style decision. I don't think we need to concentrate on re-naming everything. I would rather handle it in a more passive way. If you are working in an area that has a horribly named function then think about renaming it. We can't be too sensitive about it or else we'll start renaming functions all the time and it may confuse developers when they can't find the functions they were working with before.

I agree that the World and Engine class are probably a little bloated. And it also can be confusing when referring to "engine" since the FIFE code is also referred to as the engine. Again I'd rather do things piece meal though. As you said we are already moving GUI related code to the Hud class so if you can think other things to move out then please make th reccomendation. For instance what data handling aspects would to move out to another class and what would you call the new class?

If we keep moving things out of the Engine Class we can then shape into Game Class as you mention which sounds like a good idea to me.
Logged
tZee
Community member

Posts: 190


View Profile Email
« Reply #4 on: August 19, 2009, 02:41:27 PM »

All the addObject addPC functions, that are currently present in both world.py and engine.py. I'd like to have a class that in the background manages our data layer and the FIFE objects. (Ensuring that they are in sync, updated properly. That new objects are added to the layers peroperly etc. etc.)
Right now this handling is decentralized and quite confusing.

As for the naming thing: I agree with you. I didn't want to make this a separate task. I'd rather have the developers think a bit about the name, when they add new functions/classes. And, like you said, if they encounter really bad names, change them.
Logged

Atomic
Community member

Posts: 31



View Profile
« Reply #5 on: August 19, 2009, 03:27:06 PM »

I agree completely on World and Engine.

I also think having a high-level architecture design would help in refactoring and developing our class structure.  From the research I've done, it seems that the majority of games have a very similar overall architecture.  Here's a quick and dirty drawing of a lot of the commonalities that I've seen:



I think if we had an agreed upon architecture then we could review the existing code / new proposals and determine if a class fits within one of the subsystems well or if it crosses too many boundaries and should be refactored.

I definitely agree on the naming.  Whatever new restructuring we decide upon, I think that at a minimum the Engine class in engine.py should be renamed.  Its too easy to confuse that and fife.Engine.  If you look at the various constructors for a few the other classes, they have the format __init__(self, engine, world, model).  Most of the time engine = fife.Engine and model = engine.Engine.  Very confusing.
Logged
fasmith718
Community member

Posts: 5


View Profile
« Reply #6 on: August 19, 2009, 09:38:33 PM »

I definitely agree on the naming.  Whatever new restructuring we decide upon, I think that at a minimum the Engine class in engine.py should be renamed.  Its too easy to confuse that and fife.Engine.  If you look at the various constructors for a few the other classes, they have the format __init__(self, engine, world, model).  Most of the time engine = fife.Engine and model = engine.Engine.  Very confusing.

Agreed.  Comments in other files (such as world.py) refer to 'engine' and it's never clear whether they mean engine.py or are referring to something in FIFE.

Furthermore, I like the diagram that Atomic has presented and think it might be useful to use it as a basis for a package structure.  As it is parpg has almost everything thrown into the scripts folder with only two vaguely named folders below.  It may be too much to ask that files be renamed, aggregated and put into sensibly named modules in the short term, but it's something we should move towards.

As to the engine.py class specifically... From my experience, the game engine should initialize the objects and start the gui loop.  Here we have the engine class receiving events and moving the character around.

I'll have to delve more into the architecture before i can make better criticism, but the OP is definitely correct- there is a problem here and there will have to be a lot of refactoring to make it make sense.
Logged
Kaydeth
Community member

Posts: 185



View Profile Email
« Reply #7 on: August 20, 2009, 04:03:50 PM »

Ok, I'm hearing consensus on renaming the Engine class. So what do we rename it to?

"Game"? Any other suggestions?
Logged
fasmith718
Community member

Posts: 5


View Profile
« Reply #8 on: August 20, 2009, 05:34:27 PM »

Maybe we could change it to something like PARPGEngine.py or something to distinguish it from files inside of FIFE, but then it would be important to make it a more tightly focused file.  It could be responsible for loading the classes, "wiring" them together, and starting the main loop.

For instance, it should not be handling mouse clicks- that should be part of a GUI file.

In my vision it should load objects onto the map, associate the map with the GUI and then wait until a map is changed to reload and prepare a level or area.  If there is a separate object tracking game logic, etc. then that should be handled similarly.

That's from my experience at least- I've not developed games, but I have developed GUI driven programs before.
Logged
tZee
Community member

Posts: 190


View Profile Email
« Reply #9 on: August 20, 2009, 10:03:46 PM »

A game is not really GUI driven.

The loading of the map should the map class itself (in case of decentralized loading functions) or a loader class handle. The engine/game class is actually a good point to inherit from the input listener, as it will possess all the game data/class and can delegate the work. If we had a separate object just for listening it would need the references to the other objects to delegate/trigger things.

PARPGEngine is, in my opinion, not a better name than engine. There exists already a PARPG class and because people are lazy they would not write out the name fully each time they talk about it and it would require further explanation again, which engine class is meant.
Logged

Kaydeth
Community member

Posts: 185



View Profile Email
« Reply #10 on: August 24, 2009, 01:52:19 PM »

What if we rename FIFE engine instances as something like "graphics_engine" or "gfx_engine"?

EDIT: Shoot that's not really going to work because we are stuck with the "engine" name from the ApplicationBase class.
« Last Edit: August 24, 2009, 02:01:42 PM by Kaydeth » Logged
mvBarracuda
Admin
Community member

Posts: 1308



View Profile Email
« Reply #11 on: August 25, 2009, 04:52:32 PM »

FIFE is not really a graphics engine either. FIFE provides sound, input handling, GUI, pathfinding, etc.
Logged
Kaydeth
Community member

Posts: 185



View Profile Email
« Reply #12 on: August 25, 2009, 05:03:40 PM »

FIFE is not really a graphics engine either. FIFE provides sound, input handling, GUI, pathfinding, etc.

True. No insult intended Smiley.
Logged
Kaydeth
Community member

Posts: 185



View Profile Email
« Reply #13 on: August 26, 2009, 01:40:06 PM »

kind of silly that we can't think of good name.

How about "GameData"? The engine class is already referred to as "data" by the World class. And since it's suppose to be the Model in an MVC architecture it is where all the data is managed.
Logged
mvBarracuda
Admin
Community member

Posts: 1308



View Profile Email
« Reply #14 on: August 26, 2009, 02:51:07 PM »

True. No insult intended Smiley.
No offense taken :-) Just wanted to underline that renaming the FIFE engine instances to graphics_engine would be misleading due the nature of FIFE.
Logged
Pages: [1]
Print
Jump to: