Welcome, Guest. Please login or register.

Pages: [1]
Print
Author Topic: Dialog Demo Patch  (Read 5455 times)
aspidites
Community member

Posts: 29


View Profile Email
« on: November 27, 2010, 09:38:48 AM »

Simplifies a couple of methods, and shows choices as index 1 instead of 0.

* dialog_demo.patch (1.26 KB - downloaded 259 times.)
Logged
mvBarracuda
Admin
Community member

Posts: 1308



View Profile Email
« Reply #1 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.

Ticket: http://parpg-trac.cvsdude.com/parpg/ticket/272
Logged
aspidites
Community member

Posts: 29


View Profile Email
« Reply #2 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

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

Code:
not box.isOpen()

a convenience method can be written as:
Code:
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.

* dialogue_demo_r2.patch (2.7 KB - downloaded 273 times.)
Logged
mvBarracuda
Admin
Community member

Posts: 1308



View Profile Email
« Reply #3 on: November 28, 2010, 08:58:12 AM »

Coolio, posted it at trac:
http://parpg-trac.cvsdude.com/parpg/attachment/ticket/272/dialogue_demo_r2.patch

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.
Logged
Technomage
Admin
Community member

Posts: 80



View Profile
« Reply #4 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.
Logged

"There are more atoms in the period at the end of this sentence than you can count in a lifetime." Science is awesome.

aspidites
Community member

Posts: 29


View Profile Email
« Reply #5 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.
Logged
Pages: [1]
Print
Jump to: