ExposureSoftware / TEC-Client

A client for the Skotos prose RPG The Eternal City
2 stars 3 forks source link

[WIP] Exit indicator fixes, plugin framework. #52

Closed PatrickDattilio closed 8 years ago

PatrickDattilio commented 9 years ago

Made some fixes for exit indicators.

Started a plugin framework, needs some work for a release, but it works as follows currently: For a plugin to be loaded it must:

  1. Be located in the plugin directory
  2. End in .py
  3. Contain a class named "Plugin"

At this point the plugin will be loaded, and we will search for method names which match to API events. If a plugin implements any of the event listener methods below, it's added to the endpoint's listener list. When we receive an API event, if the plugin is currently enabled, we pass along the update.

API Event Listeners

"override" any/all of these methods within your plugin to receive the desired event.

All loaded plugins are displayed from the "Plugins" dropdown. All are enabled (denoted by the checkmark) by default, click an enabled plugin to disable it.

marshall-davis commented 9 years ago

I see commits after our discussion but the diff is still showing the pprint and removable of the window icon.

I'm going to assume we don't have test cases to go over the block of code that pprints.

PatrickDattilio commented 9 years ago

This is definitely [DO NOT MERGE], just want your feedback as I'm going along!

I'm currently working on a few more of the basic API hooks before I tackle how to handle plugins in production.

Currently they work by recursively scanning the the plugin_manager/plugins directory for .py files, checking if they have a Plugin class, and then loading them. I've never made an msi/windows executable from a python script before, is it trivial to add a folder in which a user could dump plugins? I'm open to suggestions, I just figure if it works for WoW it's probably good enough for us.

marshall-davis commented 9 years ago

The terminology here for your API endpoints is kind of strange. An API endpoint is always available, even if it returns nothing. What we we are dealing with here are events. I think you're describing registering event listeners and promising we will fire these events at the given times.

Event based plugins and hooks are great idea in my opinion though.

PatrickDattilio commented 9 years ago

Yes! Events/EventListeners! This is what happens when I write documentation at 2 am..

I added two more hooks (previously hardcoded/required, but no longer) set_send_command and set_echo, which let you request the ability to send commands to the server and to the client respectively.

marshall-davis commented 9 years ago

Checked out the branch and did some playing with it. I did some ludus combat and some social interactions and I saw no crash or obvious issues I think this would be the cause of.

I do however see some color issues and such that are present in other branches. I've created a separate issue for that and intend to sort those out relatively soon.

marshall-davis commented 8 years ago

:+1:

PatrickDattilio commented 8 years ago

Tonight I might try and add a "preference" folder (like the resource folder) to the installer. This will be the place we always look for new plugins. Anything dropped in there should just work!

marshall-davis commented 8 years ago

I'd call it plugins unless you were thinking like local\plugins type hierarchy. Doing this should not be that big a deal, I think I have some experience with it.

Also, if the conflicts are resolved I'll merge this branch in.

PatrickDattilio commented 8 years ago

Ah, that was a typo, should meant to say 'plugins'. I updated the build script to keep the plugins directory with the example script. Feel free to merge, but I should probably persist the on/off state of the plugins to disk, that way we can default the example to off. I'd default them ALL to off, but it feels nicer to just drop in a new plugin and have it automatically show up.

marshall-davis commented 8 years ago

OK, I haven't looked into what was already done with this but I imagine something could be like:

1) On startup load all files in plugins/ generating a list of names. 2) Update the config.ini file with a plugins section.

[plugins]
4_part_courses.py = true|false
3_part_courses.py = true|false

3) On opening the Plugins dialog from the menu bar display a list or parsed names (or possibly a name given from with the script, or __init__.py if the plugins can be full modules) with check boxes to select on|off. 4) On change or on closing the dialog write this configuration back to the config.ini file. 5) Additionally, the dialog should have a Load or Refresh button to force a re-read of the directory, updating the list. 6) Use this to load plugins on demand.

marshall-davis commented 8 years ago

Good god there are a lot of conflicts. Any way in the future we do pull requests into the dev branch?

PatrickDattilio commented 8 years ago

We do 1, 3, 5/6 right now. I was thinking of doing a plugin_manager_config.ini, as to not pollute the config.ini with plugins.

After this is done, I'll pull the upstream and you should be able to merge without issue.

PatrickDattilio commented 8 years ago

Want me to move this to dev? It's basically 0 more work, just a different pull request.

marshall-davis commented 8 years ago

Nah, almost got it resolved now.

marshall-davis commented 8 years ago

I am not against a plugin configuration file. For an power users or whatever that want to dig into it having multiple files means looking in multiple places is all.

marshall-davis commented 8 years ago

@PatrickDattilio Alright man, you're going to have pull upstream. I messed up somewhere big time.

At this time I expected no conflicts, I expected this branch to be merged completely. It's apparently not. Also, I have introduced a bug into master now. It may be due to this being a WIP.

File "D:\Marshall\Documents\Code\Python Code\TEC Client\client\clientui.py", line 399, in create_plugin_menu
    self.plugin_checkboxes[plugin] = tk.BooleanVar(value=self.plugin_manager.plugin_status[plugin])
AttributeError: 'PluginManager' object has no attribute 'plugin_status'
PatrickDattilio commented 8 years ago

I think we are good

marshall-davis commented 8 years ago

Good! Going to merge this in so that I don't make your life difficult again. :laughing:

PatrickDattilio commented 8 years ago

Think you could do a release? My exe won't run, some error pops up really fast and closes whenever I build. I think it might be related to cx_freeze

marshall-davis commented 8 years ago

Yeah, we were making such progress I thought maybe we could hit the 1.0 milestones and release. I'll get one out tomorrow though.

:laughing: Actually, someone in the WA just asked me about this issue and when I said I thought it was fixed in the current version he ask if I would release tonight. I'll see how it goes.

marshall-davis commented 8 years ago

Released a release. I have no idea why, but some stuff was merging badly. Also, the application icon is not updating.

The next few releases and merges I'll pay closer attention to in regards to what is changing.