4rtzel / poe-archnemesis-scanner

Tool for Path of Exile game to automatically scan Archemesis inventory and display related information
Apache License 2.0
64 stars 25 forks source link

Added a "Shopping List" mode feature #37

Closed williammetcalf closed 2 years ago

williammetcalf commented 2 years ago
4rtzel commented 2 years ago

That's a lot of work! Einhar approves

I'll try to review it as soon as possible. Trying to catch up with real life stuff after providing support for this tool for last 5 days 8 hours a day.

meepen commented 2 years ago

Haha, both of us did th same two small things with settings and rename treant :)

JustLKS commented 2 years ago

Omg thats amazing! where can I doiwnload it williammetcalf?

4rtzel commented 2 years ago

Testing the changes. Some feedback so far:

Running scan without shopping list causes an exception:

...\src\RecipeShopper.py", line 26, in get_missing_items
    complex_missing_items = list(item for item in missing_items if len(self._item_map.get_subtree_for(item).components) > 0)
RuntimeError: generator raised StopIteration

The button in the settings to set shopping list is misnamed (Set scan/hide hotkey).

Trying to clear the shopping list raises an exception here (line 459):

    def _update_shopping_list(self) -> None:
        # TODO: validate it
        shopping_list = list(map(lambda x: x.strip(), self._shopping_list_entry.get().split(",")))
        for item in shopping_list:
            if item not in self._items_map.items():
                print("Unknown Recipe:", item)
>>              raise ValueError
williammetcalf commented 2 years ago

@4rtzel alright I added some error handling. TBH I haven't written python in like 7 years I think, and I'm definitely no expert with using TKinter, so idk if I did this especially well. I'm definitely open to suggestions on how to handle errors for this type of thing.

I'm also sure it would be a bit confusing to a new user to understand exactly what to put in for the recipes, since its case/hypen sensitive

python3 8_VWonfWPyMG

williammetcalf commented 2 years ago

Omg thats amazing! where can I doiwnload it williammetcalf?

For the time being you would need to clone my fork of this repository and setup/run the python code directly, I don't feel comfortable personally packaging and distributing someone else's hard work. One this PR gets merged the project will need to be re-released

4rtzel commented 2 years ago

@4rtzel alright I added some error handling. TBH I haven't written python in like 7 years I think, and I'm definitely no expert with using TKinter, so idk if I did this especially well. I'm definitely open to suggestions on how to handle errors for this type of thing.

I'm also sure it would be a bit confusing to a new user to understand exactly what to put in for the recipes, since its case/hypen sensitive

python3 8_VWonfWPyMG

I'm not a Python developer either so I can't really tell how good/bad it is :). For me it seems "good enough". Especially giving that this tool is a temporary solution for the UI problems in PoE.

As long as the main feature of your changes is functional, I'll merge them. You can leave the polishing to me.

williammetcalf commented 2 years ago

I've been using it myself for a few days and it does seem to work. There's a few more additions I'd like to make but cant be bothered until I see what changes GGG makes.

Ideally I'd like it to show on-screen the items you are missing to make it easier to keep an eye out for them, and it would be nice if it filtered the recipes shown even smarter to not lead you to making duplicates of items you've already made. But I think this is a good baseline for the concept

4rtzel commented 2 years ago

I agree with you about waiting for the solution that GGG will come up with, before doing any more changes. My current plan is to merge your work and release v0.2.0 with keyboard changes.

After that, I'll merge https://github.com/4rtzel/poe-archnemesis-scanner/pull/35 for v0.2.1 release (@meepen changes modify the algorithm quite significantly, so it would be nice if people had a version to fallback to in case of any issues).

4rtzel commented 2 years ago

Played a little bit with the changes. Besides the issues, that I mentioned above (already fixed), everything seems to be working fine.

The code also looks much better now with the split. There are a few stylistic issues but nothing critical. Merging.