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

feat: add backpack with find method #1657

Merged
merged 2 commits into from
May 16, 2022
Merged

Conversation

razvan-pro
Copy link
Collaborator

@razvan-pro razvan-pro commented May 13, 2022

This change is Reviewable

@razvan-pro razvan-pro self-assigned this May 13, 2022
Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @razvan-pro)


aimmo-game-worker/simulation/avatar_state.py line 19 at r1 (raw file):

        )
    else:
        avatar_state_dict["backpack"] = Backpack()

So in the end we're giving creating an Backpack object even if the worksheet doesn't have it?


aimmo-game-worker/simulation/backpack.py line 15 at r1 (raw file):

    def find(self, artefact_type: str) -> int:
        """
        Finds an artefact of type `artefact_type` and returns its index or -1 if there's no artefact of that type.

We should specify that it finds the first index of the artefact type

Copy link
Collaborator Author

@razvan-pro razvan-pro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on @faucomte97 and @razvan-pro)


aimmo-game-worker/simulation/avatar_state.py line 19 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

So in the end we're giving creating an Backpack object even if the worksheet doesn't have it?

Ah, true - fixed and added test, thanks 🙂


aimmo-game-worker/simulation/backpack.py line 15 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

We should specify that it finds the first index of the artefact type

Done.

Copy link
Member

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @razvan-pro)

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

❗ No coverage uploaded for pull request base (worksheet4@1e59903). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             worksheet4    #1657   +/-   ##
=============================================
  Coverage              ?   67.82%           
=============================================
  Files                 ?      165           
  Lines                 ?     3360           
  Branches              ?      283           
=============================================
  Hits                  ?     2279           
  Misses                ?     1054           
  Partials              ?       27           

@razvan-pro razvan-pro merged commit d79a354 into worksheet4 May 16, 2022
@razvan-pro razvan-pro deleted the worksheet4-backpack branch May 16, 2022 15:13
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.

None yet

2 participants