Converting Python Scripts to meet PEP-8 (and bugs)

Announcements and discussion regarding any projects related to Cyan Worlds' Plasma Engine including (but not limited to) CyanWorlds.com Engine, Drizzle, OfflineKI, PyPRP, and libHSPlasma.

Re: Converting Python Scripts to meet PEP-8

Postby tachzusamm » Tue Jul 23, 2013 7:21 pm

johnsojc wrote:Alas, I have found another bug in xAgeSDLBoolRespond.py. It occurs when you are viewing one particular screen on the control imager in Er'cana (the one where you can see the grinding wheels). On switching to that screen, value become 2 when it should only be 0 or 1. This make the tuple index go out of range and triggers a traceback.
[..]Once this happens, none of the control buttons on any screen will display. The 4th, 5th, 6th and 7th lines above are print statements I added to try to figure out what is going on.

My first theory is that in the old script, the test is if value is TRUE. an integer value of 2 returns TRUE in if ageSDL[stringVarName.value][0]:. Your script uses the integer value of ageSDL[sdlName.value][0] as an index. On the one screen, ageSDL[sdlName.value][0] returns the value 2.

[..] I put my hack in after the PtDebugPrint() so the actual value of ageSDL[sdlName.value][0] is displayed.

I must say I'm impressed by your detective work, and how carefully you avoid to cause other issues. Especially when I read the last sentence. That proves you're thinking about what you do. You're doing a good job here.
User avatar
tachzusamm
 
Posts: 575
Joined: Thu May 29, 2008 2:03 am
Location: Germany

Re: Converting Python Scripts to meet PEP-8

Postby johnsojc » Tue Jul 23, 2013 7:50 pm

Well, I did code maintenance for a living back in the dim days. But it's been 10 years since I really did any work with software but the rules are still the same... treat any code change as a time bomb just waiting to go off. I'm running a lot of modified code in my testbed but I'm not pushing it up to GitHub. I suspect Adam would like to have the last word on any of my changes so I just pass along what I've figured out.
johnsojc
 
Posts: 246
Joined: Sun Dec 07, 2008 10:27 am

Re: Converting Python Scripts to meet PEP-8

Postby johnsojc » Tue Jul 23, 2013 8:05 pm

Tsar Hoikas wrote:I just recently pushed fixes for sphere rotating issues, and there's also a pull request for invisible quabs on Dirtsand. (Though sometimes they just get stuck on ledges above the kill region)

That's Er'cana issue makes me want to strangle Cyan even more. I have a truckload of "real work" to get done, unfortunately :(

Ahnonay gives me fits. I ran the spheres 3 times solo collecting all the Yeesha pages and journey cloths. As soon as I brought in a 2nd avvie, the invisible quab showed up and the spheres stopped rotating. I did notice that using fly mode leaves a false pressure plate activation at the spot you flew off from. But then you aren't supposed to use flymode :twisted: .

I know this is supposed to be a PEP8 thread but I need to test the scripts I modify so I keep finding these little issues to fix.
johnsojc
 
Posts: 246
Joined: Sun Dec 07, 2008 10:27 am

Re: Converting Python Scripts to meet PEP-8

Postby Acorn » Wed Jul 24, 2013 2:30 am

Tsar Hoikas wrote:I just recently pushed fixes for sphere rotating issues, and there's also a pull request for invisible quabs on Dirtsand.


great news!
Image
Acorn
 
Posts: 724
Joined: Sun Feb 26, 2012 9:56 am

Re: Converting Python Scripts to meet PEP-8

Postby Wamduskasapa » Wed Jul 24, 2013 2:50 am

Acorn wrote:
Tsar Hoikas wrote:I just recently pushed fixes for sphere rotating issues, and there's also a pull request for invisible quabs on Dirtsand.


great news!

THANK-YOU
Computer = MotherBoard MSI X99S GAMING 7 - Intel I7-6950X
Dual MSI GeForce GTX 1080
64GB Kingston HyperX DDR4 Predator Memory
Dual Samsung 1TB SSD Pro - Dual Seagate 4TB SSHD
Excelvan 5.25" Multi-Function Media Dashboard
User avatar
Wamduskasapa
 
Posts: 943
Joined: Fri Apr 30, 2010 6:56 am

Re: Converting Python Scripts to meet PEP-8

Postby johnsojc » Wed Jul 24, 2013 6:18 am

Tsar Hoikas wrote:I did however manage to finish looking over your first commit.

Ack! I missed seeing this reply until I went looking for something else back in the thread. I made note of your preferences and I will start working on them. I guess we'll just settle for mostly PEP-8 compliant. ;)

EDIT: and ACK! again. I totally hosed up my local repositories and had to rebuild them from scratch. I think I got everything put back correctly and synced with the online copies. User error <bleep>. I also found your comments on a page I normally do not see. Bear with me while I climb this steep learning curve. Accept apologies for lack of skill in Python... I learn as I go.

EDIT2: OK, I crawled through all the SDL files again fixing the indent issue and fixing up a few oversights I noticed. They have been pushed to my pep8 branch up on GitHub but I have not done a PR on them.

Since I've only done about 35 python scripts, I think it would be easier if I simply started over from scratch. (Just have to remember all the little fixes we've come up with.)

I've been looking through your comments. With the exception of the typos I made, poor use of isinstance(), and some misuse of parens to wrap long lines, this is not my code. I will research the language to understand and try to implement your changes.
johnsojc
 
Posts: 246
Joined: Sun Dec 07, 2008 10:27 am

Re: Converting Python Scripts to meet PEP-8

Postby johnsojc » Thu Jul 25, 2013 8:03 am

I just tried Ahnonay again with miserable results. I used the ahnyQuabs script you pushed 12 days ago (the miscount avvie as quab one).

I cleared all the quabs, then went to get a 2nd character to try the rotate to get to sphere 4 but when the 2nd avvie arrived, all 20 quabs had respawned after only a few minutes. Are there pending fixes to Ahnonay that I haven't seen yet?

Update on PEP8 work. I've clean up the SDL files and have processed 28 of the scripts (ignoring long lines). When I get back to where I was before I started over, I will push the new cleaned up scripts (sans touching the license and stripping commented-out code) up to my pep8 branch... or sooner if you want what I have done so far. I plan to go through them again to apply any of the changes you suggested and make sure I used isinstance() correctly.

Question on Python:
In ahnyLinkGUIPopup.py you questioned whether def HideBook(self, islinking=0) should be def HideBook(self, islinking=false).
The only call in the module is self.HideBook(1). Is islinking=0 used to initialize a default value and does the call override it? My skewed logic tells me the reason they used 0 was to make isLinking an int since the call uses an int value of 1.
johnsojc
 
Posts: 246
Joined: Sun Dec 07, 2008 10:27 am

Re: Converting Python Scripts to meet PEP-8

Postby Tsar Hoikas » Thu Jul 25, 2013 12:17 pm

johnsojc wrote:I cleared all the quabs, then went to get a 2nd character to try the rotate to get to sphere 4 but when the 2nd avvie arrived, all 20 quabs had respawned after only a few minutes. Are there pending fixes to Ahnonay that I haven't seen yet?


Ugh, I saw this bug on Gehn a few weeks ago. I think it's another symptom of some of the SDL problems that DirtSand has (variables not getting saved, blobs getting trashed). I don't really have any leads on why it happens. Usually, kicking the quabs off again lets you proceed.

johnsojc wrote:In ahnyLinkGUIPopup.py you questioned whether def HideBook(self, islinking=0) should be def HideBook(self, islinking=false).
The only call in the module is self.HideBook(1). Is islinking=0 used to initialize a default value and does the call override it? My skewed logic tells me the reason they used 0 was to make isLinking an int since the call uses an int value of 1.


The former is correct. I don't really know why Cyan used an int there. In Python, any name implicitly converts to a boolean. I've already shared that empty sequences (lists, strings, sets, bytes) are False. However, it's better, IMO, to use a boolean when you intend to show two states. BTW in Python, True and False have the first letters uppercase. Cyan defines their own true and false in PlasmaTypes.py, but that's somewhat evil.
Image
Tsar Hoikas
Councilor of Technical Direction
 
Posts: 2180
Joined: Fri Nov 16, 2007 9:45 pm
Location: South Georgia

Re: Converting Python Scripts to meet PEP-8

Postby Sirius » Thu Jul 25, 2013 1:29 pm

Tsar Hoikas wrote:
johnsojc wrote:In ahnyLinkGUIPopup.py you questioned whether def HideBook(self, islinking=0) should be def HideBook(self, islinking=false).
The only call in the module is self.HideBook(1). Is islinking=0 used to initialize a default value and does the call override it? My skewed logic tells me the reason they used 0 was to make isLinking an int since the call uses an int value of 1.


The former is correct. I don't really know why Cyan used an int there. In Python, any name implicitly converts to a boolean. I've already shared that empty sequences (lists, strings, sets, bytes) are False. However, it's better, IMO, to use a boolean when you intend to show two states. BTW in Python, True and False have the first letters uppercase. Cyan defines their own true and false in PlasmaTypes.py, but that's somewhat evil.

Personally, I always assumed the reason was typing 1 or 0 requires tapping a single key, while writing True or False takes 4-5 characters :lol: . About the minor case true and false: maybe they prefer the C++ version which doesn't require holding the shift key...
User avatar
Sirius
 
Posts: 1506
Joined: Mon Jul 26, 2010 4:46 am
Location: France

Re: Converting Python Scripts to meet PEP-8

Postby Deledrius » Thu Jul 25, 2013 7:34 pm

johnsojc wrote:I would love to help with this project but it is painfully obvious to me that I simply do not know enough C++ or Python (of either version) to take it on. I can do the mundane work of cleaning up code style and troubleshoot some of these annoying little bugs... I have always been able to fix existing code whether I know the language well or not (an odd talent but there it is). I still need to figure out just how it all the parts work together.

This work is very much appreciated; cleaner code is easier to maintain, yet with all there is to do it's hard to make time to do something with less immediate and obvious benefits.

johnsojc wrote:At this point, the only traceback error I am getting is for Sharper's Journal.

Code: Select all
(07/22 14:57:58) UnboundLocalError: local variable 'journalContents' referenced before assignment

Is there a specific reason this happens or is it something I could possibly look at trying to fix?

EDIT: OK, I know why this error happens. The question now is whether there is anyway to put the Journal contents in dirtsand or were you trying to change the way this journal works?

EDIT(again): Apparently one line was removed in H-uru's xJournalBookGUIPopup.py:

Code: Select all
journalContents = "I'm an empty book"

If inboxChildList is an empty list, then the compile journal text section falls through without ever assigning any value to journalContents hence you get the UnboundLocalError.


Yup, that was part of a change I made to make the journals more flexible, and I missed that potential path. It assumes that we've added a Global.Journals.Empty entry to the loc files to get that string out of the script, but here we're having a problem when it can't find the entry in the vault for dynamic journals. That section still needs to be re-written to select an appropriate entry for different languages, but despite that we should probably initialize journalContents to an empty string anyway. Thanks for catching and debugging that!
User avatar
Deledrius
Gehn Shard Admin
 
Posts: 1377
Joined: Mon Oct 01, 2007 1:21 pm

PreviousNext

Return to Plasma Development

Who is online

Users browsing this forum: No registered users and 20 guests

cron