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

Fix rock garden tracking #1523

Merged
merged 8 commits into from
Feb 13, 2023
Merged

Conversation

libraryaddict
Copy link
Contributor

What we do

The player harvests an item from the rock garden, then uses get_campground() to check what their garden status is.

After that, they use their packet of tall grass seeds, then call get_campground() again for some reason.

What happens

Their first get_campground() will claim that there was no harvest done, because mafia does not parse campground on rgarden[123]. So whetstone will report 1

The second get_campground() will claim that rock garden and the harvests are still available.

What we expected

The first check will properly see whetstone as 0 in get_campground

The second check will remove garden seeds and harvests.

Disclaimer

So I realized after I made the test that its not to my knowledge possible to have a garden, then get rid of said garden to have no garden. So the test may not be valid.

That said, it seems obvious that the crop clearing should be performed just before we update what crops are available.

@codecov
Copy link

codecov bot commented Feb 10, 2023

Codecov Report

Merging #1523 (f1e7969) into main (6d1a9ea) will increase coverage by 0.00%.
The diff coverage is 57.14%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1523   +/-   ##
=========================================
  Coverage     34.88%   34.88%           
- Complexity    17543    17551    +8     
=========================================
  Files          1065     1065           
  Lines        163434   163430    -4     
  Branches      34936    34938    +2     
=========================================
+ Hits          57010    57019    +9     
+ Misses        96859    96854    -5     
+ Partials       9565     9557    -8     
Impacted Files Coverage Δ
...t/sourceforge/kolmafia/request/UseItemRequest.java 14.89% <ø> (+<0.01%) ⬆️
...ourceforge/kolmafia/request/CampgroundRequest.java 45.61% <57.14%> (+1.29%) ⬆️
...net/sourceforge/kolmafia/textui/DataFileCache.java 37.73% <0.00%> (-5.67%) ⬇️
...forge/kolmafia/textui/langserver/FilesMonitor.java 79.48% <0.00%> (-1.29%) ⬇️
...lmafia/swingui/widget/ListCellRendererFactory.java 29.10% <0.00%> (-0.47%) ⬇️
...net/sourceforge/kolmafia/swingui/GenericFrame.java 23.91% <0.00%> (-0.28%) ⬇️
src/net/sourceforge/kolmafia/KoLCharacter.java 64.15% <0.00%> (-0.13%) ⬇️
...et/sourceforge/kolmafia/session/ChoiceControl.java 9.18% <0.00%> (+0.04%) ⬆️
src/net/sourceforge/kolmafia/RequestLogger.java 30.80% <0.00%> (+0.41%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d1a9ea...f1e7969. Read the comment docs.

@libraryaddict libraryaddict marked this pull request as ready for review February 10, 2023 18:12
@libraryaddict libraryaddict requested a review from a team as a code owner February 10, 2023 18:12
Copy link
Member

@midgleyc midgleyc left a comment

Choose a reason for hiding this comment

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

Thanks!

@midgleyc midgleyc enabled auto-merge (squash) February 13, 2023 10:11
@midgleyc midgleyc merged commit de89c58 into kolmafia:main Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants