News:

Want to get involved in developing SMF, then why not lend a hand on our github!

Main Menu

Best Answer

Started by Avalanche91, December 05, 2013, 09:09:24 AM

Previous topic - Next topic

Arantor

QuotePHP added this quite some time ago along with the new syntax for arrays

That's PHP 5.4, however in this case that's not what the issue is. Using numbers to pull array indices has been standard forever, and interesting recently they changed how array handling worked under the hood so that you could access strings with array notation, e.g. $string[0] would return the first character of a string even though $string was not an array.

As part of the changes in 5.4, they also brought in the ability to access the result of a function via an array directly, which is what you're doing here. But there are some interesting quirks around using variable functions (which is what $smcFunc is) and array syntax and that's why it's much better to use list rather than trying to directly address it.

There has been, AFAIK, one vulnerability in PHP itself since 5.4 because of this.

Also please note that SMF 2.0 technically still supports the later 4.x releases of PHP, with reduced functionality in some places, and 2.1 is currently requiring 5.1 minimum, so 5.4+ syntax is not recommended.

* Arantor Beeblebrox the First will have to see about renewing his Zend certification in the new year, depending on whether they've updated for 5.4 or 5.5, currently only actually certified for 5.3
Holder of controversial views, all of which my own.


Avalanche91

Quote from: littlenicki on December 05, 2013, 12:38:30 PM
Hello, this is an excellent mod! Could you please do a SMF 1.1 verson of it? Thanks so much!
Thanks for the support.

I love to make things compatible and accessible for everyone and everything. I am sure that a version for SMF 1.x would be very helpful but it is very discouraging to hear that the official support for 1.x version will soon be cancelled. I guess that I would rather work on translations and fixing already existing problems than going from the scratch again.

Kindred

Why is that discouraging?  1.1.x has reached the end of its lifecycle....   2.0 is out and has been stable for years, and 2.1 will soon be out.
Слaва
Украинi

Please do not PM, IM or Email me with support questions.  You will get better and faster responses in the support boards.  Thank you.

"Loki is not evil, although he is certainly not a force for good. Loki is... complicated."

Avalanche91

Quote from: Kindred on December 05, 2013, 03:15:47 PM
Why is that discouraging?  1.1.x has reached the end of its lifecycle....   2.0 is out and has been stable for years, and 2.1 will soon be out.
I am sorry, I absolutely approve what you stated. For example, I am using integrated hooks for my MOD, which are only supported in v2.x, and if I want to make my MOD compatible with SMF 1.x I will have to use some other way of constructing my MOD which eventually may lead to changes in the entire structure of the MOD. But if I were to make changes for SMF 2.1 or even SMF 3, I would love to do it because I know that this will be the feature of the platform. That's why it is so discouraging.

Arantor

And this is why 1.1.x is being EOL'd. It has been superseded by 2.0 and 2.1. 3.0 will likely be very different under the hood in any case.
Holder of controversial views, all of which my own.


Adrek

One more thing - notification in first message is not showing up. When I replaced this (in Display.template.php):
if ($context['topic_first_message'] == $message['id'] && $context['ba_jump_to_msg'] && ($context['total_visible_posts'] > 4))
to
if ($context['topic_first_message'] == $message['id'] && ($context['total_visible_posts'] > 4))

notification is visible, but it contains wrong message ID
Polskie wsparcie SMF na simplemachines.org

the simplest solution is most likely the right one

Avalanche91

Quote from: phantomm on December 05, 2013, 04:25:31 PM
One more thing - notification in first message is not showing up. When I replaced this (in Display.template.php):
if ($context['topic_first_message'] == $message['id'] && $context['ba_jump_to_msg'] && ($context['total_visible_posts'] > 4))
to
if ($context['topic_first_message'] == $message['id'] && ($context['total_visible_posts'] > 4))

notification is visible, but it contains wrong message ID
I just love you man, I owe you a beer... at least! The problem was coming from Display.php where I, assuming that it contains the id of the topic, was passing $output['id'] to the query that was supposed to find which answer is marked as best. As strange as it seems, it was working at some places and obviously it didn't at ohers, that's why I haven't noticed it. I have added a separate query that finds the real id of the topic and now the links are being built correctly. The job of the piece of code ($context['total_visible_posts'] > 4) is to prevent displaying the link to the best answer if it actually is one of the first four replies.

I have added the fix to the previous version (BestAnswer-v1.1.zip).

Adrek

Thanks for quick fix, but please test is on clean forum :)

URL in notification is still wrong, for example:
now it is: /index.php?topic=100973.msg104379#msg104379
and it should be: /index.php?topic=16431.msg104379#msg104379

in this example - 100973 is ID of my first message in topic
16431 is real topic ID
and Best answer ID is 104379, so basically you need to fix topic ID in URL :)
Polskie wsparcie SMF na simplemachines.org

the simplest solution is most likely the right one

Avalanche91

Quote from: phantomm on December 05, 2013, 05:32:21 PM
Thanks for quick fix, but please test is on clean forum :)

URL in notification is still wrong, for example:
now it is: /index.php?topic=100973.msg104379#msg104379
and it should be: /index.php?topic=16431.msg104379#msg104379

in this example - 100973 is ID of my first message in topic
16431 is real topic ID
and Best answer ID is 104379, so basically you need to fix topic ID in URL :)
With pleasure, I just had to replace the variable in Display.template.php. I fixed so many bugs this evening that I decided to drop the first version and leave the most recent one containing least bugs. Thank you guys!

Dzonny

Nice and handy mod, useful to have, thanks! ;)

Biology Forums

Quote from: littlenicki on December 05, 2013, 12:38:30 PM
Hello, this is an excellent mod! Could you please do a SMF 1.1 verson of it? Thanks so much!

There is an excellent one already for 1.x

Avalanche91

Quote from: Liam_michael on December 05, 2013, 11:07:43 PM
There is an excellent one already for 1.x
The reason I decided to create this MOD was due to the fact that I couldn't find such kind of a MOD. May be you should share a link?


Also, I've made another update. Cheers!
QuoteNotification message below the question now shows only if the best answer is third or greater post.

Kindred

Nice job, Avalanche91.

It is good to see a responsive mod author :)
Слaва
Украинi

Please do not PM, IM or Email me with support questions.  You will get better and faster responses in the support boards.  Thank you.

"Loki is not evil, although he is certainly not a force for good. Loki is... complicated."

4Kstore

now work great but I don't like that the mod add 2 query for each reply on the topic:



If the topic have 6 replies
Without the mod the topic load with 13 querys
With the mod the topic load with 29 querys.

If the topic have 7 replies
Without the mod the topic load with 13 querys
With the mod the topic load with 32 querys.

¡¡NEW MOD: Sparkles User Names!!!

Avalanche91

Hello 4Kstore and many thanks for your interest. I feel so embarrassed, I was in such a rush to finish the MOD that I totally forgot to put some if statements to lower the database usage. Could you please tell me what kind of tool are you using to track the forum performance, that picture looks very useful?

I am using 4 queries;
- the first one is to figure out which topic I am going to deal with (id_topic from DB);
- the second one is to find if this topic contains a post marked as best answer;
- the third one I added this morning to help me prevent from the displaying the notification message below the first post (the question);
- the fourth one contains... I was about to describe what was the fourth query doing when I took a more precise look on it and I saw that I can actually combine it with my second query.

I think that there should be only 3 queries in total that the MOD will be using now. Could you please test the version that I published as BestAnswer-v1.1.zip or at least give me a tip how to manually track the queries that are being executed.

Arantor

QuoteCould you please tell me what kind of tool are you using to track the forum performance, that picture looks very useful?

Add $db_show_debug = true; to your Settings.php file. This will give you the entire debug output.

OK, I'm going to have to look at what you're doing but it sounds like you're putting it in prepareDisplayContext, which is run once per post.

Quote- the first one is to figure out which topic I am going to deal with (id_topic from DB);

Use the global $topic for that.
Holder of controversial views, all of which my own.


Avalanche91

Quote from: Arantor Beeblebrox the First on December 06, 2013, 02:15:39 PM
Use the global $topic for that.
Thanks, this is exactly what I needed to drop one more query.  :) So now the MOD is using only two queries in total.
Btw $topic doesn't seem to work in Display.template.php but at least I am able to assign it to a variable in Display.php.

Arantor

You're not supposed to do anything other than purely presentation stuff in the templates. All main logic should already have been done in the main Display.php.

Don't forget that earlier in Display.php, there's a list built of all the messages going to be displayed to the user, look before the point where $messages_request is declared, you'll see an array (off hand I think it's actually called $messages) which contains all the ids to be loaded.
Holder of controversial views, all of which my own.


Avalanche91

Quote from: Arantor Beeblebrox the First on December 06, 2013, 02:43:58 PM
You're not supposed to do anything other than purely presentation stuff in the templates. All main logic should already have been done in the main Display.php.

Don't forget that earlier in Display.php, there's a list built of all the messages going to be displayed to the user, look before the point where $messages_request is declared, you'll see an array (off hand I think it's actually called $messages) which contains all the ids to be loaded.
Yep, indeed. This is the very idea of MVC.

The array is actually called $message, and it contains the information about the posts. Unfortunately it doesn't hold anything such as id_topic. But it's not a problem, I already have the id of the topic stored in the database (the table that I am using to store the best answers).  :)

Arantor

Thanks for ignoring what I was trying to tell you.

1. I am not referring to $message which is a per-post item created during the depths of prepareDisplayContext. This is why you are generating queries per post.

2. I mentioned $messages, not $message, as an array of the message ids that will be fetched for the current post. I only didn't tell you where to look because I was in the middle of something else. Look at Display.php, line 814 onwards.


Using this information, let's be *really* clever and avoid extra queries.

The query on line 180 fetches the topic data. Fetch the msg id as the 'best answer' in that query, no extra queries required. This will be pushed into $topicinfo which is global.

Then by the time you get to the messages query stuff at line 814, you'll already know whether or not you have the best answer. But even if you didn't worry about that, you will have the topic's 'best answer' id available in $topicinfo, which means by the time you get to prepareDisplayContext, you don't have to do any queries but simple comparisons.
Holder of controversial views, all of which my own.


Advertisement: