General Discussion

[picobyte]

In the very last commit in my branch, a patch to cleanup the auto-buy function. There were a few potential ctds. Is this ok or does the autobuy require a complete rewrite?
In case of an amount==1 auto-buy, the chance to select items from the lower sections of this function is slim. This due to the top-down traversal and a return on the first appropriate item.

[Xela]

In the very last commit in my branch, a patch to cleanup the auto-buy function. There were a few potential ctds. Is this ok or does the autobuy require a complete rewrite?
In case of an amount==1 auto-buy, the chance to select items from the lower sections of this function is slim. This due to the top-down traversal and a return on the first appropriate item.

Oki, so if you;re willing to mess with deepest anals of pytfalls code: 

This method should be optimized for speed instead of code readability, the first version of this I wrote was executed in 12 secs buying 10 items for 1000 chars. Last version did .6 - .8 secs if i recall correctly. We do randomize when chars go shopping and it is not bloody likely that there will ever actually be 1000 free chars in the game world buying sh!t at the same time but it can theoretically happen (also even chars that player has not met yet, still go shopping as long as they are in "city" location).

Code: [Select]

+            pool = set(item for item in auto_buy_items if not(item.badtraits.intersection(self.traits)) and (item.price <= self.gold))
+
             if self.status == "slave": # for slaves we exclude all weapons, spells and armor immediately
-                pool = list(item for item in auto_buy_items if not(item.badtraits.intersection(self.traits)) and (item.price <= self.gold) and not(item.slot in ("weapon", "smallweapon")) and not(item.type in ("armor", "scroll")))
-            else:
-                pool = list(item for item in auto_buy_items if not(item.badtraits.intersection(self.traits)) and (item.price <= self.gold))
+                pool = pool.difference(item for item in pool if item.slot in ("weapon", "smallweapon") or item.type in ("armor", "scroll"))             
I am pretty sure that this is a step in the wrong direction, also the general rule is to use sets if you do a lot of membership testing and lists when you got to iterate over them. You can also do choice from lists at lighting speeds. So you end up with:

Code: [Select]

selected_item = random.choice(tuple(newpool))
creating tuples all over the place to get functionality we had by default with lists.

Code: [Select]

+                newpool = set(item for item in pool if item.type == "restore")
Iterating over items in sets a lot, which is also a bit slower.

And I do not believe that we're actually testing if an item is in the pool anywhere...

Other than that, there are issues with the code itself, cause while it was working in the past, data structures were changed and it is a complete fail today As you pointed out, it can ctd because of the money filter which I think was added later. It would not ctd on shuffle as you can shuffle a list if it's empty.

Code: [Select]

            for i in pool:
                # This will make sure that girl will never buy more than 5 of any item!
                if i.id in self.inventory
This code is meaningless and broken everywhere it is used in the game, we no longer use if strings in inventory, we now use item objects. So it can prolly be fixed with:

Code: [Select]

if i in self.inventory:
I am also not sure if this does anything meaningful either, as it did in the past:

Code: [Select]

newpool = list(item for item in pool if item.goodtraits.intersection(self.traits))
(I've tested it and it does work, guess it was updated at some point).

Otherwise, a lot can be improved in the method, in terms of logic, design and speed but the idea was:

Figure out best logic for items buying/equipping.
Autobuy/equip methods. They do work reasonably well as it is, might require a go over just in case, both content and code.

to do exactly what you did, trying to fish out new problems. Maybe also to throw out some of the old code and add new but I am not even sure what that means myself, we've added base traits, different job handling, skills and etc. Problem is that it may be too early to actually write code for that in auto_buy func, at least until those concepts are finalized and their code and structure is rock solid. I figure if what we have now is optimized and tested, it should be good enough for Beta release.

[Xela]

PS:

We do only care about code optimization inside of stuff that we execute a lot of times during next day and especially inside of tight loops (I once put a complex calculation that only had to be done once inside of a for loop in strippers job, it took 50 secs to finish under testing conditions) Moving it outside improved performance like 2000 times

In VN/Interface mode stuff like this doesn't matter at all. It also doesn't matter that much for funcs that we call on the next day once (although I try to keep it in the back of my head when writing them).

[DarkTl]

Problem is that it may be too early to actually write code for that in auto_buy func, at least until those concepts are finalized

No, it's not that bad. Unlike autoequipment, which may have differences from character to character but still should be useful for the current job, autobuying is not supposed to be perfectly logical and predictable.

During shopping people, especially females, often make weird decisions even irl. While character with low hp will try to buy something that restores health, under normal conditions there are no strict rules. Only various dices based on mood, traits and some items fields. It should be totally possible for chars to buy something totally useless - maybe even only to sell it a few turns later

[Xela]

No, it's not that bad. Unlike autoequipment, which may have differences from character to character but still should be useful for the current job, autobuying is not supposed to be perfectly logical and predictable.

During shopping people, especially females, often make weird decisions even irl. While character with low hp will try to buy something that restores health, under normal conditions there are no strict rules. Only various dices based on mood, traits and some items fields. It should be totally possible for chars to buy something totally useless - maybe even only to sell it a few turns later

It's one of those things I was talking about earlier. Background logic that we cannot properly relay to player is meaningless. It'll look random and confusing or we'd have to add more info to reports. We got to be careful about stuff like this as it breeds useless code that adds almost nothing to gameplay and is often difficult to maintain. It's nice when stuff happens "behind the curtains" but results of these things must have noticeable effects on gameworld/play and even better, be relayed to player somehow.

[DarkTl]

Reports already contain info about bought items. If both autobuying and autoequipping systems will be perfectly logical, there will be nothing for player to do in terms of items management.

I've encountered a problem. Sex scene can only exist inside GMs, and doesn't work with direct jump to it. But sometimes I need to activate it outside of GMs. I dunno how.
While it's possible to rewrite it to make sure it works everywhere, it means full rewriting. Perhaps it's possible to forcibly initiate GMs and then run the scene?

[Xela]

I've encountered a problem. Sex scene can only exist inside GMs, and doesn't work with direct jump to it. But sometimes I need to activate it outside of GMs. I dunno how.
While it's possible to rewrite it to make sure it works everywhere, it means full rewriting. Perhaps it's possible to forcibly initiate GMs and then run the scene?

What label does it start on?

[DarkTl]

There are many labels depending on how it was started. The safest one is interactions_sex_scene_select_place, it doesn't need additional variables (if I'm not mistaken). You just need to make char=needed character.

The problem is, it heavily relies on gm.set_img(...) which does nothing at all outside GMs, so while logic works, pictures don't change.

[Xela]

There are many labels depending on how it was started. The safest one is interactions_sex_scene_select_place, it doesn't need additional variables (if I'm not mistaken). You just need to make char=needed character.

The problem is, it heavily relies on gm.set_img(...) which does nothing at all outside GMs, so while logic works, pictures don't change.

Have you tried using this function:

Code: [Select]

    def interactions_run_gm_anywhere(char, place, background):
        """           
        Runs (or doesn't) gm or interactions with the char based on her status; place is where we jump after gm is over
        """
        if chars[char].status == "slave" or not(chars[char].is_available):
            narrator("Nobody's here...")
            renpy.jump(place)
        elif chars[char] in hero.chars:
            gm.start("girl_interactions", chars[char], chars[char].get_vnsprite(), place, background)
        else:
            gm.start("girl_meets", chars[char], chars[char].get_vnsprite(), place, background)
It looks like something that could work?

[Xela]

No, it will still just to the main interactions menu

[DarkTl]

It's my function which helps to write less lines sometimes, it cannot do fancy stuff 
It will initiate normal GMs, while I need a certain part of GMs, ie sex scene.

[Xela]

Try it now, I've extended the functionality a bit but so many people worked on the system that I can't tell for sure if it's going to work reliably. You may be required to add something like:

Code: [Select]

if gm.mode == "custom":
    $ gm.exit()
else:
    # Whatever happened before.
to exit point from the intercourse interactions.

Also you may be required to set up sayers. I am not 100% sure. The idea is to run the function and show background/setup sayers and/or flags so you'd have more control. I can do better (more automation) if you come up with a testing case (if you feel that it's required) but whatever is needed for basic gm functionality should work.

[DarkTl]

Ok, either one of us or we both don't understand something 

I added testing case. You should include a character into MC team (if there are two, one will be picked randomly) and go to city_beach_left. There will be button with palms. It should (after a few lines from another event that was there before testing case) start GMs using label interactions_sex_scene_begins. Without any other GMs functionality, such as greetings or GM menu. It should, but it doesn't atm because I don't understand the logic behind you push.

Moreover, even simple $ interactions_run_gm_anywhere(member, exit="city_beach_left", background="beach_rest", custom=True) gives a weird error

  File "game/library/screens/locations/beach_left.rpy", line 190, in script
    $ interactions_run_gm_anywhere(member, exit="city_beach_left", background="beach_rest", custom="interactions_sex_scene_begins")
  File "game/library/screens/locations/beach_left.rpy", line 190, in <module>
    $ interactions_run_gm_anywhere(member, exit="city_beach_left", background="beach_rest", custom="interactions_sex_scene_begins")
  File "game/library/interactions/function - interactions (GM).rpy", line 294, in interactions_run_gm_anywhere
    gm.start("custom", chars[char], chars[char].get_vnsprite(), exit, background)
KeyError: <store.rChar object at 0x0E48B350>

If it requires major rewriting of GMs, it can wait until beta, when you wanted to rewrite GMs anyway.

[Xela]

If it requires major rewriting of GMs, it can wait until beta, when you wanted to rewrite GMs anyway.

It does not require anything of a sort, the problem is that I don't know interactions script beyond the GM class itself cause it was written by you and CW. Like for example, you set a whole bunch of required variables in a label where you choose a location for interaction... so I need to trace all the global vars required and make sure they are setup before we jump to the interactions label with positional choices. Give me 15 more mins.

[Xela]

Oki, try it now.

Also, as you write core, try to separate your thoughts with empty lines, it's really hard to read when it's just one endless wall of script/lines.