iminurnamez / pyroller

pygame casino
65 stars 29 forks source link

Games as plugins #176

Closed bitcraft closed 9 years ago

bitcraft commented 9 years ago

I've seen this suggested on the pyroller subreddit. I'd like to make this a reality. Anybody have any insight on modularizing the game states?

MarquisLP commented 9 years ago

Pardon my recent involvement in this project, but I think this feature would be crucial in promoting development of any future minigames. Reading the subreddit post, it is indeed quite daunting to have to scour through so many different files that are key to the rest of the program and actually have to edit them, with the risk of breaking so many other things and all.

The first thing that has to go is the static state_dict over in main.py. Instead, all of the initialization functions for each state should be kept in a list dynamically generated at runtime -- since the minigames included can vary -- which is then converted into a tuple for immutability. Since introducing plugins gives each minigame sort of an 'anonymous' quality, it's logical for the program to refer to them only by index number rather than a specific name. As for the 'key' states that are always included (title_screen, snake_splash), you could use an enum to label their IDs and maintain readability across the other core scripts.

That sounds like a lot for one change, but it's a start. Again, I'm quite new to this effort so feel free to ignore the wall of text above if it isn't feasible. In the meantime, I'll read over the code base and see how else this feature might be implemented; this refactoring is too good to overlook and, dare I say it, ultimately inevitable in the long-term.

bitcraft commented 9 years ago

Welcome! I have to agree...adding new games needs to be easier, and losing all the hard-coded references will be a great start. I think Mekmire designed the state dict and such, maybe we can get his opinion on it here or on the subreddit.

On Tue, Jul 7, 2015 at 8:59 PM, Mark Padilla notifications@github.com wrote:

Pardon my recent involvement in this project, but I think this feature would be crucial in promoting development of any future minigames. Reading the subreddit post https://www.reddit.com/r/PyRollersCasino/comments/2ylrjx/worth_redesigning_games_as_plugins/, it is indeed quite daunting to have to scour through so many different files that are key to the rest of the program and actually have to edit them, with the risk of breaking so many other things and all.

The first thing that has to go is the static _statedict over in main.py. Instead, all of the initialization functions for each state should be kept in a list dynamically generated at runtime -- since the minigames included can vary -- which is then converted into a tuple for immutability. Since introducing plugins gives each minigame sort of an 'anonymous' quality, it's logical for the program to refer to them only by index number rather than a specific name. As for the 'key' states that are always included (title_screen, snake_splash), you could use an enum to label their IDs and maintain readability across the other core scripts.

That sounds like a lot for one change, but it's a start. Again, I'm quite new to this effort so feel free to ignore the wall of text above if it isn't feasible. In the meantime, I'll read over the code base and see how else this feature might be implemented; this refactoring is too good to overlook and, dare I say it, ultimately inevitable in the long-term.

— Reply to this email directly or view it on GitHub https://github.com/iminurnamez/pyroller/issues/176#issuecomment-119398160 .

Mekire commented 9 years ago

I have been mulling this over lately. I do think it will constitute a major change, but it could be worth it. It really depends how much more we reasonably percieve being added to this project. It would also be great if we decide to entertain a similar project in the future (I like the idea of mini retro arcade games). If I get a chance I'll try and hack up an auto-detection prototype and then people can see if they think implementing it would be desired/feasible.

bitcraft commented 9 years ago

That sounds great. Hopefully we can leave each state as-is, just change how the loader deals with them.

MarquisLP commented 9 years ago

Yes, ideally this shouldn't affect anything about the states themselves -- just the way they're organized and accessed!

Mekire commented 9 years ago

I have something pretty rough that I'll push to a new branch later for people to check out. Currently it works by making the lobby state auto detect games that are in a new "games" folder in the "states" directory. There is no loading/unloading going on for the moment; only auto detection, but to illustrate how easy it is to add a game state now, you can get a new one in the lobby screen as simply as adding a new game directory with an __init.py__ that just has:

from data.tools import _State as Game

Of course it won't start when clicked on as that is just the abstract state class though.

It would look something like this for a real implemented game:

from .blackjack import Blackjack as Game

I haven't yet tackled auto-detection on the stats screen.

Mekire commented 9 years ago

plugin branch added. Pretty rough, but very easy to add a new game now in my opinion. Simply copy and rename the folder "template" in the "data/states/games" directory to your game name. The file "image.png", if found in that directory, will be used for the lobby button. If not found a default image is used. All games have the @staticmethod initialize_stats() now. I'm sure there are some broken things (the -S straight flag currently won't work for a start).

bitcraft commented 9 years ago

Nice work. I wonder...would it be possible to make all states (lobby, splash, ect) plugins? That would make the entire game design a bit cleaner, and would facilitate more customization in the future since you could drop in new games and splash, title, ect, without even touching the game code (i"m thinking about your mini arcade games).

MarquisLP commented 9 years ago

That sounds like a neat idea. The only concern would be with how the lobby/splash/etc plugins interface with the engine, since they're integral to the program and lobby is responsible for fetching the game states. If we're to go through with it, I'd imagine those states would need a slightly different format.

How much customization would be involved in lobby/splash/etc, theoretically? Is it purely aesthetic, or would the actual processes be modifiable as well?

bitcraft commented 9 years ago

The way I see it is that game/plugin discovery would get moved to the Scene Manager. The scene manager would have a customizable default state to load on startup. A reasonable default would be a scene/plugin called 'splash screen'. So, on startup, the Scene Manager would load all scenes/plugins it finds, then load the default one. If starting from the splash screen, then the game would have identical flow as it is now, all while avoiding two different implementations of scenes ('plugins', and 'splash/intro/hardcoded').

To give the lobby screen the ability to list games, each scene could have metadata associated with it, stored maybe in the individual scene class. The lobby could simply look for the presence of a class attribute called 'is_game', or something like that. In that case, the lobby would query the Manager for a list of all loaded (or available to load) states, then extract all game states from it, and ultimately list them for selection on the screen.

Mekire commented 9 years ago

It seems to me that the possibility of certain scenes having multiple entry/exit points doesn't transfer well to the plugin idea. The games don't suffer from this as they only ever exit to the lobby. It doesn't seem necessary to make something a plugin if its existence is critical to the program (the lobby for instance). The concept of plugin to me entails the fact that if it isn't found the program still runs which wouldn't be the case here.

bitcraft commented 9 years ago

What I am saying is, that if plugin discovery mech. gets moved to a Scene Manger type class, then all scenes can be treated as plugins and be loaded dynamically. Realistically, no one scene should have stronger dependencies to manager than others anyway. If the Scene manager operates like a stack, as it should, then there would be no problem if states push new states or pop themselves off of it. The default scene then could be really useful, as you could jump right into a scene, bypassing the lobby, all without hacks. I'll go ahead and implement 'all scenes as plugins' and see how it works.

On Thu, Jul 9, 2015 at 8:42 PM, Sean J. McKiernan notifications@github.com wrote:

It seems to me that the possibility of certain scenes having multiple entry/exit points doesn't transfer well to the plugin idea. The games don't suffer from this as they only ever exit to the lobby. It doesn't seem necessary to make something a plugin if its existence is critical to the program (the lobby for instance). The concept of plugin to me entails the fact that if it isn't found the program still runs which wouldn't be the case here.

— Reply to this email directly or view it on GitHub https://github.com/iminurnamez/pyroller/issues/176#issuecomment-120196450 .

Mekire commented 9 years ago

I agree the placement of the discovery mechanism is currently not good or final. At the moment both lobby and the stats screen do the discovery independently which is obviously ugly. I'm not quite convinced that a stack architecture is absolutely necessary for the manager though.

bitcraft commented 9 years ago

Ok. So we keep the 'flip_state' mech., but make everything a plugin? I'm currently knee deep into pyroller ATM, with all scene treated as 'plugins'. its all working except for stats.

On Thu, Jul 9, 2015 at 11:58 PM, Sean J. McKiernan <notifications@github.com

wrote:

I agree the placement of the discovery mechanism is currently not good or final. At the moment both lobby and the stats screen do the discovery independently which is obviously ugly. I'm not quite convinced that a stack architecture is absolutely necessary for the manager though.

— Reply to this email directly or view it on GitHub https://github.com/iminurnamez/pyroller/issues/176#issuecomment-120226704 .

Mekire commented 9 years ago

If you implement it as a stack and it looks cleaner, I'm all for it; I'm just not sure of the necessity. The stats is a bit of a pain. Are you still creating a dictionary of all scenes based on folder name; or some other method?

bitcraft commented 9 years ago

I'm moving your auto discovery code to tools.Control. All states are instanced when found, and an attribute called 'controller' is added to the instance, so that they can be aware of the scene manager 'Control'. Its honestly not a pain, I'm nearly finished; moving the stats init code to each state was the correct thing to do, and saves a lot of time.

In the end, it will operate the same as before, except that the scene name will be the folder name.

On Fri, Jul 10, 2015 at 12:15 AM, Sean J. McKiernan < notifications@github.com> wrote:

If you implement it as a stack and it looks cleaner, I'm all for it; I'm just not sure of the necessity. The stats is a bit of a pain. Are you still creating a dictionary of all scenes based on folder name; or some other method?

— Reply to this email directly or view it on GitHub https://github.com/iminurnamez/pyroller/issues/176#issuecomment-120230003 .

bitcraft commented 9 years ago

All states are now scanned when program starts and are instanced only when used. I will go though and clean up what I can and verify that no bugs have been introduced due to the new state behavior. I still have a couple concerns:

In general memory use is still pretty high, and I'd like to see this run on Android. As we move closer to the point where we can make a binary release, we will need to be vigilant in identifying code that uses too much memory, and reduce those points so we can run on embedded systems. Part of the state changes that I implemented (instance on demand) helps memory use. We can find other parts on change those as well.

Anyway, besides general bug checking and the above concerns, what is blocking us from merging the plugins branch to master?

Mekire commented 9 years ago

After a state is swapped, it gets a reference to the previous state that was running. Is this needed?

I don't think this is really needed actually. Not much harm in letting them get GCed right away as far as I can tell. I had the games getting GCed and recreated on demand previous to the "all states as plugins" change and didn't notice a performance hit, so saving the memory is likely worth it.

A simple change in the import mech of the Control class will remove the need to mess with `init.py, and also allow a folder to contain many states. Any interest in that functionality?

I'm interested in the functionality, though I have questions on implementation. Would all Scene classes need to have the same name as we aren't using the import x as Scene structure? Is it assumed that all files in these folders are scenes or is there a way the proposed auto-detect can differentiate between scene and non-scene .py files? I would also like the games to again be in their own folder as the states directory is getting a bit crowded and I think this could be accomplished with the same change.

Anyway, besides general bug checking and the above concerns, what is blocking us from merging the plugins branch to master?

There is nothing stopping us if everyone is comfortable with it.

bitcraft commented 9 years ago

The 'auto discover' mechanism wouldn't use importlib, instead would inspect each python file and import any classes that are derived from data.state.State (as opposed to modules). It can be accomplished using the imp module in the std lib. That means no special treatment (import x as Scene) in init_.py. It should also work fine without an __init, but I don't know how intra-module imports will be affected (can be tested). Currently, the state name is derived from the folder name, and while I don't want to open that can of worms again, I will say that as long as the folder name is used as the state key (in the Control), it is not possible to allow many states in the same folder.

I should have a PR sometime soon with the new code to remove need to manage init.py.

Mekire commented 9 years ago

I don't care if scenes are stored based on a self.name attribute rather than the folder or loaded module name. However, currently for scenes displayed in the lobby (games) if the self.name attribute doesn't match the folder, the lobby image can't load (it determines the path based on the name).

Mekire commented 9 years ago

There may be more work to be done on the plugins functionality, but for now it has been merged into master.

bitcraft commented 9 years ago

I'm closing this as the plugins branch has been merged, and there is rudimentary support for loading game states at runtime from folders (plugins).