MinoMino / minqlbot-plugins

A collection of plugins for minqlbot.
GNU General Public License v3.0
5 stars 10 forks source link

please consider to split balance and ban functionality into separate plugins #9

Closed WalkerY closed 9 years ago

WalkerY commented 9 years ago

For example: instead of single balance.py there could be:

balance.py (!teams, !balance, !do functionality but no kicking, no elo restrictions, no setrating, no getrating, no fetch player ratings, no cache)

ratings.py (fetch player ratings and cache with nice interface for other plugins, implementation of aliases and !setrating !getrating / !elo)

rating_restriction.py (plugin that restricts elo and kicks etc., why in seperate plugin? for example, I would like to implement extension - multilevel limit - if there are no players limit is lower etc. currently I would have to alter balance.py in many places instead of giving server owner just new rating_restriction or introducing there new options. Also there might be some other algorithms and to avoid multiplying options in config it would be better to for example enable plugin rating_restriction_ALGx.py and delete rating_restriction from config.)

instead of single ban.py there could be: ban.py (!ban commands but no leavers ban, no new account ban) ban_accountage.py (new account ban) ban_leavers.py (leavers ban)

why this would be helpful? for example I have a quick hack that bans ppl that have x times more quits on their ql profiles than wins (frequent quiters). I would prefer to be able to put it into configurable plugin but its not possible now (no proper extension points). I could call it for example: ban_leavers_qlprofile.py or sth shorter.

How to implement !checkban then? For example, there could be a queue of checks in ban.py that other plugins can subscribe to and provide ban_plugin.checkban (Boolean) command (without sideeffects) and for example callback function ban_plugin.banned. The ban plugin could go through all subscribers and check if any subscried plugin says player is banned with checkban function. not banned by any plugin - say so. banned by any plugin? execute ban_plugin.banned for every ban. (those can display messages for each ban user has). Also, there could be an abstract class BanAlgorithmPlugin or sth that plugins can inherit instead of plugin class.

With above I think the code would be much cleaner and features would get proper separation and will be easier to extend. This is just a suggestion, there might be better split options. Also I understand that you may decide its an overkill.

WalkerY commented 9 years ago

actually rating_restriction.py should be ban_rating.py in this scenario as it is de facto a ban.

MinoMino commented 9 years ago

I can see your point, but I also feel like you could solve a lot of your issues by simply making a plugin that inherits balance or ban and override the methods you'd like to change, then make someone remove balance from their config and add your version of it.

In any case, with the Steam update coming, I'd honestly prefer to not spend too much time on these plugins since they'll break when it hits. Since nicknames won't be unique identifiers, all the data in the DB will become useless. I'll probably use that as an opportunity to move away from SQLite and use a client-server NoSQL database instead. In other words:

So ultimately, pretty much nothing will make it through. Might as well rewrite the whole thing at that point, at which point I'll consider splitting up functionality more than I've done so far.