Integrated Install Debugging

If you wish to contribute to the Ace Attorney Online Game Creation Engine open source project, or just know more about the way it is developed, this is the place.

Moderator: EN - Forum Moderators

Post Reply
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Integrated Install Debugging

Post by Enthalpy »

This is a weird one. Any advice would be appreciated. I tried to set up the integrated install, and made the test forum. When I tried to access the main page of integrated!AAO, I got this message:
PHP wrote:Fatal error: Class 'UserDataHandler' not found in /directory_name_removed/trunk/common_render.php on line 56
I established that the __autoload function defined in /trunk/common.php was being overwritten by the autoloading framework of PHPBB. Because of this, UserDataHandler was never included. I was puzzled as to why this didn't break the main AAO but still rewrote the __autoload as an spl_autoload_register, which allows for different simultaneous autoload functions. I got this error:
PHP wrote:Warning: require_once(/directory_name_removed/trunk/includes/phpbb\config_php_file.class.php): failed to open stream: No such file or directory in /directory_name_removed/trunk/common.php on line 5

Fatal error: require_once(): Failed opening required '/directory_name_removed/trunk/includes/phpbb\config_php_file.class.php' (include_path='.:') in /directory_name_removed/trunk/common.php on line 5
That file should be handled by the PHPBB internals, not AAO's autoloading function. Once it skipped over the first autoload I had queued up, it should have gone on to PHPBB's autoloading. After I confirmed that PHP was entering the AAO autoload function to look for this file, I didn't have anywhere else to investigate. (Unless I went into the PHPBB internals, which sounds far too messy.)

Does anybody have an idea as to how to fix this? I could leave it as it was but manually include the relevant classes, but that would be equivalent to getting rid of the autoloader.

For what it's worth, I'm using PHPBB 3.1.9, PHP 5.6.21, and MySQL 5.7.12
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
smkramer313
Posts: 128
Joined: Wed Feb 23, 2011 7:18 pm
Gender: Male
Spoken languages: English

Re: Misc. Questions

Post by smkramer313 »

Enthalpy wrote:This is a weird one. Any advice would be appreciated. I tried to set up the integrated install, and made the test forum. When I tried to access the main page of integrated!AAO, I got this message:
PHP wrote:Fatal error: Class 'UserDataHandler' not found in /directory_name_removed/trunk/common_render.php on line 56
I established that the __autoload function defined in /trunk/common.php was being overwritten by the autoloading framework of PHPBB. Because of this, UserDataHandler was never included. I was puzzled as to why this didn't break the main AAO but still rewrote the __autoload as an spl_autoload_register, which allows for different simultaneous autoload functions. I got this error:
PHP wrote:Warning: require_once(/directory_name_removed/trunk/includes/phpbb\config_php_file.class.php): failed to open stream: No such file or directory in /directory_name_removed/trunk/common.php on line 5

Fatal error: require_once(): Failed opening required '/directory_name_removed/trunk/includes/phpbb\config_php_file.class.php' (include_path='.:') in /directory_name_removed/trunk/common.php on line 5
That file should be handled by the PHPBB internals, not AAO's autoloading function. Once it skipped over the first autoload I had queued up, it should have gone on to PHPBB's autoloading. After I confirmed that PHP was entering the AAO autoload function to look for this file, I didn't have anywhere else to investigate. (Unless I went into the PHPBB internals, which sounds far too messy.)

Does anybody have an idea as to how to fix this? I could leave it as it was but manually include the relevant classes, but that would be equivalent to getting rid of the autoloader.

For what it's worth, I'm using PHPBB 3.1.9, PHP 5.6.21, and MySQL 5.7.12
Actually, I created a pull request for this change today. It looks like the problem was that along with the __autoload method being overwritten, those phpBB files didn't exist in the directory that it was looking them up in. After adding a file_exists() conditional inside the __autoload() function, and also incorporating in the spl_autoload_register(), the index page was able to load without any problems.

Here is the pull request, in case you want to test out what I did.
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Re: Misc. Questions

Post by Enthalpy »

Thanks! I'll test the commit tomorrow and see if the details about the cause of the bug make more sense then. It's been so long since I first reported this bug that the details escape me!

I recommend adding a comment explaining why we're doing a file_exists check in the first place, so we can remember why we did that change.

How much programming experience do you have, and what features are you particularly interested in working on? I'd like to check if we're likely to interfere with each other in future. My focus is on removing known bugs, and then improving editor usability. For what it's worth, I'm planning to investigate Issue #108 next.
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
smkramer313
Posts: 128
Joined: Wed Feb 23, 2011 7:18 pm
Gender: Male
Spoken languages: English

Re: Misc. Questions

Post by smkramer313 »

Enthalpy wrote:Thanks! I'll test the commit tomorrow and see if the details about the cause of the bug make more sense then. It's been so long since I first reported this bug that the details escape me!

I recommend adding a comment explaining why we're doing a file_exists check in the first place, so we can remember why we did that change.

How much programming experience do you have, and what features are you particularly interested in working on? I'd like to check if we're likely to interfere with each other in future. My focus is on removing known bugs, and then improving editor usability. For what it's worth, I'm planning to investigate Issue #108 next.
I added in a comment explaining why it was added. As for my programming experience, I know a great deal about JavaScript, but I've been away from it for a while, and I'm just starting to learn PHP.

I currently have the integrated install completed. I initially thought that I had set something up wrong with phpBB when I came across this error on the index page. But it's good to know that that wasn't the case!

I'll probably also be looking into the editor in the future, but for now I'll search for oddities that I can try fixing in the integrated install and the mocked install.
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Re: Misc. Questions

Post by Enthalpy »

That change makes perfect sense now. For some reason, I thought spl_autoload_register silently ignored errors. That looks good, thanks! I'm confused why AAO doesn't have this problem now, but maybe it has to do with me running an newer version of phpBB than AAO probably uses?

EDIT:

...Go to the Manager page. What do you see? I see an error message about super_globals. I've got this one fixed on my machine, but I want to see if you have the problem as well.
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
smkramer313
Posts: 128
Joined: Wed Feb 23, 2011 7:18 pm
Gender: Male
Spoken languages: English

Re: Misc. Questions

Post by smkramer313 »

Enthalpy wrote:That change makes perfect sense now. For some reason, I thought spl_autoload_register silently ignored errors. That looks good, thanks! I'm confused why AAO doesn't have this problem now, but maybe it has to do with me running an newer version of phpBB than AAO probably uses?

EDIT:

...Go to the Manager page. What do you see? I see an error message about super_globals. I've got this one fixed on my machine, but I want to see if you have the problem as well.
Yes, I noticed this problem as well after I navigated around the page. It looked like replacing the $_SERVER and $_GET calls with a request class or request_var like the error suggests will remove those error messages. I can only assume it still works on AAO because the code itself is still valid despite these issues.

However, I also came across a SQL insert error when attempting to create a trial in the Manager because I apparently have a NULL id. For new sequences I also got a warning about an empty needle, so that seems to validate that assumption I have with the SQL table.
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Re: Misc. Questions

Post by Enthalpy »

Fun stuff. To the best of my knowledge, AAO uses phpBB 3.0.9, so the right move is to use the request_var function, not the request class, which is a 3.1 addition. I've already got it working on my machine, but if you want to do some more testing, layout/common.php's getCurrentPageUrl function should be

Code: Select all

function getCurrentPageRedirectUrl()
{
	// phpBB disables _SERVER for security purposes; We use request_var instead
	// TODO: Upgrade to the request class if the forums use phpBB 3.1
	$url = request_var('SCRIPT_NAME', '');
	$query = request_var('QUERY_STRING', '');
	if(!empty($query))
	{
		$url .= '?' . $query;
	}
	
	return $url;
}
I'll take a look at the insert error for creating a new trial in-manager, unless you want to take that one?

EDIT: I'm not getting your null id error when I try to create a new trial/sequence, but an error about super globals, as before. I'll change the _GET calls to request_var and see what happens afterwards.

EDIT 2: And after killing all the super globals errors when I try to make a trial, I'm getting a new, more substantive error. Column count doesn't match value count at row 1. Investigation revealed the cause: Per test_install/db_build.sql, trial insertion should take 11 values, but only 10 are being submitted. Based on the values being passed and when integers are expected, it's the 'collaborateurs' or 'testeurs' field that is missing.

Given that the code we've been given says that v6 trial creations passes to v5 trial creation after doing some error checks, and that v5 had no playtesters, I'm assuming that it's the 'testeurs' field that's missing. That said, I'm skeptical that this error could have been dealt with silently in AAO. I can believe that Unas included some add super globals command to bypass the super globals problems, or that the error didn't exist in 3.0.9. But when trial creation doesn't even have the right number of values? Something is amiss.
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
smkramer313
Posts: 128
Joined: Wed Feb 23, 2011 7:18 pm
Gender: Male
Spoken languages: English

Re: Misc. Questions

Post by smkramer313 »

Enthalpy wrote:Fun stuff. To the best of my knowledge, AAO uses phpBB 3.0.9, so the right move is to use the request_var function, not the request class, which is a 3.1 addition. I've already got it working on my machine, but if you want to do some more testing, layout/common.php's getCurrentPageUrl function should be

Code: Select all

function getCurrentPageRedirectUrl()
{
	// phpBB disables _SERVER for security purposes; We use request_var instead
	// TODO: Upgrade to the request class if the forums use phpBB 3.1
	$url = request_var('SCRIPT_NAME', '');
	$query = request_var('QUERY_STRING', '');
	if(!empty($query))
	{
		$url .= '?' . $query;
	}
	
	return $url;
}
I'll take a look at the insert error for creating a new trial in-manager, unless you want to take that one?

EDIT: I'm not getting your null id error when I try to create a new trial/sequence, but an error about super globals, as before. I'll change the _GET calls to request_var and see what happens afterwards.

EDIT 2: And after killing all the super globals errors when I try to make a trial, I'm getting a new, more substantive error. Column count doesn't match value count at row 1. Investigation revealed the cause: Per test_install/db_build.sql, trial insertion should take 11 values, but only 10 are being submitted. Based on the values being passed and when integers are expected, it's the 'collaborateurs' or 'testeurs' field that is missing.

Given that the code we've been given says that v6 trial creations passes to v5 trial creation after doing some error checks, and that v5 had no playtesters, I'm assuming that it's the 'testeurs' field that's missing. That said, I'm skeptical that this error could have been dealt with silently in AAO. I can believe that Unas included some add super globals command to bypass the super globals problems, or that the error didn't exist in 3.0.9. But when trial creation doesn't even have the right number of values? Something is amiss.
Yeah, I think so as well. I delved further into the php database creation code, and it looks like the INSERT INTO does in fact miss either 'collaborateurs' or 'testeurs'. Adding in another empty string in the appropriate place resolves this issue.

The file I looked into was the TrialMetadataHandlerV5.class.php, which was called from the create() function in the V6 TrialMetadataHandler.class.php. I still have no idea how trial creation would even work, however, given that this error was popping up.

EDIT:

One thing that I'm considering about why this error may not be causing an issue elsewhere on AAO is that Unas may have set up one of these two columns to be nullable. If that is the case, it would explain why this error is only appearing over our ends because we created the liste_proces table with all non-nullable columns. But after making this fix, I can confirm that a new trial is successfully created.

Also, when the user creates a trial without specifying a title, the SQL query still gets executed and the trial is created and added into liste_proces without a title. Needless to say, we should definitely replace this with an appropriate redirection so we don't create excess data.

Code: Select all

		$query = 'INSERT INTO ' . self::$DBV5_TRIALS_TABLE . '
			VALUES(
				NULL,
				"' . $db->sql_escape($title) . '",
				' . intval($author->getId()) . ',
				"' . $db->sql_escape($filename) . '",
				0,
				"",
				"", //Added line here
				"' . $db->sql_escape($language) . '",
				0,
				"",
				0
			)';
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Re: Integrated Install Debugging

Post by Enthalpy »

Trial creation should be impossible if these files are accurate. I'm going to be cautious and wait for Unas to explain this before submitting any fixes to things that aren't problems on the live version of AAO.

Blocking trial creation if the title is blank, though, can be done on live AAO, so I'd be fine with a fix to that.

For your reference, I'm not going to have a computer for a while starting tomorrow.
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Re: Integrated Install Debugging

Post by Enthalpy »

Coincidentally, I tried to run the mocked install on the branch where I had made the changes for the integrated install... Unsurprisingly, PHP couldn't find the request_var function. This will complicate a fix to integrated.

Fun with method_exists, I assume?
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
User avatar
Unas
Admin / Site programmer
Posts: 8850
Joined: Tue Jul 10, 2007 4:43 pm
Gender: Male
Spoken languages: Français, English, Español
Contact:

Re: Integrated Install Debugging

Post by Unas »

AAO forums are currently running phpBB 3.0, which likely explains most of your issues if you tried with 3.1.

phpBB 3.0 didn't interfere with the autoload I defined. Apparently, 3.1 does, so indeed I'll need to pay attention to that. Thanks smkramer for the PR, I'll have a look soon !
Same for the superglobals : I'm guessing phpBB 3.1 disables them out of security and modularity concerns, but as of phpBB 3.0 they are perfectly accessible.

Regarding the trial creation issue, there is simply an error in the db_build.sql file that I wrote ! :-D
The 'testeurs' column shouldn't be there - it never existed in my local database, nor on the one of the website. Not sure what I had in mind when I put it there...
In the V5 DB, testers are stored inside the collab field with a specific syntax.
So your fix is simply to remove the "testeurs" line in the sql file. ;-)
ImageImageImage
If knowledge can create problems, it is not through ignorance that we can solve them.
Si le savoir peut créer des problèmes, ce n'est pas l'ignorance qui les résoudra. ( Isaac Asimov )
User avatar
Enthalpy
Community Manager
Posts: 5169
Joined: Wed Jan 04, 2012 4:40 am
Gender: Male
Spoken languages: English, limited Spanish

Re: Integrated Install Debugging

Post by Enthalpy »

For those who would rather not bother with the various changes needed to get phpBB 3.1 working, an alternate way to download 3.0 is available.
[D]isordered speech is not so much injury to the lips that give it forth, as to the disproportion and incoherence of things in themselves, so negligently expressed. ~ Ben Jonson
Post Reply