« Login/Register Feature
These two bytes may not matter to you...
Need some help on a UI matter »

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: These two bytes may not matter to you...
« Reply #105, on September 15th, 2011, 01:47 AM »
My understanding was that topicseen is an older item, legacy from 1.1 or somesuch, but I have no idea offhand if that's still the case or not.

And yeah, I think the option can go, no matter how we change the index, I don't see us having fully expanded page indexes in there.

As for pm_email_notify, be careful with this, because there's a separate set of notification preferences buried in the members table and I'm not sure whether that isn't used in preference or not.

The other thing I'd point out is that there are instances in SMF where 0 is incorrectly used, so it's possible that even if the field is the right one, that 0 is the right value for the job.
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
Re: These two bytes may not matter to you...
« Reply #106, on September 16th, 2011, 07:29 PM »
Re: topicseen, I really don't think it's a legacy thing... Its main use is in Display.php, things like this:

Code: [Select]
// Have we recently cached the number of new topics in this board, and it's still a lot?
if (isset($_REQUEST['topicseen'], $_SESSION['topicseen_cache'][$board]) && $_SESSION['topicseen_cache'][$board] > 5)
$_SESSION['topicseen_cache'][$board]--;

The idea really seems to be about saving extra work for the server.

Re: the option, it will go away then :) Adding to my to do list.

Re: pm_notify, I'm pretty sure my local install is vanilla with respect to that setting, and I can assure you PM notification is disabled for me. Maybe it's because I don't have a mail server on my machine, and Wedge saw it and disabled the feature...? I don't know.
Ahhh, can't bother to look into it now... :(

Another thing now. While looking into topicseen and other things, I noticed a VERY strange thing on my local install and its single topic (sigh). When posting a new message, I'd be brought to the second-to-last message instead. After looking into it, I found a SMF bug. I can reproduce it here.

One of my posts in the topic is actually unapproved (so I can check on styling unapproved posts.) The COUNT that generates $context[start_from] from start=new takes care of that by removing all unapproved posts from the post count, but then... In the meantime, Wedge meets this piece of nasty code:

Code: [Select]
// Go to the last message if the given time is beyond the time of the last message.
if (isset($context['start_from']) && $context['start_from'] >= $topicinfo['num_replies'])
$context['start_from'] = $topicinfo['num_replies'];

At this point, $context['start_from'] is 20 (according to COUNT(*)) if I have an unapproved post and 19 if I don't.
And $topicinfo['num_replies'] is 18 if I have an unapproved post, and 19 if I don't.
So, no unapproved post = it works.
Unapproved post(s) = it gets reset to < 19 and voilà, the bug happens.

IIRC, num_replies caches the number of actual visible posts for everyone, so it won't take unapproved posts into account, hence the bug. I unapproved a post of mine here and tried /new/#new and was redirected to the second-to-last post, again.
Any suggestions as to how we should fix this...?

I just tried again (posting 2 messages as an 'approvable' account and then logging in as admin, approving the last one...) Did the /new/, and I got redirected to the second-to-last post, i.e. the unapproved one. So it's definitely something that can be reproduced.

- Oh, and a bonus bug... Apparently, when the very last post in a topic is unapproved, the topic won't show up at all on the Message Index. It just shows nothing... :-/ I have to go to the mod area and then follow the link to the unapproved posts.
Funtastic. Today is bug day -- also found YET another bug in IE with white-space crap... Breaks the entire layout in certain conditions. :-/ Spent an hour on this, couldn't find a way to fix.
Re: These two bytes may not matter to you...
« Reply #107, on September 16th, 2011, 10:46 PM »
Fixed, BTW... Replaced $topicinfo['num_replies'] with $context['total_num_replies'] - 1, basically. Tried with various situations and it seems to always work.
Now for the IE bug.
Oh, and micro-optimizing more code, as always...
Re: These two bytes may not matter to you...
« Reply #108, on September 16th, 2011, 11:30 PM »
Pete, bed time and I haven't found a suitable way to fix this. Can anyone with svn read access give a look into this post under IE8? Either run it from XP if you have it, or Win7 -> run IE9, F12, choose IE8 browser mode.

Note: the bug doesn't happen in IE6/7 but user boxes are completely broken in them... Meh.

Just quote this post and copy the contents after the hr tag below. The text is random content. It's broken in multiple places (entire post overflowing on the right, code tags having a fixed height...)

Quote from Nao on November 7th, 2010, 01:03 PM
Bienvenue dans l'installateur du Forum Simple Machines !
;D ;D
Quote splitter. I see what you mean now I've seen it, very clever[/quote]You bet!

Code: [Select]
[quote author=Nao link=msg=1 date=1289131411]Bienvenue dans l'installateur du Forum Simple Machines ![/quote]Quote splitter. I see what you mean now I've seen it, very clever
Quote from Nao on November 7th, 2010, 01:03 PM
Bienvenue dans l'installateur du Forum Simple Machines !
Quote splitter. I see what you mean now I've seen it, very cleverotallyrandomcontentthatwilldefinitelyoverflowfromthisbox.

Long URL:
http://www.sainsburys.co.…&isJavaScriptEnabled=true

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: These two bytes may not matter to you...
« Reply #109, on September 16th, 2011, 11:41 PM »
Quote
Pete, bed time and I haven't found a suitable way to fix this. Can anyone with svn read access give a look into this post under IE8? Either run it from XP if you have it, or Win7 -> run IE9, F12, choose IE8 browser mode.
That would require me to turn my PC on when I'm currently on my Mac, experimenting with stuff in the hope of clearing my mind after spending two and a half days on rewriting the add-table routine.

What exactly is the bug with respect to whitespace? I'm presuming it's related to code tags given what you're saying, and I know I altered the code a while ago due to whitespace problems because IE has funny ideas about how it should handle whitespace in code tags.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: These two bytes may not matter to you...
« Reply #111, on September 17th, 2011, 10:27 AM »
So....? :^^;:
Posted: September 17th, 2011, 08:08 AM

If I remove the max-width from the css, Opera will fail too. It doesn't fail in SMF AFAIK so maybe it's due to something else (i.e. not the code tag styling itself), like the use of display:table-cell or some other esoteric CSS...
Posted: September 17th, 2011, 09:43 AM

Fixed.

table-layout: fixed was all I needed... In #forumposts .wrc, FFS. I'll never properly understand the logics of table layouts but at least it's done -- and what led me to the right one was the fact that it was broken in Opera as well when I remove the max-width. Now I don't need the freaking max-width anymore, meaning I'm not tied to a certain width. All is good!
Re: These two bytes may not matter to you...
« Reply #112, on September 28th, 2011, 12:26 AM »
Odd little thing I noticed last week... I can't seem to find any use for $context['languages']['english']['selected'].
It's not used anywhere I could find -- and it's not even set, i.e. if I select the English version, 'selected' isn't set to true or whatever.

This should probably go... Can someone look into this?
Posted: September 27th, 2011, 08:17 AM

Up.
Pete, I suspect you still don't have access to the codebase right....?

Arantor

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

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: These two bytes may not matter to you...
« Reply #114, on September 29th, 2011, 12:01 AM »
- aeva_get_num_files() function is seriously awful. It uses scandir() to iterate through the list of files and increment a counter.
There are plenty of ways of doing that in a single instruction -- e.g. the various ones here:
http://stackoverflow.com/questions/224408/return-total-number-of-files-within-a-folder-using-php
I don't know if it's worth it though... But on forums that heavily use the media gallery...

- selected: not only is this never used in Wedge, but it's set twice -- once in Load.php, and once in Register.php. It's set -- and never used after that...

Code: [Select]
if (!empty($modSettings['userLanguage']))
{
$selectedLanguage = empty($_SESSION['language']) ? $language : $_SESSION['language'];

// Do we have any languages?
if (!isset($context['languages']))
getLanguages();

// Try to find our selected language.
foreach ($context['languages'] as $key => $lang)
{
$context['languages'][$key]['name'] = strtr($lang['name'], array('-utf8' => ''));

// Found it!
if ($selectedLanguage == $lang['filename'])
$context['languages'][$key]['selected'] = true;
}
}

Remove or not...?

- getid3 is now at version 1.9.1, we're still at 1.7.9 (2 years old). I originally took a LOT of time to delete all of the PHP comments from getid3 -- there are so many, and sometimes so awfully indented, it's just a waste of space. I saved at least 20% of space by removing these. Problem is -- if I keep removing the comments manually, I can't update getid3 as easily as one would think... Meh. On the other hand, it's not like it needs to be updated a lot. I don't know.

Opinions welcome...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: These two bytes may not matter to you...
« Reply #115, on September 29th, 2011, 12:08 AM »
Using iterator_count is fine if you know it only contains images, otherwise you're pretty much implementing the exact same logic in a different method. As for which is fastest, the only way to test that is to get some folders, some purely images, some with half and half, some with mostly non images and benchmark it.

Selected is removable if we never provide any such UI as the language dropdown; using the flags pretty much invalidates that requirement, so yes, it should go. (Is it not even used in ManageServer?)

I feel your pain re external libraries with comments, I'm having similar fun with phpseclib. What's the benefit of updating?

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: These two bytes may not matter to you...
« Reply #116, on September 29th, 2011, 12:16 AM »
We can't really test for images because AeMe can really hold any type of files anyway -- and the file extensions are gone, either way. I'm just trying to get it to be simpler. The problem with glob() is that it's not available on all servers, but OTOH if it's not, then Wedge won't work (Subs-Cache uses it.) I've done too much benchmarking these days, I hardly even trust it anymore... Just doing a long loop will usually not work well, because PHP will cache results internally I believe. For instance, $a[strlen($a)-1] is faster than substr($a, -1) when calling them thousands of times -- but substr is always faster on a single call. (e.g. if you modify the string dynamically...)

No, it's not used in ManageServer... Sigh.
As for getLanguages, I'm tempted to always call it, even with just the English files... lulz.

getid3 changelog:
http://getid3.sourceforge.net/source/changelog.txt
Hmm...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: These two bytes may not matter to you...
« Reply #117, on September 29th, 2011, 12:34 AM »
So, what exactly does the function do? If it's purely a count of files, the iterator count is exactly what you want.

glob, btw, is available on every system I've ever seen, and given that it's implemented internally for Windows support, it can be assumed to be available on any platform we'd actively be supporting.

If it's not used in ManageServer (and it was a long shot that it would be used in the lang editor, I wasn't really expecting it to be), then it really can go.

As far as getID3, v1.8.0 is worth doing (PHP 5.3+ compat fixes) but the number of bug fix entries would suggest it's beneficial to get to the current version. I should note that I'm in the same boat with keeping up with Bad Behaviour, and if phpseclib can be minified enough to be included, we'll have it there as well. That said, these things are not updated regularly enough for us to be a problem IMO - but if we get up to date with versions, it should be easier to keep up to date.

Nao

  • Dadman with a boy
  • Posts: 16,082
Re: These two bytes may not matter to you...
« Reply #118, on September 30th, 2011, 10:37 AM »
- File counter. Its code was obviously lifted from another, more complicated function, so it was doing unnecessary things in the middle, most notably doing a recursive search... When we really only needed to have the files in the current folder. count(glob()) was the faster solution in the end.

- It can be disabled individually... But then again, why would it?

- Gone.

- Updated to 1.9.1, with a few files removed in the process (never actually got around to using these id3 tags from mp3 files, so we might as well not include the files... Until someone actually wants to implement this.)
Also, the PHP 5.3 compat fixes were long overdue at the time, and obviously Aeva Media shipped with a version of 1.7.9 that was custom-fixed for PHP 5.3.
I'd estimate that around 10 to 20% of all code in getid3 is comments, and it doesn't compress well on top of that. Still, with the files I removed, the additional code and comments aren't too huge...

- Another thing worthy of note.
http://techblog.procurios.nl/k/news/view/34972/14863/Cache-a-large-array-JSON-serialize-or-var_export.html
I was curious about var_export(), because it stores the array in PHP format, so I made my own test...

Code: [Select]
global $cachedir, $modSettings;
$myDataArray=array_merge($modSettings,$modSettings);
// Store cache
file_put_contents($cachedir.'/test1.php', json_encode($myDataArray));
file_put_contents($cachedir.'/test2.php', serialize($myDataArray));
file_put_contents($cachedir.'/test3.php', "<?php\n\$myDataArray = " . var_export($myDataArray, true) . ";");
file_put_contents($cachedir.'/test4.php', "<?php\nreturn '" . serialize($myDataArray) . "'; ?>");
// Retrieve cache
$a=microtime(true);
$myDataArray = json_decode(file_get_contents($cachedir.'/test1.php'));
echo microtime(true)-$a, ' ';
$a=microtime(true);
$myDataArray = unserialize(file_get_contents($cachedir.'/test2.php'));
echo microtime(true)-$a, ' ';
$a=microtime(true);
include($cachedir.'/test3.php');
echo microtime(true)-$a, ' ';
$a=microtime(true);
$myDataArray = unserialize(include($cachedir.'/test4.php'));
echo microtime(true)-$a;

As a result: (and I'm getting proportionally identical results with just a single string)

0.00023102760314941
0.00013899803161621
0.00025486946105957
0.00014996528625488

i.e. the unserialize solution, as used in SMF/Wedge, is nearly twice as fast as storing an array in PHP format...!
Meaning that if we actually went ahead and turned all of the array declarations to serialized strings in Wedge, it would improve performance :lol:
Well, of course it's hardly doable, if only because some array entries are dynamic, permission-based or similar, so that would require unsetting them after the unserialize which would waste some time, but... well, it's always good to know! :^^;:

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: These two bytes may not matter to you...
« Reply #119, on September 30th, 2011, 10:46 AM »
- Yeah, I've seen enough copied code in my time.

- The only reason a host would disable glob is out of some misguided notion of security, and frankly, if they're doing that, best to move hosts to someone competent.

- Good. Every little thing that isn't needed, should go.

- Sounds like the right decisions have been made. Unfortunately I don't have the luxury of just ripping code out with phpseclib, but I can certainly trim the comments. I did also have to attend to a PHP 5.3.3 backwards-incompatible change while I was at it.[1]

- Hmm, that's interesting! And yeah, that doesn't surprise me, actually, though a goodly number of them are derived from the DB.
 1. phpseclib is 4.x compatible which means although it's in classes, it uses the class name as constructor. As of 5.3.3, those are not going to be called. I just renamed the constructor functions to __construct instead.
« Login/Register Feature
Need some help on a UI matter »