Wedge

Not even in the admin area. It's primarily search results and the skin selector.It might not use the msg skeleton but it uses the template.I'm not. I've been poking around with various things I'm not happy with in the admin area (language editor, smiley stuff, message icons) and not really making any headway with any of it in a way I like. No-one seemed interested in my wanting a 3DS icon :/ (Wedge actually works reasonably well on a 3DS, btw, save for the numbers in note/notewarn/notenice type classes)Makes sense, provided there's not a huge bandwidth hit I guess.

Yeah, he's now set it to $35 and vbgamer hasn't updated the page listing yet.

That's something that was mentioned between me and the SMF moderators when he removed the small post I made - if what I have to say is full of crap, he can refute it publicly. But if it's legit, then he can defend himself publicly, not through deleting my posts.

json_encode is absolutely fine on UTF-8, assuming it's available. It's only for non English ISO stuff that there's a problem, assuming it's available. There is a compat function kicking around for that use.

He made 5,000 lines of mod to make a nice interface around editing that array. Including such gems as a 50 line easter egg loaded every page, and his own file caching system that insists on pretty-printing everything, including comments, to a file, as opposed to, say, using serialize.

In fact, lemme dig out all my comments. I tried to treat it exactly as I would a conventional mod review: study the code first then reviewing it in use.



modifications.xml
1. QueryString edit need not be an actual edit, it should have been tied to the integrate_buffer hook.

2. I'm presuming the first Subs edit is to make sure it's after the menu hook? You could always do what SimpleSEF does and check the position of the hook periodically and if it isn't the last entry, adjust the hooks.
The only advantage is that anyone who added their code after the main code (like SimpleDesk) might be caught by it.

3. Why not just ditch the first edit to Subs and wrap everything up in the end of the menu handler and be done with it?
This would mean you would have one edit (and only one edit) but you'd know you'd nail every single item, hook or otherwise, just do a bit more clean-up on it.

4. With the index.php edit, exactly the same deal, do it on the actions hook, and do it last, you can even fix the hook check for any hooks you do use if you want to.
That said, I can see the logic of doing it there, in the hopes of cleaning up any mods that injected after the main array - but there are very few of those.


*install_mep_hooks.php
There is a part of me that wants to recommend a single file, and whether it's installing or uninstalling is based on whether it finds the mod's files.
Certainly it would mean that if you add new or change hooks later, it's one less thing to remember to maintain.


integrate.php
theLegacy() - yay for easter eggs. Amusing comments are fine, but big easter eggs of 50 lines of code, especially one that loads *every page* is not clever.

Also, integrate.php is included with integrate_pre_include, but I see at least it's included in the code modification. I'd also prefer to see the admin hook function not to be loaded every page load - every bit of parsing is important.


global.php
Seems to me this could easily be in the main code. Kind of surprised Aeva isn't in there.

Also a bit annoyed at the way SD support is handled. If it exists, Subs-SimpleDesk.php will already be loaded, no need to reinclude it again, just check for the function existence, will do the same job without the performance of a non-firing require_once.

The last bit in global.php could be streamlined to:


There's other various minor optimisations like that - if something starts out as false, then through some process becomes true, and can't ever become false again, you might as well exit as soon as you know it's true, rather than slogging through multiple loops whose outcome won't change it any further. Also, no need to pull the keys if you're not going to use them.

If you really wanted to go nuts, you could convert that to a count(array_intersect($these_membergroups, $user_info['groups'])) > 0 but I think you'd have to be careful about type juggling.


portal-compat.php
Hmm, interesting methods of detecting whether portals are active or not, but I'll come back to that later on. Any reason TinyPortal, PortaMX and Dream Portal are excluded?


default-logs.php
I know you're listing default actions, but linking to easter eggs? about:*, modifycat, and clock are all easter eggs. I'd also discourage linking to viewquery since it's really not that useful as a menu item. In fact linking to it directly will have unintended consequences unless you're providing a query id number.


tasks.php
Why have you used your own cache? Aside from a security risk, there is a performance limit, since it's now I/O bound as opposed to being able to use memcache or similar memory cache.

If you are going to cache anything, at least look at SMF's code, serialize, addslashes, output. No need to artificially fluff the writing of the cache, also no need to make the cache bigger just to be more readable - no-one should be looking at it directly. Incidentally that makes it more likely to trip the bug that meant file_put_contents even needs LOCK_EX in the first place. And you really don't need to be dumping comments in the cache file. It's not meant to be human readable. Go look at SMF's to see how, generally, it should be done.

Also, WTF is '01001001010011000100110001001101' and why is it used as a literal value in a query without proper escaping? (Line 103. Should be pushed to a {string:var} and imported that way, not just escaped with inline backslashes, since SMF doesn't support a {literal:value} type) Hmm, that looks like ASCII... ILLM. That wouldn't be a typo on the LILM you normally dedicate your mods to, would it?

Another example of streamlining would be the switch in lines 244 onwards. Better would be to declare an array up front:


Then in place of the switch:

Of course we'd declare the array outside the where specifically so that we don't have to redeclare it every iteration.

Similarly, the internal action three-way branch could be simplified to:

On top of that, you have lists of items already imploded, which you store in the cache as such. Save yourself some time, explode that to an array, typecast all the members to ints then save it as part of the serialized array directly. It doesn't save *quite* as much room in the cache file but it'll be faster than exploding it when you come to parse it later.


OK, so now I'm actually going to try it.

Yay for an extra query, every single page, to check the packages table to see if I've suddenly installed a portal - which would be fine if I installed it through the package manager. Also, why no caching? I hear you're a big fan of that. I also hear you don't understand how to do it properly especially on a very large site where caching is absolutely critical.

Modifying buttons
1. Can't add a button just for 'regular members'.

2. Please tell me you didn't add jQuery just for slow animation effects. And if you are going to have animations, no more than 200ms for elements people want to edit (for what we do, we found 150ms for expanding panels and 200ms for closing worked best)

3. Do you really need to dump every single one of your language strings into the page? In fact, you're going to have *so* much fun with that, because json_encode is not standard even in 5.2 (WordPress had to include compatiblity functions for it even when they went 5.3 only), and the best part, json_encode doesn't even work properly on non-UTF-8 strings. It's not a problem for English, of course, but anything else it will be.

4. The Associated Permissions icon on a button seems very ambiguous, especially for admin where it's not obviously controlled by a permission, e.g the admin button is shown as not having an associated permission except that it actually doesn't just rely on group access ordinarily, it uses array('admin_forum', 'manage_boards', 'manage_permissions', 'moderate_forum', 'manage_membergroups', 'manage_bans', 'send_mail', 'edit_news', 'manage_attachments', 'manage_smileys') in fact. SP at least also adds its own to that, as well as SD doing its own thing.

5. The shiny 'Your changes have been saved' thing at the top... it would be cool if it were AJAXively saving changes, but to do it with animation when you're coming back from another page load just looks tacky.

6. Layout is buggy when SimpleDesk is enabled, when using a screen of 1000px wide, the tickbox overlaps onto the 'Button Name' textbox.

7. In fact, the SD integration doesn't work. The blue box says it will automatically be set to active when SD is enabled, or that I can enable the helpdesk button from Display Options... except it is enabled by default (the option is to turn it off), and no combination of pressing things in the SD options area allows me to re-show it. Oh, and it dumps the submenu too, both for the admin and the helpdesk items, some of which might be useful, but will need to be recreated from scratch - as will any other mods with dynamic menu items.

8. Now that I look at it again, the first time it builds the action cache, it's actually incorrect. Even if SD is installed and running (and active) then install MEP, it still doesn't let me add the button until I clear the cache. The same seems to hold true of any other mods I've tried.

9. Um, why does restoration not show anything when you first install?

10. More disturbingly for something 'so advanced and customisable', I now realise why Aeva and SD menus aren't supported... unless it fits the SMF permissions model, there's actually no way to add a button. For example Aeva's tab doesn't have any children unless there's some new items. But there's no way to configure it, which means it isn't 'completely customisable'. It's 'completely customisable provided it doesn't do anything that doesn't exactly fit SMF'.


Honestly? It feels like a beta version, not a 1.0.0 version, and certainly not from someone who has been trusted with the keys to a major platform. I go on about that but it's important. If you wrote code like this for the SMF trunk, you would get criticised for poor coding style, as quite rightly you should. If you're telling me that 5076 lines of your own code (which is what I make the code count, excluding comments and blank lines, plus excluding jQuery because you didn't write that) took you 3 years and this is the result, you have further to go than I thought.

And remember, this is being written by a core SMF developer as a paid mod. You see now why it bothers me so much?

I wasn't aware that pretty URLs did that, but on my local site I never, ever use PURLs anyway.Stripping trailing =.I understand this, only too well.Doesn't it already?There's only one file because I spent two days combining it into one file and making his syntax nicer for us to read. But I won't do that next time for the simple reason that it was two days trudging through a lot of files for fairly little benefit. It's not like I can just magically drop all the dependencies if I want to use SSH to upload stuff, since pretty much everything is cross-dependent.

Especially since anything related to an editor instance will be a true method call to said instance... ;)

Fugue's pretty thorough, but when it's 3k and change icons, it's hard to imagine it doesn't support what you want almost entirely out of the box.

But the IconDock icons look like they might be more interesting in that respect, plus I've wanted to phase some of the old icons out for a while and it would be nice to have something a little different and not 'like everyone else' which Fugue's getting a bit towards these days.

I've seen other issues in Chrome with flexbox but they all seem to be randomly reproducible not consistently so >_<