MyBB Community Forums

Update: The plugin has been submitted to mods.mybb.com: http://mods.mybb.com/view/mybb-captchapack

This thread is posted in "Development" forum as I don't consider my plugin mature yet and want examinations from other plugin developers.

MyBB CAPTCHA Pack is my project to clone Drupal's CAPTCHA Pack, to provide various alternative CAPTCHAs for MyBB. The Github repo is:

https://github.com/richardgv/mybb-captchapack

Currently only 3 CAPTCHAs are implemented: ASCII art CAPTCHA, CSS CAPTCHA (which uses CSS float to break the positions of characters), and route CAPTCHA (paints a zig-zag line with different characters). And those CAPTCHA are much easier to break than the default image CAPTCHA / reCAPTCHA -- if anyone bothers to do so. Its sole good part is it's different from the default. Oh, and this plugin requires PluginLibrary 11.

The configuration is pretty simple: All in "CAPTCHA Pack" setting group.

The plugin is working in my tests, but as what I stated in README: "mybb-captchapack could be considered of pre-alpha quality right now, since I know pretty little about PHP, MySQL, and MyBB, and the tests I’ve done are few. Usage on productive sites is highly discouraged. For all possible outcomes of using the plugin (including but not limited to burning your CPU, making your monitor explode, or attracting aliens to abduct you) I’m not liable." I'm posting here in hopes that some more experience developers could examine the plugin for possible bugs, especially security issues. Thanks in advance.

Minor things I noticed (without actually testing the plugin, just reading the code):

If you need multiple files in the plugins/ directory, it's better to put them in a subfolder (i.e. yourplugin.php and yourplugin/extras.php). Because the ACP Plugins page loads all PHP files, and if your plugin loads it too, you get a function already defined error and such. Also it's better for users so they can see all the files belonging to your plugin, and not wonder if it's a separate plugin or what.

It won't make a difference for most people, but the convention is that you use $db->write_query() instead of $db->query() when you modify the database. $db->query() is for selects. Also while at queries, this is just style, but CREATE TABLE and such should be capitalized. Full caps is horrible everywhere else but with queries it helps readability.

You're deleting settings in the deactivate() function, it's better to do that in uninstall. So people can deactivate the plugin temporarily without losing their settings. Create/update settings on activate(), delete them on uninstall().

@frostschutz:

Firstly, thanks for all your suggestions.

If I had found more documentation about writing a MyBB plugin, of the quality of the Drupal module API documentation, I wouldn't have made so many design mistakes. The things in the Wiki look pretty messed up (due to a migration to GitHub pages?), to be honest. So many broken links and formats that I don't even want to read.

I will correct the issues tomorrow. (It's close to midnight here.)

(2012-12-09, 02:31 PM)frostschutz Wrote: [ -> ]If you need multiple files in the plugins/ directory, it's better to put them in a subfolder (i.e. yourplugin.php and yourplugin/extras.php). Because the ACP Plugins page loads all PHP files, and if your plugin loads it too, you get a function already defined error and such. Also it's better for users so they can see all the files belonging to your plugin, and not wonder if it's a separate plugin or what.

Ah, I don't see the method of handling multiple-file plugins being mentioned in the documentation, so I just randomly put captchapack-data.php there... I'm always using require_once so fortunately the problem did not occur in my tests.

(2012-12-09, 02:31 PM)frostschutz Wrote: [ -> ]It won't make a difference for most people, but the convention is that you use $db->write_query() instead of $db->query() when you modify the database. $db->query() is for selects.

I see. I didn't read the Database method page on the wiki as it looks terribly messy.

(2012-12-09, 02:31 PM)frostschutz Wrote: [ -> ]Also while at queries, this is just style, but CREATE TABLE and such should be capitalized. Full caps is horrible everywhere else but with queries it helps readability.

I typically use lowercase SQL queries because my book (PHP and MySQL Web Development) does so. Full caps is more readable indeed, though.

(2012-12-09, 02:31 PM)frostschutz Wrote: [ -> ]You're deleting settings in the deactivate() function, it's better to do that in uninstall. So people can deactivate the plugin temporarily without losing their settings. Create/update settings on activate(), delete them on uninstall().

I chose to drop the settings in captchapack_deactivate() as multiple plugins that I use as references (reCAPTCHA and invite) are doing the same, and it sounds really inconsistent to create the settings in captchapack_activate() but remove them in captchapack_uninstall(), like what you did in hello_pl.php.

Another plugin I saw, jsbotprotection, creates the settings in _install() and removes them in _uninstall(). Could I use this style?

Re: creating settings in install, the convention is to modify the database in the install routing (add tables, columns etc) and populate the database in activation. I would seriously advise looking at frostschutz's PluginLibrary to aid you with tasks such as the settings and templates (if there are any?).

(2012-12-09, 03:09 PM)euantor Wrote: [ -> ]Re: creating settings in install, the convention is to modify the database in the install routing (add tables, columns etc) and populate the database in activation. I would seriously advise looking at frostschutz's PluginLibrary to aid you with tasks such as the settings and templates (if there are any?).

Thanks for the guide. :-) I must be blind to ignore the "The _install function should really ... insert things like settings" sentence on the Wiki.

I'm already using frostschutz's PluginLibrary to do many tasks, but... Looks like the sample plugin hello_pl.php included in PluginLibrary creates settings in _activate(), too?

Yes, settings should be created in _activate() by convention and only removed in _uninstall(). The hello_pl is a great guide to how things should be done.

(2012-12-09, 03:04 PM)RichardGv Wrote: [ -> ]Another plugin I saw, jsbotprotection, creates the settings in _install() and removes them in _uninstall(). Could I use this style?

You could but then you'd end up having the same problem - if you add a new setting in a future version of your plugin, the user would have to uninstall it (and lose data), instead of just deactivating / activating it once.

Using activate/uninstall for settings may sound inconsistent, but it really isn't. It's done this way to preserve data until the user actually uninstalls the plugin.

There are two things I forgot to ask:

1. There are some strings (messages shown to the user) in the plugin that should be translatable. Is there any guides that I could follow regarding handling of languages and translations in plugins? I don't want to make extra design mistakes.
   
2. Is there a convenient function somewhere to insert a task? I'm currently relying on copied code from MyBB core to insert a task.
   

(2012-12-09, 03:18 PM)euantor Wrote: [ -> ]Yes, settings should be created in _activate() by convention and only removed in _uninstall(). The hello_pl is a great guide to how things should be done.

Ah, I see. I thought you mean settings should be created in _install(). Then the MyBB Authoring Plugins page on wiki probably needs an update. It says I should insert settings from _install(), if I didn't misunderstand the whole thing.

(2012-12-09, 03:18 PM)frostschutz Wrote: [ -> ]You could but then you'd end up having the same problem - if you add a new setting in a future version of your plugin, the user would have to uninstall it (and lose data), instead of just deactivating / activating it once.

Yeah, indeed. I'm glad PluginLibrary seemingly handles changes in settings beautifully in settings().

(2012-12-09, 03:31 PM)RichardGv Wrote: [ -> ]There are some strings (messages shown to the user) in the plugin that should be translatable. Is there any guides that I could follow regarding handling of languages and translations in plugins? I don't want to make extra design mistakes.

You'd have to create a language file in inc/languages/<language>/ and use $lang->load() to load it when necessary. You should be aware that all language files share the same namespace (keys of a single array or object properties if you like), so your plugin name should be incorporated in the key somehow.

(2012-12-09, 03:31 PM)RichardGv Wrote: [ -> ]Is there a convenient function somewhere to insert a task?

Not that I know of, but I don't use tasks much. When I attempted to write my own captcha plugin (unfinished, I don't have a spam problem currently), I just hooked into MyBB's captcha and used MyBB's random image string as reproducable random number source for my own captcha so I got by without creating my own table / task for it. I still have the code somewhere if you're interested, but I can't vouch for its quality at all. It's a bit of a hack.

While on the topic of tasks, I'd allow more than just one hour of time. People leave their tabs open and forget about them, so... you could make it a setting

(2012-12-09, 03:31 PM)RichardGv Wrote: [ -> ]Then the MyBB Authoring Plugins page on wiki probably needs an update. It says I should insert settings from _install(), if I didn't misunderstand the whole thing.

That'd probably be what you'd do when not using PluginLibrary. When you don't have a method to update settings, you would end up removing and recreating everything anyway.

(2012-12-09, 03:31 PM)RichardGv Wrote: [ -> ]There are two things I forgot to ask:

1. There are some strings (messages shown to the user) in the plugin that should be translatable. Is there any guides that I could follow regarding handling of languages and translations in plugins? I don't want to make extra design mistakes.
   
2. Is there a convenient function somewhere to insert a task? I'm currently relying on copied code from MyBB core to insert a task.
   

1. Sure is. You can create language files in the /inc/languages/xx/ folder (where xx is the language - default is english) named yourplugin.lang.php with different language strings like this:

<?php
/**
 * Language strings for my plugin.
 * @author Euan T.
 */

$l['myplugin_some_string'] = 'Some String';
$l['myplugin_some_other_string'] = 'Some Other String';

You can then load this language file like this:

global $lang;

if (!isset($lang->myplugin_some_string)) {
    $lang->load('myplugin');
}

And then access the new language strings as if they were properties of the $lang object.


2. Not that I know of. I was actually thinking about submitting a pull request on PluginLibrary for this purpose. Here's the code I use in my MyAlerts plugin though:

$taskExists = $db->simple_select('tasks', 'tid', 'file = \'myalerts\'', array('limit' => '1'));
if ($db->num_rows($taskExists) == 0) {
    require_once MYBB_ROOT.'/inc/functions_task.php';

    $myTask = array(
        'title'       => $lang->myalerts_task_title,
        'file'        => 'myalerts',
        'description' => $lang->myalerts_task_description,
        'minute'      => '0',
        'hour'        => '1',
        'day'         => '*',
        'weekday'     => '1',
        'month'       => '*',
        'nextrun'     => TIME_NOW + 3600,
        'lastrun'     => 0,
        'enabled'     => 1,
        'logging'     => 1,
        'locked'      => 0,
    );

    $task_id = $db->insert_query('tasks', $myTask);
    $theTask = $db->fetch_array($db->simple_select('tasks', '*', 'tid = '.(int) $task_id, 1));
    $nextrun = fetch_next_run($theTask);
    $db->update_query('tasks', 'nextrun = '.$nextrun, 'tid = '.(int) $task_id);
    $plugins->run_hooks('admin_tools_tasks_add_commit');
    $cache->update_tasks();
} else {
    require_once MYBB_ROOT.'/inc/functions_task.php';
    $db->update_query('tasks', array('enabled' => 1, 'nextrun' => fetch_next_run($myTask)), 'file = \'myalerts\'');
    $cache->update_tasks();
}

(2012-12-09, 03:31 PM)RichardGv Wrote: [ -> ]

(2012-12-09, 03:18 PM)euantor Wrote: [ -> ]Yes, settings should be created in _activate() by convention and only removed in _uninstall(). The hello_pl is a great guide to how things should be done.

Ah, I see. I thought you mean settings should be created in _install(). Then the MyBB Authoring Plugins page on wiki probably needs an update. It says I should insert settings from _install(), if I didn't misunderstand the whole thing.

You are correct. The wiki needs some major work done on it. I'll likely modify the plugin page when I get time.