New revs - Public comments

Nao

  • Dadman with a boy
  • Posts: 16,061
Re: New revs - Public comments
« Reply #45, on August 18th, 2011, 03:44 PM »
All right all right, I'll just get back to work and reply these... :P
Quote from Arantor on August 2nd, 2011, 01:22 PM
Only last night I encountered a change in behaviour between my local PC and the server that my code was deployed onto; it was a schoolboy error on my part for forgetting it (call_user_func_array expects the callee to be specifying its parameters explicitly as references, rather than values, especially given that the caller was using references) - on my 5.2 setup it worked as expected, on a 5.3 box it broke quite interestingly.
It had more bugs than that... I remember fixing a few things when I installed v5.3.
Quote
The reason it doesn't matter in the SMF world is because so much of what goes on is DIY coding rather than letting PHP's own functions do the job. Number formatting, timezone offsets, time generally, etc.
Indeed. It's a bit slower probably, though...
Quote
So, I needed something that would operate during the SD ticket load phase and then proceed to add a template in. Since I'm slightly lazy and had no desire to force yet another file to be loaded when the template was a very simple one, I just put the source and template in the same file.
Yeah, I'd tend to say that's something that's bound to happen, and it's not going to kill anyone.
If you want to encourage users to become modders, then you have to be ready to accept a few quirks like them not following the MVC model. Heck, if it works, and it's nicely written, the MVC stuff isn't a big issue... It starts to become an issue when the program is large. Like Wedge.
Quote
So, getting back to the point: I'd suggest we don't have it load files in template hooks. Any new template will require code to back it up anyway, which means you're going to be loading a file to engage the logic-powering hook anyway, and that new file can load a template if it wants, or it can embed the template if it wants. Then if the mod wants/needs theme specific changes for some random reason, it can be left to the mod author to make it happen if they so desire.
Does that mean you want to remove the template hooks, or simply not change anything and just discourage people from loading files in template hooks? I'm not sure I'm following you...
Quote from Ara Potter on August 7th, 2011, 01:48 PM
It may be version specific but int(4) should do something different to tinyint(4) - in theory, an int(4) should consume at least twice the space of a tinyint(4) and potentially quadruple it; (snip).
Err... I may have forgotten. What were you trying to say? :P
Quote
I don't believe MySQL uses that value for type hinting though, because of the above fact that it could be unreliable, meaning that I think it's always going to allocate 4 byte integers for everything even if it's only a one byte column, so it might be worth glancing through the code sometime to see if we do actually need 4 byte ints everywhere, since I'm not sure.
Oh that would be such a fuuuun job to do.... :whistle:

I'm the one who got stuck with all of the smf/sm renames, so nahh :P
Quote
In other news, at some point we need to make sure that the list of tables that we end up with is in the listed of reserved tables in the DB class(es).
Yes we do... I can already say the media tables aren't in it ;)
Class-DBPackages.php::32
Quote from Ara Potter on August 18th, 2011, 03:30 PM
Nope, because I updated before committing and got your changes first, but mine happened to be nowhere near yours.
By conflict I meant the program stopped because your version wasn't up to date. (It prints out red text and simply says to update it, but per se there's no actual conflict within the file.)
That's one of my specialties these days because you can spend a long time without committing so it's been a long time since I've updated my svn before committing to it ;)
Quote
Incidentally, at some point recently we broke the installer somehow, it never gets onto step 2 :/
Ouch... Much be AeMe. I didn't test the installer after I did all of the changes. But OTOH it should be working, I used the rest of the file as a template...
Quote
More specifically, if there's any kind of error that would force it to fall out of DatabaseSettings(), it's just going back to the start rather than actually showing an error :/
That... Isn't good.

Did you try reverting? That's usually what I do... I revert until it doesn't crash anymore.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #46, on August 18th, 2011, 03:54 PM »
Quote
It had more bugs than that... I remember fixing a few things when I installed v5.3.
That was the main thing I encountered; 5.3 is less forgiving than 5.2.
Quote
Indeed. It's a bit slower probably, though...
It definitely is slower. But it remains to be seen whether it's better or not, because I know there are problems with locale libraries out there.
Quote
Yeah, I'd tend to say that's something that's bound to happen, and it's not going to kill anyone.
If you want to encourage users to become modders, then you have to be ready to accept a few quirks like them not following the MVC model
Exactly. The one thing I will say is I very specifically still kept logic and presentation apart, just I didn't bother making them separate files. The result structure is still maintained, so I don't see a problem with it, provided that I don't do the same thing on larger stuff.
Quote
Does that mean you want to remove the template hooks, or simply not change anything and just discourage people from loading files in template hooks? I'm not sure I'm following you...
Leave the hooks in - you still have to have points to call from. But I'd really rather not encourage people to be loading files there if possible.

For smaller stuff, like the aforementioned, you'll load presentation and logic and execute them. But for anything of any real substance, you'll load the logic separately and pull in a template if you need it, meaning that there's no real harm in leaving the facility for templates to load files but it's a habit that should be discouraged.
Quote
Err... I may have forgotten. What were you trying to say?
Simply that SMF's DB library got it wrong at times and that types aren't always what they seem, such that it might be worth sometime figuring out what the maximum value per column is, to save some space on big forums.
Quote
Oh that would be such a fuuuun job to do....
At least with the renames, you have a finite list to contend with, and it's (reasonably) confined to a find/replace witch hunt. DB fields on the other hand, aren't necessarily so.
Quote
Yes we do... I can already say the media tables aren't in it
I don't think all the ones I've added are there either.
Quote
By conflict I meant the program stopped because your version wasn't up to date. (It prints out red text and simply says to update it, but per se there's no actual conflict within the file.)
No, because I updated it before committing so I didn't get a conflict anyway.
Quote
Ouch... Much be AeMe. I didn't test the installer after I did all of the changes. But OTOH it should be working, I used the rest of the file as a template...
Nope, the changes are prior to any of the changes in install.sql, and even manually reverting the changes in install.php, it still fails.

There is a secondary issue that I'm aware of on my configuration (for reasons I won't get into, I'm running 5.1 with a 5.0 connector library) but it shouldn't be failing how it is, it should be failing with an appropriate error.

Come to think of it, I didn't test with the 5.1 version number but have no idea why that should have caused it to fail the way it does.
Re: New revs - Public comments
« Reply #47, on August 19th, 2011, 02:21 PM »
Oh, and I figured out what the problem with the installer was.

The stock Settings.php file contains a test to see if install.php exists, and if it does, redirect there. Once the DB settings have been validated, that test is removed from Settings.php.

At least in theory, the test used to check for the if statement, plus there being braces either side of the redirection itself, but since it was a one line if branch, the braces got trimmed in r941 and failed to remove the redirect.

Now, I have to see about upgrading my install or hacking the installer test (since while we require 5.1, there's nothing about the connector that will be broken by what I'm doing, but I might as well fix it rather than leave it quasi-broken as it is now)
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,061
Re: New revs - Public comments
« Reply #48, on August 19th, 2011, 06:21 PM »
Oh my, sorry -- it was my fault then... I didn't think these brackets were linked to anything else. To me, they were just disgracious... :^^;:

I'll take care of that later.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #49, on August 19th, 2011, 07:17 PM »
Nah, I fixed it by loosening the check in the installer so it doesn't demand the brackets.

Nao

  • Dadman with a boy
  • Posts: 16,061

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #51, on August 19th, 2011, 07:41 PM »
I already committed the fix hours ago :P

This is what prompted me to muck about with my setup because the installer worked as expected, and failed to allow me to finish installation.

Nao

  • Dadman with a boy
  • Posts: 16,061

Arantor

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

Nao

  • Dadman with a boy
  • Posts: 16,061
Re: New revs - Public comments
« Reply #54, on August 19th, 2011, 11:15 PM »
I looked at your commit... Is it normal that you reverted my change to the MySQL version test...?
I only made that to avoid calling a useless eval()...

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #55, on August 19th, 2011, 11:17 PM »
Quote
I looked at your commit... Is it normal that you reverted my change to the MySQL version test...?
I know I did that at one point to see if it made a difference to how it was being evaluated, but no, that wasn't intentional, the only actual change should be to the function that updates Settings.php to strip the redirection.

Nao

  • Dadman with a boy
  • Posts: 16,061
Re: New revs - Public comments
« Reply #56, on August 20th, 2011, 09:53 AM »
So, I reverted your revert. ;)

Oh, I fucking hate IE... I'm looking at the post editor under IE7 and IE8. It's so incredibly broken, especially in Wysiwyg... There are glitches everywhere, it's comical. I wouldn't even know where to start.

And I fucking don't want to fix them!

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #57, on August 20th, 2011, 11:31 AM »
Quote
Oh, I fucking hate IE... I'm looking at the post editor under IE7 and IE8. It's so incredibly broken, especially in Wysiwyg... There are glitches everywhere, it's comical. I wouldn't even know where to start.
This is really why we need to take a long look at how we approach WYSIWYG, and HTML in general. It seems to me that replacing the current editor with something like CKEditor would be a good idea but that we then need to decide whether we're keeping bbcode or not. (It is possible to hybrid them, as xenForo does)

Nao

  • Dadman with a boy
  • Posts: 16,061
Re: New revs - Public comments
« Reply #58, on August 20th, 2011, 12:17 PM »
I don't know, but I don't think we'll have CKEditor at all in v1.0... It'll probably take too long to implement. Or we'll have to postpone 1.0 until mid-2012... And I don't think people will like that. (If we're only doing Wedge for ourselves, okay, but I think we're past that aren't we...? :^^;:)

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: New revs - Public comments
« Reply #59, on August 20th, 2011, 12:25 PM »
I was able to get a limited version working in SMF in under an hour. The biggest part is marrying up the stuff in wedit, like the bbc converter, and having Wedge control what buttons there are rather than CKEditor doing it itself.

That sort of assumes we stick with bbc and have it perform the same kind of mutant bbc/HTML mashup that the current editor does because complex bbc can't be represented in simple HTML and vice versa, like the quote and gallery bbcode items.