Post-Apocalyptic RPG forums

Development => Programming => Topic started by: aspidites on November 27, 2010, 09:38:48 AM

Title: Dialog Demo Patch
Post by: aspidites on November 27, 2010, 09:38:48 AM
Simplifies a couple of methods, and shows choices as index 1 instead of 0.

Title: Re: Dialog Demo Patch
Post by: mvBarracuda on November 27, 2010, 02:01:08 PM
Coolio, I've created a ticket for it. Review should go pretty fast, so as soon as a programmer the time to look over it, it should go into SVN if no problems are found.


Title: Re: Dialog Demo Patch
Post by: aspidites on November 28, 2010, 06:28:43 AM
Another patch with the following changes:
1. removed setup_logging function - seems like needless encapsulation, so just called logging.basicConfig() directly

2. swapped OPENED and CLOSED - to correspond with bool values (see next point)

3. changed isOpen to return bool value of self.state. (1 evaluates to True, 0 evaluates to False)

4. removed isClosed - seems unnecessary, since you can check for not isOpen

5. made tuple of except clause on line 80 - without this, exceptions are not evaluated properly and improper errors crash script

6. combined print statement and input function into combined raw_input function - generally raw_input should always be used

7. check for command line arguments before asking which sample to run - if a dialog file is given, there is no point in asking which sample to run

8. changed return value of open and close methods to strings - the whole point of isOpen/isClosed is to check the state, so returning state in these methods seemed pointless

Point 2, 3, and 4 are more so a preference thing than anything else, so feel free to strip them from the patch. If:
is more desirable than:

not box.isOpen()

a convenience method can be written as:
def isClosed():
    return not self.isOpen()

Also note that I could have continued to use a range, but that would have required explicit casting to bool() when returning the value.

Title: Re: Dialog Demo Patch
Post by: mvBarracuda on November 28, 2010, 08:58:12 AM
Coolio, posted it at trac:

Unfortunately we haven't found a way to configure trac so it only blocks the spam but not the ham. Sometimes valid tickets get through, like the the one you recently filled, but other times, they get blocked. I guess the best approach is to post patches at the forums for now.

Title: Re: Dialog Demo Patch
Post by: Technomage on December 02, 2010, 09:30:55 AM
Just saw this. Good point about the boolean issue, in fact I wonder why there's a "self.state" variable at all. I think I'll just replace self.state, isOpen, isClosed and the state constants with a single "self.opened" boolean attribute (seems more "Pythonic" to me).

Main section is definitely improved, though why combine the print and raw_input statement? I haven't used raw_input much, so I'm not aware of its associated idioms.

Title: Re: Dialog Demo Patch
Post by: aspidites on December 03, 2010, 07:41:06 PM
Agreed about the state variable.

As for raw_input, I changed it to that for consistency (see line 83 in get_reply function).

The difference between raw_input and input is that raw_input always assumes input is a string, whereas input essentially eval()'s the input. So, if rather than a number, an unquoted string is passed, you will get NameErrors.