PyBoy
PyBoy copied to clipboard
Baekalfen
Add easy way to add plugins (game wrappers) to Plugin Manager
Hello! I've been attempted to add the WIP Pokemon Red Plugin to my local project, but I'm finding it's quite difficult due to the way the PyBoy and PluginManager classes are structured. Would you be willing to accept a PR that makes the design more modular, and allows the user to something like the following?
game = pyboy.PyBoy(PATH, game_wrapper=True)
game.plugin_manager.add_plugin(PLUGIN)
Additionally, inside the plugin manager, there is a lot of .enabled() calls from plugins, but it appears as though that function will return the same result everytime. This can be seen by the fact that the PluginManager class has extra attributes to cache the result for all of them.
# example:
self.window_sdl2 = WindowSDL2(pyboy, mb, pyboy_argv)
self.window_sdl2_enabled = self.window_sdl2.enabled()
self.window_open_gl = WindowOpenGL(pyboy, mb, pyboy_argv)
self.window_open_gl_enabled = self.window_open_gl.enabled()
I personally think this would be structured better if the enabled function in each plugin was changed into a _enabled helper function that is called in the initializer and set to an enabled attribute, so the attribute .enabled would be available and these extra (plugin name)_enabled attributes wouldn't be needed.
If there's conflicts with evaluating enabled state before hand, that's okay as that doesn't impact the user at the end of the day. What does though, is the ability to add custom plugins, so let me know what you think about these suggestions! :)
Would you be willing to accept a PR that makes the design more modular
Absolutely! But the problem is, that Cython is very strict on what you can do in runtime vs. compile time. It is definitely possible to make all of this more dynamic, but I initially postponed doing it, because it requires us to dynamically load external compiled modules.
Additionally, inside the plugin manager, there is a lot of .enabled()
This is related to Cython. Again, it's difficult to do tricks in runtime while keeping the Cython performance. But it would be awesome to see it become more dynamic without losing significant performance (slight performance hit is acceptable).
Have a look at manager_gen.py. That's the code that generates all the _enabled = ... code.
Long-term goal would be to install plugins from other PyPi packages -- say pip install pyboy-pokered-wrapper -- and have it available. Inspiration could be found in pytest on this.
Do you mean installing the package makes it automatically available, or you have to add it? Regardless, I think this could be a step in the right direction for the time being. If I made these changes would you accept the PR?
If you can refactor the plugin code to dynamically locate and load plugins at run-time while using Cython -- while not sacrificing any meaningful amount performance -- then I'll definitely accept the PR.
You can start by dynamically loading the plugins in the pyboy/plugins/ directory -- likely by deleting manager_gen.py and modifying manager.py. We can wait until a second iteration to add plugins from third-parties.
And in a third iteration, we can possibly look at locating plugins installed from PyPi packages. As inspiration, in pytest, you'll see it listing third-party plugins that it found (notice xdist-2.5.0 installed with pip install pytest-xdist):
$ pytest code
===================================================================== test session starts ======================================================================
platform darwin -- Python 3.10.6, pytest-7.1.3, pluggy-1.0.0
rootdir: /Users/mads
plugins: xdist-2.5.0, forked-1.4.0
collected 0 items
If you can refactor the plugin code to dynamically locate and load plugins at run-time while using Cython -- while not sacrificing any meaningful amount performance -- then I'll definitely accept the PR.
You can start by dynamically loading the plugins in the
pyboy/plugins/directory -- likely by deletingmanager_gen.pyand modifyingmanager.py. We can wait until a second iteration to add plugins from third-parties.And in a third iteration, we can possibly look at locating plugins installed from PyPi packages. As inspiration, in
pytest, you'll see it listing third-party plugins that it found (noticexdist-2.5.0installed withpip install pytest-xdist):$ pytest code ===================================================================== test session starts ====================================================================== platform darwin -- Python 3.10.6, pytest-7.1.3, pluggy-1.0.0 rootdir: /Users/mads plugins: xdist-2.5.0, forked-1.4.0 collected 0 items
sounds good
It's to access ROMs used for tests, but only I have the key as to not distribute the ROMs illegally
You should be able to run the other tests without problems
is there a way I can make it skip the tests that require it? a lot of tests are failing with that being the error.