Pink Petal Games

PyTFall => PyTFall: Bugs and Game balancing => Topic started by: rudistoned on June 15, 2013, 11:50:04 AM

Title: Code review
Post by: rudistoned on June 15, 2013, 11:50:04 AM
Hey guys, I thought we could start a thread to discuss changes to the code (if we think they need discussing).

I noticed that the current revision of master (6cc3e2d77e1e38cd327d244b6c2f19c7d138a770) throws the following error when trying to load image files on windows:

Quote
I'm sorry, but an uncaught exception occurred.

While running game code:
  File "game/library/screens/pyt - screens - nextday.rpy", line 6, in script
  File "game/library/screens/pyt - screens - nextday.rpy", line 130, in python
  File "game/library/classes - characters.rpy", line 668, in python
  File "game/library/functions.rpy", line 148, in python
Exception: Backslash in filename, use '/' instead: u'D:\\PyTFall\\renpy-6.15.4-sdk\\pytfall/game\\content/chars/naruto/Sakura/strip (1).png'

Revision a9e216809559bd2a7f98906cef19762116e0e7b7 (the one the "tags" branch is based on) does not have this problem, so it was probably introduced since.

If one of you feels like fixing it, the following function might come in handy. However, once we use the TagDatabase to get paths to image files, the image loading code will be changed anyway, so you could just ignore it for now.
Code: [Select]
    def normalize_path(path, start=""):
        '''Returns a filesystem path following the conventions of the local OS.
       
        If start is empty, an absolute path will be returned.
        If start is not empty, a path relative to start will be returned.
        '''
        path = os.path.normpath(path)
        # determine an absolute version of path
        if os.path.isabs(path):
            abspath = path
        else:
            abspath = os.path.normpath(os.path.join(gamedir, path))
        # return the normalized path
        if start=="":
            normpath = abspath
        else:
            startpath = os.path.normpath(start)
            normpath = os.path.relpath(abspath, start=startpath)
        return normpath
Title: Re: Code review
Post by: Xela on June 15, 2013, 11:53:09 AM
Hey guys, I thought we could start a thread to discuss changes to the code (if we think they need discussing).

I noticed that the current revision of master (6cc3e2d77e1e38cd327d244b6c2f19c7d138a770) throws the following error when trying to load image files on windows:

Revision a9e216809559bd2a7f98906cef19762116e0e7b7 (the one the "tags" branch is based on) does not have this problem, so it was probably introduced since.

If one of you feels like fixing it, the following function might come in handy. However, once we use the TagDatabase to get paths to image files, the image loading code will be changed anyway, so you could just ignore it for now.
Code: [Select]
    def normalize_path(path, start=""):
        '''Returns a filesystem path following the conventions of the local OS.
       
        If start is empty, an absolute path will be returned.
        If start is not empty, a path relative to start will be returned.
        '''
        path = os.path.normpath(path)
        # determine an absolute version of path
        if os.path.isabs(path):
            abspath = path
        else:
            abspath = os.path.normpath(os.path.join(gamedir, path))
        # return the normalized path
        if start=="":
            normpath = abspath
        else:
            startpath = os.path.normpath(start)
            normpath = os.path.relpath(abspath, start=startpath)
        return normpath

I've just pushed and a fix for the windows path should be there as well.
Title: Re: Code review
Post by: rudistoned on June 20, 2013, 04:39:24 PM
Code: [Select]
max_page = len(content) / page_size if len(content) % page_size not in [0, page_size] else (len(content) - 1) / page_size

I've never seen this syntax before. Am I correct in assuming that "len(content) / page_size" is assigned to "max_page" if "len(content) % page_size not in [0, page_size]" and if not "(len(content) - 1) / page_size"  is assigned to "max_page"? If so, why not write it like this:
Code: [Select]
if len(content) % page_size not in [0, page_size]:
    max_page = len(content) / page_size
else:
    max_page = len(content) - 1) / page_size
Title: Re: Code review
Post by: Xela on June 20, 2013, 05:40:32 PM
Code: [Select]
max_page = len(content) / page_size if len(content) % page_size not in [0, page_size] else (len(content) - 1) / page_size

I've never seen this syntax before. Am I correct in assuming that "len(content) / page_size" is assigned to "max_page" if "len(content) % page_size not in [0, page_size]" and if not "(len(content) - 1) / page_size"  is assigned to "max_page"? If so, why not write it like this:
Code: [Select]
if len(content) % page_size not in [0, page_size]:
    max_page = len(content) / page_size
else:
    max_page = len(content) - 1) / page_size

Feel free to swap, either way works :)
Title: Re: Code review
Post by: Xela on June 22, 2013, 03:52:33 PM
@ Rudi/Matt

Take a look at this function:

Code: [Select]
    # Throwing dice. value = chance, max - limits
    # if dice(60): means probability of 60%
    def dice(value, max=100, show=True):
        number = random.randrange(0,max+1)
        if number <= value:
            result = True
        else:
            result = False
        if config.developer and show:
            notify(u"Resulted in %d from %d, limit - %d, and result - %s." % (number,max,value,str(result)))
        return result

This is from Alkion (written by Roman, not me) but this seems flawed because it actually picks from 101 numbers and not 100. Can any of you confirm this, I ran this on pure python 2.7 and it seems to agree with me.

Code should be:

Code: [Select]
    # Throwing dice. value = chance, max - limits
    # if dice(60): means probability of 60%
    def dice(value, max=100, show=True):
        number = random.randrange(1, max+1)
        if number <= value:
            result = True
        else:
            result = False
        if config.developer and show:
            notify(u"Resulted in %d from %d, limit - %d, and result - %s." % (number,max,value,str(result)))
        return result

Otherwise dice(1) actually means 2% chance because number variable can be anything in range of 0 to 100 (BUT INCLUDING 0 as well)  and same counts for any other throw? What do you think?
Title: Re: Code review
Post by: rudistoned on June 23, 2013, 04:15:57 AM
You are correct. Number should be anything between 1 and 100. That way, 0 will be False, 1 will have 1% chance for True, 99 will have 99% for True and 100 will always be True.

I would prefer to write the function this way:
Code: [Select]
def throw_dice(value, limit=100, show=True):
        '''Returns True if value is <= a random integer between 1 and limit.

        If limit is 100, this means that a value of 60 has a 60% chance for True
        '''
        number = random.randint(1, limit)
        if number <= value:
            result = True
        else:
            result = False
        if config.developer and show:
            notify(u"Resulted in %d from %d, limit - %d, and result - %s." % (number, limit, value, result))
        return result
randint is simpler than randrange and does exactly what we want. limit is a better keyword than max because it does not overwrite a stdlib function name. rolldice or roll_dice are better function names as those names represent the activity of the function better. Documentation on how to use the function should be in the docstring rather than in comments.

Title: Re: Code review
Post by: Xela on June 23, 2013, 04:52:11 AM
You are correct. Number should be anything between 1 and 100. That way, 0 will be False, 1 will have 1% chance for True, 99 will have 99% for True and 100 will always be True.

I would prefer to write the function this way:
Code: [Select]
def throw_dice(value, limit=100, show=True):
        '''Returns True if value is <= a random integer between 1 and limit.

        If limit is 100, this means that a value of 60 has a 60% chance for True
        '''
        number = random.randint(1, limit)
        if number <= value:
            result = True
        else:
            result = False
        if config.developer and show:
            notify(u"Resulted in %d from %d, limit - %d, and result - %s." % (number, limit, value, result))
        return result
randint is simpler than randrange and does exactly what we want. limit is a better keyword than max because it does not overwrite a stdlib function name. rolldice or roll_dice are better function names as those names represent the activity of the function better. Documentation on how to use the function should be in the docstring rather than in comments.

Great, thanks for your input, I like the name, just don't like the range error.
Title: Re: Code review
Post by: mijh on June 23, 2013, 09:51:16 AM
I was going to rewrite and rename the dice function to something like this:
Code: [Select]
def chance(value):
    """Randomly generated percentage chance to return True"""
    return (value / 100.0) > random.random()

Given that the extra arguments are NEVER used anywhere, that's all it needs to be.

Neither does a chance out of 100 have anything to do with dice...
Title: Re: Code review
Post by: Xela on June 23, 2013, 09:53:22 AM
I was going to rewrite and rename the dice function to something like this:
Code: [Select]
def chance(value):
    """Randomly generated percentage chance to return True"""
    return (value / 100.0) > random.random()

Given that the extra arguments are NEVER used anywhere, that's all it needs to be.

Neither does a chance out of 100 have anything to do with dice...

Hey, actually it can be useful if you want to throw out of 1k (very small chance). Leave it as it is now.
Title: Re: Code review
Post by: mijh on June 23, 2013, 09:56:59 AM
Like....
Code: [Select]
chance(0.1)?

A dice() function should return the number of a dice throw, not True or False, anyway. People don't know how to name functions :S
Title: Re: Code review
Post by: Xela on June 23, 2013, 10:04:55 AM
Like....
Code: [Select]
chance(0.1)?

A dice() function should return the number of a dice throw, not True or False, anyway. People don't know how to name functions :S

LoL

I just don't want to get used to writing chance instead of dice (and dice is shorter  :P )
Title: Re: Code review
Post by: rudistoned on June 23, 2013, 10:08:07 AM
Neither does a chance out of 100 have anything to do with dice...
It seems you never played a pen&paper RPG (or maybe only those without percentage rolls?)  ;)
Title: Re: Code review
Post by: rudistoned on June 26, 2013, 08:23:06 AM
As you prolly have figured out by now, game is coded this way:

1) All logical game elements are coded in pure python. We access, display and change those in RenPy labels.

2) User interactions:

- We jump to a RenPy 'label' (labels are RenPy's way of running the game, instead of the usual main game loop you'd expect to find in Hentai Sims, we simply jump labels until we run out of them (we can obviously keep jumping to the same label any amount of times))
- In that label, we set up background and call screens (screens are coded in RenPy's screen language), at the same time we lock game's flow in a while loop.
There is a number of things that are not smart to include in screens, mainly because screens are being refreshed on all user interactions, once every 20 secs (If I recall correctly) or even many times per second if game requires them to. Another thing is that certain functions (like action calls from buttons) are not searching for variables on the same namespace as the screen first (very unusual for Python).

Problem was here:
Code: [Select]
                        for brtl in hero.brothels:
                            if brtl.highlighted:
                                $imgType = im.Scale
                            else:
                                $imgType = lambda *args: im.Sepia(im.Scale(*args))
                            vbox:
                                null height 20
                                text(u'{size=-10}%s' % brtl.name) align(0.5,0.5)
                                use rtt_lightbutton(img = imgType(brtl.img, 240, 150), return_value = ['brothel', brtl], tooltip = 'Send %s off to work in %s and choose from available actions.' % (chr.name, brtl.name))

In precious revision brothel was used to iterate instead of brtl
Code: [Select]
for brothel in hero.brothels: and at the same time we also had brothel variable in the label and while loop namespaces:

Code: [Select]
label girl_assign:
    scene bg wood
    hide screen girlProfile
    show screen pyt_girl_assign

    python:
       
        if isinstance(chr.location, Brothel):
            brothel = chr.location
            brothel.highlighted = True
            brothel_selected = True
        else:
            brothel = None
            brothel_selected = False

        while True:
            result = ui.interact()

            if result[0] == 'brothel':
                for brothel in hero.brothels:
                    brothel.highlighted = False

                brothel = result[1]
                brothel.highlighted = True
                brothel_selected = True

so this bit of old code:
Code: [Select]
return_value = ['brothel', brothel] didn't actually return a brothel that came from iterating over hero.brothels in the screen (something any Python coder would expect, it actually works exactly like you'd expect something in Python to work since rtt_lightbutton itself is defined on the same namespace as label (it is in fact for all intent and purposes a separate screen itself), but it is still a bit confusing) but instead returned whatever brothel variable was set to in the label. As you can imagine, this is in no way a problem with one brothel but a rather nasty issue with multiple brothels that doesn't throw an Error making it very hard to catch.

In any case this is what I believe was wrong with the code

Well, I just checked because it would be horrifying if renpy would do something like that. Luckily for us, I can not confirm what you say. The change from "brothel" to "brtl" in the for loop (commited in c11e60d...) does not change which brothel is returned by "return_value = ['brothel', brothel]".
How did I check this? I added the following lines to the girlassign screen:
Code: [Select]
------------- game/library/screens/pyt - screens - girlassign.rpy -------------
index ef6f4d6..3b4b973 100644
@@ -4,7 +4,7 @@ label girl_assign:
     show screen pyt_girl_assign
 
     python:
-
+        devlog.info("label girl_assign")
         if isinstance(chr.location, Brothel):
             brothel = chr.location
         else:
@@ -17,9 +17,11 @@ label girl_assign:
 
             if result[0] == 'brothel':
                 for brothel in hero.brothels:
+                    devlog.info("brothel at %i: %s" % (hero.brothels.index(brothel), brothel.name))
                     brothel.highlighted = False
 
                 brothel = result[1]
+                devlog.info("brothel returned: %s" % hero.brothels.index(brothel))
                 brothel.highlighted = True
                 brothel_selected = True


Then I launched the game, bought some buildings and assigned girls in a pattern I remembered. The entries in the devlog show that the return value was always the brothel I had clicked on.

I tried this for both c11e60d (the commit with btrl in the for loop) and its parent c4ba128 (the commit with brothel in the for loop) and got identical results.

Am I still misunderstanding the problem?
Title: Re: Code review
Post by: Xela on June 26, 2013, 08:44:08 AM
Am I still misunderstanding the problem?

In any case this is what I believe was wrong with the code

Well, apparently my believe was False... but I am not spending another minute figuring out that issue, I am having trouble focusing on Guard code as it is. If you want to keep digging at what the hell goes wrong there, be my guest :)
Title: Re: Code review
Post by: rudistoned on June 26, 2013, 08:51:42 AM
Not right now, but maybe later. Do you remember the symptoms of the problem you observed?
Title: Re: Code review
Post by: Xela on June 26, 2013, 09:00:05 AM
Do you remember the symptoms of the problem you observed?

There is no call to "remember", it becomes obvious the moment you change iterator name back to brothel and buy more than one brothel.

Try actually setting girls working in the brothels :)
Title: Re: Code review
Post by: rudistoned on June 26, 2013, 09:43:15 AM
I checked out revision c4ba128 (- Updated Traits File)
I bough two extra buildings
I set girls to work in these buildings
I clicked trough all events on next day

Nothing out of the ordinary happened.
Title: Re: Code review
Post by: Xela on June 26, 2013, 09:56:48 AM
I checked out revision c4ba128 (- Updated Traits File)
I bough two extra buildings
I set girls to work in these buildings
I clicked trough all events on next day

Nothing out of the ordinary happened.

After you buy several brothels:

Note that if iterator name in the girls assign screen is 'brothel':

1) When you set girls to work, tooltip displays faulty data.
2) Jobs available for girls in the brothel of your choice is not being updated either (Stripper job should be available to Mei in that brothel).

Note that problem disappears if you iterate with brtl (or anything else on the screen) with rest of the code being exactly the same, job is available, tooltip is functioning correctly.
Title: Re: Code review
Post by: rudistoned on June 26, 2013, 10:00:30 AM
Now I checked out the newest revision of master and changed "brtl" back to "brothel". Something should be wrong because the "brothel" variable of the label is used in the screen here:

Code: [Select]
    if brothel_selected:
        frame background Frame("content/gfx/bg/pergament.jpg",10,10):
            align (0,0.65)
            side "c r":
                viewport id "girlaction_vp":

<...>

                        if brothel.free_rooms() > 0 or chr.location == brothel:

                            for entry in brothel.actions:
                                    if entry == 'Stripper':

If "brothel" is used in the for loop in the screen code before my quoted code is executed, it would change which actual brothel the "brothel" variable in the quoted code refers to. I suppose that is the root of the problems you saw.
Title: Re: Code review
Post by: Xela on June 26, 2013, 10:05:45 AM
If "brothel" is used in the for loop in the screen code before my quoted code is executed, it would change which actual brothel the "brothel" variable in the quoted code refers to. I suppose that is the root of the problems you saw.

Prolly is the case.
Title: Re: Code review
Post by: rudistoned on June 26, 2013, 10:06:13 AM
This kind of bug could appear in any python script. However, it is IMHO more difficult to spot in renpy scripts due to the many levels of indentation. Pytfalls code makes it even more difficult because some lines of code are extremely long, so you have to scroll sideways to see all the code.

To avoid it, we should use more functions and classes, so less code is executed in the global namespace.

UPDATE: Does renpys screen language support functions and/or classes? They might call it custom widgets.
Title: Re: Code review
Post by: Xela on June 26, 2013, 10:19:46 AM
UPDATE: Does renpys screen language support functions and/or classes? They might call it custom widgets.

Supports pretty much anything. The entire screen can even be constructed in pure Python (if you wish to use the same paradigm as you've used in your Python OW port).
Title: Re: Code review
Post by: rudistoned on June 26, 2013, 10:24:27 AM
That's great :-)
Title: Re: Code review
Post by: Xela on June 26, 2013, 10:37:50 AM
That's great :-)

Yeap, having options is always a good thing. I personally prefer the looks of screen language itself, it's a lot less confusing once you get used to it. Running anything python in the screen language is also possible.
Title: Re: Code review
Post by: rudistoned on June 28, 2013, 08:17:59 AM
Quote
+            '''On Rudi's advice, I'll try to code some screen control into pure Python classes
+            This method enables/disables security bar, and sets max value depending on brothel upgrades
+            Index 0 = Show/No show of bar, index 1 = max value of bar
+            '''

 ???
 :D
What I meant to convey with my advise is that we should package GUI code up in classes and functions, so we don't overwrite global variables with variables that look "local", when in fact they are in the global namespace. Putting GUI code in game logic classes (anything else than GUI code) is quite the opposite of what I want  ;)

With that being said, lets look at the complete method:
Code: [Select]
+        def gui_security_bar(self):
+            '''On Rudi's advice, I'll try to code some screen control into pure Python classes
+            This method enables/disables security bar, and sets max value depending on brothel upgrades
+            Index 0 = Show/No show of bar, index 1 = max value of bar
+            '''
+            result = []
+           
+            if len([girl for girl in self.get_girls() if girl.action == 'Guard']) > 0:
+                result.append(True)
+            else: result.append(False)
+           
+            if self.upgrades['mainhall']['2']['active']:
+                result.append(100)
+            elif self.upgrades['mainhall']['1']['active']:
+                result.append(50)
+            else: result.append(20)
+           
+            return result

Basically, this method answers two questions:
1) Is there a guard?
2) What's the maximum value for security at the bar?

So this method actually describes its instance and should therefore be part of the class. I would not consider that GUI code  :)

Here's another, IMHO clearer, way to answer the two questions:
Code: [Select]
        def is_guarded(self):
            '''Returns True if there is at least one guard here.
            '''
            guards = [girl for girl in self.get_girls() if girl.action == 'Guard']
            return bool(guards)

        def get_bar_security_max(self):
            '''Returns the maximum value for bar security (20 - 100).
            '''
            if self.upgrades['mainhall']['2']['active']:
                maxsec = 100
            elif self.upgrades['mainhall']['1']['active']:
                maxsec = 50
            else:
                maxsec = 20
            return maxsec

UPDATE: After reading Xelas post in the General discussion, I realize that "bar" probably refers to a GUI element. Well, my bad. It does not change anything important.
Title: Re: Code review
Post by: Xela on June 28, 2013, 09:50:52 AM
What I meant to convey with my advise is that we should package GUI code up in classes and functions, so we don't overwrite global variables with variables that look "local", when in fact they are in the global namespace. Putting GUI code in game logic classes (anything else than GUI code) is quite the opposite of what I want  ;)

One of the things I like about RenPy is screen language and how it's setup. I figured you were in favor of cleaning screens up, not recoding them in pure Python.
Title: Re: Code review
Post by: rudistoned on June 28, 2013, 10:31:03 AM
I am in favour of cleaning the current screens up and I'm probably also in favour of using renpys screen language.


Is there any way to package screen language code into a separate namespace? Something like a subscreen or custom widget?



What I don't like is code that does something for screens in classes that contain game logic. Rule of thumb: if the class is in any of our current "classes - *.rpy" files, it should not contain GUI code. GUI code should IMHO be in the label that shows the screen, or in separate classes and functions.
Title: Re: Code review
Post by: Xela on June 28, 2013, 10:49:52 AM
I am in favour of cleaning the current screens up and I'm probably also in favour of using renpys screen language.

Is there any way to package screen language code into a separate namespace? Something like a subscreen or custom widget?

One or the other, you can't have both, would be to convenient.

What I don't like is code that does something for screens in classes that contain game logic. Rule of thumb: if the class is in any of our current "classes - *.rpy" files, it should not contain GUI code. GUI code should IMHO be in the label that shows the screen, or in separate classes and functions.

I honestly have no problem with it.
Title: Re: Code review
Post by: rudistoned on June 30, 2013, 02:57:42 AM
Code: [Select]
            # Should not be called (ever)
            else:
                txt += "Brothel is reasonably clean and there are no clients left to serve! "

This is a snippet from ServiceJob.set_task.

If the else condition should only be reached when something went wrong, why does it output ingame text instead of logging an error message?
Title: Re: Code review
Post by: Xela on June 30, 2013, 03:58:05 AM
If the else condition should only be reached when something went wrong, why does it output ingame text instead of logging an error message?

Because it still works, but feel free to remove it completely or turn it into an Error. Things like that are in many places in the code, especially in part that I've programmed close to when the project was started (Not having any previous coding experience of any serious kind).
Title: Re: Code review
Post by: Xela on June 30, 2013, 06:06:49 AM
Damn, that devlog thing is useful, just traced a nasty error using it, we should do more of those :)

2 Errors fixed during the next day (Guard Job and guard event with a normal defeat result). Gonna start with shops soon.
Title: Re: Code review
Post by: rudistoned on June 30, 2013, 10:05:49 AM
Because it still works, but feel free to remove it completely or turn it into an Error.
If the game still works at that point, we should not throw an Exception. However, since your comment states that the else condition should never be reached, reaching it indicates some programming error, so it should log an error message, or at least a warning (devlog.error or devlog.warning). Creating ingame text for a condition that should never be reached seems like wasted effort.

It's no problem of course. I just want to suggest we do it differently in the future  :)
Title: Re: Code review
Post by: Xela on June 30, 2013, 11:42:58 AM
If the game still works at that point, we should not throw an Exception. However, since your comment states that the else condition should never be reached, reaching it indicates some programming error, so it should log an error message, or at least a warning (devlog.error or devlog.warning). Creating ingame text for a condition that should never be reached seems like wasted effort.

It's no problem of course. I just want to suggest we do it differently in the future  :)

Agree with you 110%. Like I've said, when that code was written, I didn't know that [item for item in items] might create a list of items...
Title: Re: Code review
Post by: rudistoned on June 30, 2013, 12:40:38 PM
I think I had been programming Python for one or two years when I first read about list comprehension  :D
Title: Re: Code review
Post by: rudistoned on August 11, 2013, 12:17:52 PM
Hey guys!


Here is the code I used in Pytherworld to load PyTFalls girl ressources, in case you want a new ressource loader that supports PyTFalls filename-based tagging and can read XMP tags. To use these classes, you need to import the tags package from Pytherworld or Image Tagger in your code. You do not need pyexiv2, because the tags package comes with an included XMP reader in pure python. This means that distributing it with renpy should be easy.  You will also have to adapt these classes a little to your applications needs and environment, of course. Just ask if anything gives you trouble :)


Code: [Select]
class PyTFallGirlDataRessourceMap(tags.resload.RessourceMap):
    '''Provides xml ressource files.'''
    def __init__(self, rootdir, picklepath="DEFAULT"):
        tags.resload.RessourceMap.__init__(self, ["data.xml"], rootdir,
                                           picklepath=picklepath)
        # maps pytfall character ids to character name tags
        self.idmap = {}


    def guess_tags(self, relpath):
        '''Returns a list of tags for this file based on relpath.'''
        abspath = os.path.join(self.rootdir, relpath)
        # parse XML
        tree = ET.parse(abspath)
        rootelem = tree.getroot()
        # remember which girls are described here in the tagmap
        nametags = []
        nametags.sort()
        for girlelem in rootelem.iterfind("girl"):
            charnametag = girlelem.attrib["name"]
            nametags.append(charnametag)
            # also rememeber the character folder for each girl
            self.idmap[girlelem.attrib["id"]] = charnametag
        return nametags




class PyTFallImageRessourceMap(tags.resload.ImageRessourceMap):
    '''Provides PyTFall image ressource files.'''
    def __init__(self, patternlist, rootdir, picklepath="DEFAULT"):
        tags.resload.ImageRessourceMap.__init__(self, patternlist, rootdir,
                                                picklepath=picklepath)
        self.girlxml = None
        self.guessmap = {"etiquette": ["learn etiquette"],
                          "dance": ["learn dancing"],
                          "nude.jpg": ["nude"],
                          "rest": ["resting"],
                          "battle_sprite": ["ignore"],
                          "combat": ["ignore"],
                          "date": ["ignore"],
                          "datebeach": ["ignore"],
                          "shop": ["ignore"],
                          "ent": ["ignore"],
                          "Quest": ["ignore"],
                          "quest": ["ignore"],
                          "0quest0": ["ignore"],
                          "0quest1": ["ignore"],
                          "0quest2": ["ignore"],
                          "quest1": ["ignore"],
                          "quest2": ["ignore"],
                          "questsad": ["sad"],
                          "questnormal": ["indifferent"],
                          "questneutral": ["indifferent"],
                          "questhappy": ["happy"],
                          "questangry": ["angry"],
                          "battle": ["ignore"],
                          "portrait.png": ["portrait"],
                          "maid.jpg": ["maid"],
                          "profilesad": ["sad"],
                          "profilehappy": ["happy"],
                          "profileneutral": ["indifferent"],
                          "profileangry": ["angry"],
                          "blowjob": ["+blowjob"],
                          "sex": ["-fuck"],
                          "Sex": ["-fuck"],
                          "anal": ["-anal"],
                          "bdsm": ["-bdsm"],
                          "mast": ["masturbate"],
                          "strip": ["undressing"],
                          "beauty": ["ignore"],
                          "les": ["ignore"],
                          "Les": ["ignore"],
                          "date.jpg": ["ignore"]}
        self.unknown = set()
        # initialize girl data resmap
        ppath = os.path.join(c.PATH.RESDIR, "pytfall", "pytfgirldata.res")
        self.girlxml = PyTFallGirlDataRessourceMap(self.rootdir,
                                                    picklepath=ppath)
        self.girlxml.load()


    def guess_tags(self, relpath):
        '''Returns a list of tags for this file based on its path and XMP tags.


        The filename is converted into a tag by splitting at whitespace and
        treating the first item of the resulting list as the tag.
        The remaining path is split at the path separator and translated into
        tags according to self.guessmap.
        '''
        guessedtags = []
        # split off filename and file extension
        base, fullfn = os.path.split(relpath)
        filename, ext = os.path.splitext(fullfn)
        # split the filename into words and assume the first word is the tag
        firstword = filename.split()[0]
        ##for encoding in ["ascii", "utf-8"]:
            ##try:
                ##firstword = firstword.decode(encoding)
            ##except UnicodeDecodeError:
                ##pass
        ##firstword.encode("utf-8")
        if firstword in self.guessmap:
            guessedtags.extend(self.guessmap[firstword])
        else:
            guessedtags.append(firstword)
            if firstword not in tags.default.known:
                logger.warning("unknown pytfall tag: %s" % firstword)
        # split the path and translate it into tags
        taglist = tags.resload.ImageRessourceMap.guess_tags(self, relpath)
        guessedtags.extend(list(taglist))
        # add tags based on the character
        base = relpath
        charnametag = None
        while base:
            base, dirname = os.path.split(base)
            if dirname in self.girlxml.idmap:
                charnametag = self.girlxml.idmap[dirname]
                guessedtags.append(charnametag)
                break
        if charnametag is None:
            msg = "could not identify charnametag for %s"
            logger.warning(msg % relpath)
        guessedtags.sort()
        # inform if there are untranslated parts
        if c.DEBUG:
            for pathpart in base.split(os.path.sep):
                if (pathpart not in self.guessmap and
                    pathpart not in self.unknown):
                    msg = "No translation for PyTFall path part: %s"
                    logger.info(msg % pathpart)
                    self.unknown.add(pathpart)
        return guessedtags




Here is one way to instantiate the classes above:


Code: [Select]
class PyTFallRessourceNamespace(object):
    '''Makes pytfall ressource loaders accessible via 'c.res.pytfall'.'''
    def __init__(self):
        self.gamedir = os.path.join(c.PATH.PYTFALL, "game")
        # provides images for character screens
        self.girls = None
        # --  initialize  -- #
        rootdir = os.path.join(self.gamedir, "content", "chars")
        # initialize girl image resmap
        picklefile = os.path.join(c.PATH.CACHE, "res_pytfall_girls.res")
        if os.path.exists(picklefile):
            msg = "loaded pytfall girl image metadata from pickled ressource"
            logger.info(msg)
            self.girls = PyTFallImageRessourceMap.from_pickle(picklefile)
        else:
            self.girls = PyTFallImageRessourceMap(["*.jpg", "*.png"], rootdir,
                                                  picklepath=picklefile)
            self.girls.load()
            logger.info("created new pytfall girl image metadata")
            self.girls.to_pickle()
Title: Re: Code review
Post by: h3nta1fun on April 09, 2014, 04:00:11 PM
Code: [Select]


# Other ----------------------
label interact_other:
    # It'll be up to me to write an occupation changer for other at the very least.
    menu:
        "What will it be?"
        "Ask her to change profession":
            menu:
                "Switch to:"
                "Prostitute" if chr.occupation != "Prostitute":
                    if chr.status == "slave" and chr.disposition > -500:
                        g "As you wish Master..."
                        if chr.occupation != "Stipper" and chr.disposition < 200:
                            "She doesn't look too happy about this..."
                            python:
                                chr.joy -= 40
                                chr.disposition -= 50                                chr.occupation = "Prostitute"


Just looking in the pyt - labels - girlinteractions.rpy file and found that "Stipper" that sould have been "Stripper".[size=78%]  [/size]
Title: Re: Code review
Post by: Xela on April 10, 2014, 07:33:15 AM
Code: [Select]


# Other ----------------------
label interact_other:
    # It'll be up to me to write an occupation changer for other at the very least.
    menu:
        "What will it be?"
        "Ask her to change profession":
            menu:
                "Switch to:"
                "Prostitute" if chr.occupation != "Prostitute":
                    if chr.status == "slave" and chr.disposition > -500:
                        g "As you wish Master..."
                        if chr.occupation != "Stipper" and chr.disposition < 200:
                            "She doesn't look too happy about this..."
                            python:
                                chr.joy -= 40
                                chr.disposition -= 50                                chr.occupation = "Prostitute"


Just looking in the pyt - labels - girlinteractions.rpy file and found that "Stipper" that sould have been "Stripper".[size=78%]  [/size]

Tnx!