-------- Forwarded Message -------- From: Vlad Dragu <[email protected]> To: [email protected] Subject: Re: [Farsides] [Hackit] [Code] Review 14/11/2010 Date: Wed, 17 Nov 2010 09:38:40 +0200
Hi Xavier, i'm having some trouble with my email address and i'm not sure i can post to the ml from a different address. So i'm sending this email to you. Can you please post it to the ml? thanks Vlad " > > Ah, I think this is a misunderstanding: the fact that answers are hidden > doesn't mean that we should hide what the user types - it means that the > user doesn't type anything, he just hits "reply", and imagines what he > typed. Like in this game: http://www.scoutshonour.com/digital/ > [vlad] ok, i got it. i will make the modification > >> [David] Vlad spent quite some time on unit tests : adapting them, but >> also modifying the code itself to be able to run the unit tests. Xav >> when you have a little time it would be interesting if you can discuss >> with Vlad to see if there's a way to make this less time consuming. >> > Proppy would be better suited for the advices here, but I can try to > help - Vlad, can you describe the issues you've encountered? > [vlad] a good exaqmple would be this: my game_session class is a singleton because i need to make sure there's only one instance throught the script execution and also i need to be able to access that instantiated class in the test files i need to instanciate the session class multiple times so i can test how it instyantiates according to different environment variables, but i cannot really do that with a singleton. So i had to add a method in the game session class that destroys the singleton, which i don't really need in the live game and which shouldn't be there anyways acording to the singleton theory. another example still related to this: in the tests i need to mock the hack_result and scan_result functions becuse they return a random result based on a probability. So i construct a class that extends the game_session class and declare the mock function inside them. However you cannot really extend a singleton. the construct function in a singleton is private. So i had just for testing purposes to declare the construc function as protected > > And the comments: > > - I think I forgot to pay attention to this on the schema file, so if > you want we can keep this for the next iteration, but as discussed > before, the Missions class needs to be split between a Mission class > (that is subclassed by the actual missions classes, such as > FirstMission) and a MissionEngine class, that is responsible for > instantiating mission objects, saving/restoring them (methods such as > save_state()) or calling the right method corresponding to an action > (process_action_queue). > => this will allow to only expose methods in Mission that can actually > be needed by someone scripting a mission, not the underlying magic. It > also gives more flexibility, allowing a player to run several missions > at the same time for example > [vlad] right now, i have the class_mission_action class that plays the role of mission engine and fits with the observer pattern And then i have the mission abstract class and the missions themselves. I'll try to move the functions that were not used by the mission scripts. I know that i got some problems when i first tried to do that and that's why i decided to keep them in the ebstract class, but this was before using the game_session as a singleton, so i'll have to take another look and see how i can accomplish that > - The session is not the right place to store mission state - if the > session is reset, the player will lose its current mission state. It > should be stored in the database. > [vlad] understood, i'll do that > - Some problems with tabs and spaces - see for example > http://bazaar.launchpad.net/~vlad-dragu/hackit/alpha2.0/annotate/124/simpletest/test/hackit_user_class.php to see what I mean. You may want to run http://pear.php.net/manual/en/package.php.php-codesniffer.php to ensure compliance with PEAR standards. > [vlad] ok, probably i should do it afteri do the other modifications > - Not sure if you did this: when you need to rename a file (for example > first_mission.php => FirstMission.php), use "bzr mv first_mission.php > FirstMission.php". It allows bazaar to understand what is going on, and > keep the file history instead of creating a new file. > [vlad] no, i didn't do it. Is there any way to do it after the fact? So, after i renamed it, can i run this command or is it too late? > - Why do you access a GET variable to get the hack results in > FirstMission, instead of using an object property? This relates to a > part of the codebase that I don't know very well, so I'm unsure. You > never trust anything that directly comes from the client, right? > [vlad] that GET var is not coming from client input. When the player does an action in the game, i use an ajax call to alert the mission engine that an action has been taken. The ajax call is a post call actually and that's where the var comes from. I get the hack results from the game session > - Need to remove the "return true" from mission file - this should be > the default, no need to add an extra line - we can just put "return > false" when needed. You should also clear out the commented code in > FirstMission.php. > [vlad] understood, i'll do that "
_______________________________________________ Farsides mailing list - [email protected] Wiki: http://farsides.com/ List: http://farsides.com/ml/ Forum: http://farsides.com/forum/ Ideas: http://farsides.com/ideas/ Chat: http://farsides.com/chat/
