Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly tracking inventory changes to the cached inventory #4787

Merged
merged 1 commit into from
Aug 27, 2016
Merged

Correctly tracking inventory changes to the cached inventory #4787

merged 1 commit into from
Aug 27, 2016

Conversation

duttonw
Copy link
Contributor

@duttonw duttonw commented Aug 27, 2016

based upon @BriceSD work with #3860 cleaned up to be able to be merged since the original branch/pull request was very messy.

Description:

With the new cached inventory becoming a core component, we can’t afford to not correctly update it after every change.

I’ve made a class (RecycleWorker) which is more or less an api wrapper.
At first I put in it the responsibility to update the inventory. While not being bad, i feel like it’s easy to misunderstand what item.remove(amount) do. Namely removing the amount from the cached inventory without making a recycling request to the server.

What I’m proposing now to recycle an item is to use item.recycle(amount). Inside this method we’re doing a call to the api and then, depending on the success of the recycle request, we’re updating the cached inventory. By doing this we’ll never change the quantity of an item from outside the class. Thus we could allow only access to Item’s attribute and deny any change from the outside. This would make the cached inventory trustable.

However this could result in a very coupled code.
Having a class that adds a level of abstraction (RecycleWorker here) seems good enough though. The item know nothing about the recycling process. Not even how is defined a succeeding call.

def recycle(self, amount_to_recycle):
    """
    Recycle (discard) the specified amount of item from the item inventory.
    It is making a call to the server to request a recycling as well as updating the cached inventory.
    :param amount_to_recycle: The amount to recycle.
    :type amount_to_recycle: int
    :return: Returns wether or not the task went well
    :rtype: worker_result.WorkerResult
    """
    if self.count < amount_to_recycle:
        raise Exception('Tried to remove more {} than you have'.format(self.name))

    item_recycler = ItemRecycler(_inventory.bot, self, amount_to_recycle)
    item_recycler_work_result = item_recycler.work()

    if item_recycler.is_recycling_success():
        self.remove(amount_to_recycle)
    return item_recycler_work_result

I haven’t work much with the the pokemon inventory, or the pokemon catcher either. Not sure if throwing a ball during an encounter from the inventory is a good idea.
Any thoughts on the subject would be highly appreciate.

@amal, @douglascamata, @TheSavior, @aeckert, @BriceSD, @solderzzc

@solderzzc
Copy link
Contributor

solderzzc commented Aug 27, 2016

@duttonw could you add your instruction into the document ?

@solderzzc solderzzc merged commit 87740d5 into PokemonGoF:dev Aug 27, 2016
@duttonw duttonw deleted the feature/update-recycle-items-doc-2 branch August 27, 2016 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants