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

Check coat of paint's modifiers after exiting avatar path #1494

Merged
merged 11 commits into from Feb 7, 2023

Conversation

libraryaddict
Copy link
Contributor

@libraryaddict libraryaddict commented Feb 3, 2023

Steps to reproduce

With coat of paint in inventory or equipped, break the prism to exit an avatar path.

As the player's class has changed, coat of paint has its modifiers changed as well.

What happens

Mafia does not recognize coat of paint has changed and the equipment modifiers are now different.

The description was not visited as it should have been by mafia.
Manually checking the description will correct the modifiers.

What we expected instead

Mafia to check coat of paint and update the equipment modifiers without player intervention required.

@codecov
Copy link

codecov bot commented Feb 3, 2023

Codecov Report

Merging #1494 (bee7acd) into main (e4b62b2) will increase coverage by 0.03%.
The diff coverage is 80.00%.

❗ Current head bee7acd differs from pull request most recent head 32c25ae. Consider uploading reports for the commit 32c25ae to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1494      +/-   ##
============================================
+ Coverage     34.73%   34.76%   +0.03%     
- Complexity    17378    17384       +6     
============================================
  Files          1062     1062              
  Lines        163275   163274       -1     
  Branches      34927    34926       -1     
============================================
+ Hits          56712    56766      +54     
+ Misses        97030    96975      -55     
  Partials       9533     9533              
Impacted Files Coverage Δ
...sourceforge/kolmafia/session/InventoryManager.java 34.18% <75.00%> (+0.38%) ⬆️
src/net/sourceforge/kolmafia/KoLmafia.java 36.78% <100.00%> (+4.42%) ⬆️
...ourceforge/kolmafia/request/ClanLoungeRequest.java 34.68% <0.00%> (+0.28%) ⬆️
src/net/sourceforge/kolmafia/RequestThread.java 38.18% <0.00%> (+1.21%) ⬆️
.../sourceforge/kolmafia/swingui/menu/ScriptMenu.java 69.23% <0.00%> (+1.53%) ⬆️
.../sourceforge/kolmafia/preferences/Preferences.java 86.42% <0.00%> (+1.58%) ⬆️

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 e4b62b2...32c25ae. Read the comment docs.

@libraryaddict libraryaddict marked this pull request as ready for review February 3, 2023 23:19
@libraryaddict libraryaddict requested a review from a team as a code owner February 3, 2023 23:19
@libraryaddict libraryaddict marked this pull request as draft February 3, 2023 23:20
@libraryaddict
Copy link
Contributor Author

Oh dear, just realized that the test that should've been failing, was succeeding.

@libraryaddict libraryaddict marked this pull request as ready for review February 6, 2023 02:32
Comment on lines 1739 to 1741
if (!KoLCharacter.hasEquipped(COAT_OF_PAINT, EquipmentManager.SHIRT)
&& !KoLConstants.inventory.contains(COAT_OF_PAINT)) {
&& COAT_OF_PAINT.getCount(KoLConstants.inventory) == 0
&& COAT_OF_PAINT.getCount(KoLConstants.closet) == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

InventoryManager.getAccessibleCount instead (or one of the related methods)? Or do we really want exactly equipped, inventory + closet?

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 7, 2023 08:53
@midgleyc midgleyc merged commit 6608960 into kolmafia:main Feb 7, 2023
@libraryaddict libraryaddict deleted the avatar_shirt branch February 11, 2023 13:09
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