« Plugin revs
New revs - Public comments
Resize images no in Wedge? »

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #585, on March 30th, 2013, 03:43 PM »
Quote
Mind you, I also tried the same on small_delete, but Pete somehow managed to crush the competition here. What tool did you use for that one..?! (small_move.gif)
All I did was take the old quick_delete that got deleted, cropped it to size and dropped it in.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,082

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #588, on March 31st, 2013, 08:54 PM »
I installed the latest Gimp, and it gave me a 78-byte file for your quick_delete.gif, instead of 75... A bit surprising. What's your magic trick, eh..? ;)

Okay, so I'm looking into  that code I removed, looking to see if I was mistaken in flagging it as buggy, but it still strikes me as odd... I can only see problems with it, and I don't think I wanna do any kind of revert. Maybe rewrite the error code to get rid of 'new_reply' and only mention 'new_replies'..?

Here's my reasoning. Please share your thoughts about it.

Code: [Select]
$txt['error_new_replies'] = number_context(isset($_GET['last']) ? 'error_new_reply_reading' : 'error_new_reply', $context['new_replies']);

"error_new_replies, which is a non-existent $txt string to begin with, should be built with the contents of $txt['error_new_reply_reading'] or $txt['error_new_reply'], depending on whether it was triggered after pressing the Reply/Quote button in topic pages, or pressing Submit in the post page. These two $txt items are arrays, thus number_context is there to choose the correct string within these arrays, as well as sprintf them to push the number in place. At this point, we have, for instance, 'Warning - while you were typing, 2 new replies have been posted. You may wish to review your post.' in our variable. So far, so good...

Code: [Select]
if ($context['new_replies'] == 1)
$txt['error_new_reply'] = isset($_GET['last']) ? $txt['error_new_reply_reading'] : $txt['error_new_reply'];

"new_reply should ALSO be set, if there's only one reply. BUT it should have the unedited string array, i.e. $txt['error_new_reply'] is now an ARRAY of unformatted strings... Which doesn't serve any purpose..?!

Code: [Select]
else
$txt['error_new_replies'] = sprintf(isset($_GET['last']) ? $txt['error_new_replies_reading'] : $txt['error_new_replies'], $context['new_replies']);

So, we're resetting the error_new_replies string that we just set a moment ago... :-/
Either with:
1/ A non-existent string, $txt['error_new_replies_reading'] (note the plural), which generates the error that prompted me to remove the code block,
2/ An array of unformatted strings, which we then pass to sprintf, which only accept strings, and not arrays -- another error waiting for us.

Really, I don't see the point.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #589, on March 31st, 2013, 08:59 PM »
I'm using GIMP 2.6.12 if that's any help.
Quote
So far, so good...
That should be it. As I said, I was wrong. It's the old code that wasn't removed properly, but at first glance looked thoroughly broken because I didn't see the extra line above the removed code that solved the problem, I just saw 'chunk of removed code that sets up variables used later on'.

I've spent a lot of time getting very frustrated lately with a lot of people and a lot of things because I feel like I'm shouting up from inside a well and no-one but me is listening to what I'm saying.

Which is why I'm now working on a new feature and until the feature is done, I'm just going to avoid forums in general, because it's hard to concentrate when I'm in this much of a bad mood.
Re: New revs - Public comments
« Reply #590, on March 31st, 2013, 11:27 PM »
Sorry your feeling bad, Hope it's not me! and sure hope you feel better!

I've been in and out of the hospital the last few days, with head and nerve issues, so please don't mind me!

regards,
Maxx

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #591, on April 12th, 2013, 08:33 PM »
Quote
! The old Profile Notifications page is still there, but it wasn't showing up to do duplicate array entries. (Profile.php)
From r2052... when it was added, it was very deliberately done the way it was so that we would have the thread of where to start when removing the old notifications system because of the new one being added.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #592, on April 12th, 2013, 09:11 PM »
I wanted to rename our new system to 'area=notifications' anyway, so I did that and noticed the other one reappeared, I was the first surprised...
I decided to commit it because at least, it's visible, and we know we have to remove it (or refactor it) at some point...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #593, on May 8th, 2013, 12:14 AM »
r2094 - JS changes

Well, I just mostly pulled the first stuff out of the templates themselves so they were like that already ;)

As far as functions having ; on the end, wasn't that because Packer needed it?

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #594, on May 13th, 2013, 12:30 PM »
Quote from Arantor on May 8th, 2013, 12:14 AM
r2094 - JS changes

Well, I just mostly pulled the first stuff out of the templates themselves so they were like that already ;)
Yep, I only figured out after I changed it ;)
Quote
As far as functions having ; on the end, wasn't that because Packer needed it?
Yes, but as I explained somewhere else, simple function () {} declarations don't require a semicolon, only var func = function () {}; (i.e. anonymous functions, or closures, or whatever) require one. This isn't related to Packer in itself, it's what JavaScript engines expect to see when everything is tightly packed. Either a semicolon, or a linebreak. Packer does semicolons, so we do semicolons. Plus, semicolons are safer in the long run -- there are instances where using linebreaks instead of semicolons will generate bugs (although they're clearly documented.)

Code: [Select]
$fonts = array_filter(array_map('trim', preg_split('~[\s,]+~', $settings['editorSizes'])));

Considering it's stored as linebreak-separated variables, why not just do an explode() on these..? Or even get rid of array_filter and array_map, as I don't see many reasons for this variable to be modified by anything else than Wedge..? And we could do the checking at save time in ModifyPostEditorSettings.

Do you see any uses for disemvowel and scramble functions? Maybe just copying their single-line contents into the main codepath would be enough? I'm not worried about disemvowel(), but scramble() might conflict with other functions with the same name. You never know -- adding an encryption library, or something like that...?

I don't know if it's a bug or something I understood incorrectly, but this infraction point system... When I'm giving someone a sanction, I find myself with having to determine a number of points to give... Why should I, when I can just select a list of pre-defined sanctions, such as hiding the signature...? Or maybe it's admin-only and I should look at how it works for mods?

Done with the checks. Overall, great work! :)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #595, on May 13th, 2013, 03:51 PM »
Quote
And we could do the checking at save time in ModifyPostEditorSettings.
Sure we could. I'm just doing what I did before with the list of fonts, and that I seem to recall I borrowed from the old reserved names code.
Quote
Do you see any uses for disemvowel and scramble functions? Maybe just copying their single-line contents into the main codepath would be enough? I'm not worried about disemvowel(), but scramble() might conflict with other functions with the same name. You never know -- adding an encryption library, or something like that...?
I suspect there won't be any separate uses so yeah, they could go.
Quote
I find myself with having to determine a number of points to give... Why should I, when I can just select a list of pre-defined sanctions, such as hiding the signature...?
Technically it shouldn't be forcing you to pick some points but there are circumstances where for performance it just assumes 0 points = no current infractions on the account.
Quote
Or maybe it's admin-only and I should look at how it works for mods?
It sort of is. Admins have all the powers, but moderators may or may not depending on what they are given. If a member group is not given ad-hoc infraction powers, the only choices they have are the pre-set ones allocated to their account. The idea is that you set up pre-set ones which cover all the standard cases which makes issuing them little more than a few clicks.

You could always set up a pre-set infraction that doesn't have any points and just removes the signature - then you wouldn't even have to type up the warning text next time, it would be defined ready for you. At least, that's the theory ;)

But I'm more than happy to change it if we find better ways of doing things.
Quote
Done with the checks. Overall, great work!
Thanks :)

live627

  • Should five per cent appear too small / Be thankful I don't take it all / 'Cause I'm the taxman, yeah I'm the taxman
  • Posts: 1,670
Re: New revs - Public comments
« Reply #596, on May 17th, 2013, 12:24 AM »
Quote
...! I spent months postponing this because I thought I had to do the query_see_topic implementation as well. Turned out that it was done in late February 2012, over 700 revisions ago
I even made a plugin in April 2012 that plays with topic privacy.
A confident man keeps quiet.whereas a frightened man keeps talking, hiding his fear.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #597, on May 18th, 2013, 05:23 PM »
Lulz.

BTW, it's quite tempting, re: rev 21+21, to go through the codebase and replace all array_intersect() calls with something that's faster...

To be specific, imagine that as a global moderator (but not an admin), you have dozens of permissions applied to you.
If I wanted to know whether you have permission can_do, in_array('can_do', $perms) is very fast. (Not as fast as an isset() test, but whatever.)
Now, if I want to test against can_do and can_eat, array_intersect(array('can_do', 'can_eat'), $perms) can become really, incredibly slow. The more permissions you add to the first or second array, the slower it gets, and exponentially. I've found that doing multiple in_array calls on can_do and can_eat can be much faster. Of course, we're talking minor speed increases, so it's not THAT important... I tested with a request of 3 permissions, all of which being wrong (it's 3 times faster if the first one is true, of course). The $perms array was arbitrarily filled with 110 entries, which is probably twice as many permissions you'll ever need to give someone... Still, array_intersect was approximately 150 times slower than three consecutive in_array tests. I'm not kidding you, guys... Call array_intersect 10.000 times, and we're talking multiple seconds to complete, while it executes in a few milliseconds with in_array().

So, maybe there are other array_intersect() calls to intercept elsewhere, what do you think..?

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #598, on May 18th, 2013, 07:44 PM »
There are probably other array_intersect calls, sure.

As far as permissions, it's long been on my plan to adjust how permissions were loaded and tested (to use isset rather than in_array) but that was part of the permissions overhaul and until I figure out how to make the UI, I won't be touching that.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: New revs - Public comments
« Reply #599, on May 18th, 2013, 10:20 PM »
Quote from Arantor on May 18th, 2013, 07:44 PM
There are probably other array_intersect calls, sure.
Noticed you added a few, yesterday, actually... ;)
Quote
As far as permissions, it's long been on my plan to adjust how permissions were loaded and tested (to use isset rather than in_array) but that was part of the permissions overhaul and until I figure out how to make the UI, I won't be touching that.
Well, isset() would be good, yes.

My only opinions (not suggestions) about the permission UI, would be that:

- I should be able to get a proper list of permissions, global and per board (and per album), for any given member, with a section in their profile area... (I've been stung with this in SMF; yesterday, I tried giving the same rights to two Noisen mods, and one of them is complaining that he doesn't have the same rights, which seems impossible according to his group list...)

- In the radio box list, I'd personally use dropdowns with an icon on each (green for enabled or enabled_own, red for disabled, another green or some blue for enabled_any). Sure, it's an extra click to change a permission, but then you can add deny permissions, right..? Unless you want to add deny for _own and _any separately, in which case, err... :-/

- I'd like to see the membergroups ajaxified... i.e. when clicking the link to modify their permissions, have it load under the permission name instead, so that you can even load multiple membergroups on one page, and then change them all in a row...

I haven't read the proper topic (yet), though.
« Plugin revs
Resize images no in Wedge? »