Hi,
I've just spent some time looking at the mission code for alpha2.0. I've said good things about it, now come the nitpicking :D * You need to serialize the whole Mission class, not the individual Mission variables; otherwise you would need to change the mission engine code every time we would need an additional variable in the mission (like hints); see http://www.php.net/manual/en/language.oop5.serialization.php (classes/class_missions.php) [Vlad] I understand, I will do that * To do this, you need to split Missions in two classes: the Mission abstract class, and the MissionsEngine. The second one will hold a list of the missions and their instances, create and call individual missions, save mission objects and restore them, keep track of the action queue... [Vlad] ok, I’m working on it. I will post it to the repository tonight and you can tell me if it’s in order or needs additional modifications * $play_data and $site_data don't belong as variables in missions class, you should create a specific class for each of them (classes/class_missions.php) [Vlad] the site data is already implemented as a class. After I instantiate the site object I hold it in the $site_data variable. If I don’t do that, I will have to instantiate a new site object each time I need a property of a site. Is that efficient? for the $play_data variable I do not have the class implemented. Basically this holds the user/session info, and implementing this in object model will take a significant amount of time. Should I start to work on this now? * The mission engine makes use of global variables through $_SESSION - cf requirements: "New features must be implemented in object-oriented model and must not use global variable." [Vlad] once I serialize the whole mission class, this should be easy * The logic in First_mission->__construct() needs to be moved to the abstract class Mission, otherwise it will have to be repeated in each mission file. This logic should also call $this->clear_action() before calling the method for the action, otherwise it has to be repeated in each method [Vlad] I can move the construct in the abstract class. There is a problem with moving the clear_action() function. This function is not automatically called regardless of the circumstances. For example, in the function home_hacked I have a couple of conditions. If they are not met, the function returns false and the action is not cleared (the mission still waits for the same action). This is why I don’t think that moving the clear action to the constructor will work * This will allow you to store the content of variables such as XXX [Vlad] not sure what is this referring to * As discussed previously, you need to do SQL requests through data <http://pear.php.net/manual/en/package.database.db-dataobject.php> objects - so you need to change methods such as Missions->give_reward() or Missions->replenish_hackscan_points() that don't follow this requirement. [Vlad] both give_reward and replenish_points functions are linked to the user/session tables. And for me to use data object I must first implement the user class. Or am I missing something here? * Good job on the code documentation, it's very good now * Same remark about the Changelog, it's perfect [Vlad] thank you * in First_mission->getting_an_isp_website_finding_it(): instead of using $_GET['tag'] which is not safe, you should access a First_mission->session->current_site object [Vlad] in the final version the $_GET will not be used. I had to use it for now in order to have the test file going. If you look in the code, you will see that the correct final code is commented out. A lot of these issues are natural when you switch from a functional to an object orientation. If you want and if it helps, you can chose a good book about Object Oriented programming with PHP and I'll order it for you. Or I could open an account for you on http://my.safaribooksonline.com/ - there is for example that book that seems interesting: http://my.safaribooksonline.com/9781590599099 . Just let me know if you're interested! [Vlad] I’m always looking to learn new things and looking at the TOC of the book I think I can learn a great deal from it. Thank you. Xavier.
_______________________________________________ Hackit Bar mailing list - [email protected] Wiki: http://community.hackit.cx/ List: http://community.hackit.cx/ml/ Forum: http://community.hackit.cx/forum/ Ideas: http://community.hackit.cx/ideas/ IRC: irc://irc.freenode.net/#politis
