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).
+ 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:
selected_item = random.choice(tuple(newpool))
creating tuples all over the place to get functionality we had by default with lists.
+ 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
![Sad :(](https://www.pinkpetal.org/Smileys/DarkB/sad.gif)
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.
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:
if i in self.inventory:
I am also not sure if this does anything meaningful either, as it did in the past: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.