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

Move limitmode to enum #1084

Merged
merged 7 commits into from Sep 17, 2022
Merged

Move limitmode to enum #1084

merged 7 commits into from Sep 17, 2022

Conversation

gausie
Copy link
Contributor

@gausie gausie commented Sep 15, 2022

This PR:

  • Turns the Limitmode class into LimitMode enum
  • Flattens out the dependency tree - instead of asking LimitMode's statics questions that then asks KoLCharacter for the current limit mode and assesses that, LimitModes have instance methods and we query against KoLCharacter.getLimitMode() directly when appropriate
  • Adds tests for all those functions in LimitMode

@gausie
Copy link
Contributor Author

gausie commented Sep 15, 2022

Why has this seemingly changed the HTTP method that api.php requests are made with? Shouldn't they be GET anyway?? What is going on!

EDIT: Ahha, it would be good for request assertions to show the full issue with the request - the URL was also wrong!

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Merging #1084 (a672c01) into main (8f37621) will increase coverage by 0.02%.
The diff coverage is 26.51%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1084      +/-   ##
============================================
+ Coverage     29.63%   29.65%   +0.02%     
- Complexity    14308    14350      +42     
============================================
  Files          1036     1036              
  Lines        160993   160939      -54     
  Branches      35165    35122      -43     
============================================
+ Hits          47703    47726      +23     
+ Misses       104728   104651      -77     
  Partials       8562     8562              
Impacted Files Coverage Δ
src/net/sourceforge/kolmafia/KoLmafia.java 24.33% <0.00%> (ø)
src/net/sourceforge/kolmafia/RequestEditorKit.java 23.64% <0.00%> (ø)
.../sourceforge/kolmafia/moods/MPRestoreItemList.java 20.52% <0.00%> (ø)
...et/sourceforge/kolmafia/moods/ManaBurnManager.java 1.48% <0.00%> (ø)
...rc/net/sourceforge/kolmafia/moods/MoodManager.java 59.45% <0.00%> (ø)
...et/sourceforge/kolmafia/moods/RecoveryManager.java 30.81% <0.00%> (ø)
...ceforge/kolmafia/persistence/RestoresDatabase.java 32.30% <0.00%> (+0.83%) ⬆️
...et/sourceforge/kolmafia/request/ArmoryRequest.java 30.00% <0.00%> (ø)
...ceforge/kolmafia/request/BatFabricatorRequest.java 39.13% <0.00%> (ø)
...t/sourceforge/kolmafia/request/BrogurtRequest.java 38.09% <0.00%> (ø)
... and 49 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 8f37621...a672c01. Read the comment docs.

@Veracity0
Copy link
Contributor

I love this.

I will be adding a few more Limitmodes - ASTRAL and MOLE, both of which limit which zones you can adventure in. Maybe BIRD, since that replaces your combat skills with bird skills. MOLE and BIRD are mutually exclusive. The question is whether ASTRAL and BIRD are.

In any case, this is really good work.

@gausie
Copy link
Contributor Author

gausie commented Sep 15, 2022

That sounds good! Though from this code it seems that limit modes are defined kol-side and indicated via the API. We'll need to keep that in mind

@Veracity0
Copy link
Contributor

Well, poo. I may need to rethink that, then. I need the equivalent, for limiting where you can adventure when you are Half-Astral or Shape of Mole.

@gausie
Copy link
Contributor Author

gausie commented Sep 15, 2022

Well, poo. I may need to rethink that, then. I need the equivalent, for limiting where you can adventure when you are Half-Astral or Shape of Mole.

I think there's a way we can do it, by looking at certain prefs/effects when we get a "0" back from that API key...

@gausie gausie marked this pull request as ready for review September 16, 2022 10:22
@gausie gausie requested a review from a team as a code owner September 16, 2022 10:22
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.

in the area, might be worth a comment that limit mode "edunder" is Ed in the Underworld, because it's fairly confusing if you don't know that.

Spelunky and Batfellow are self-explanatory.

@jaadams5 jaadams5 merged commit acdde48 into main Sep 17, 2022
@jaadams5 jaadams5 deleted the limitmode-enum branch September 17, 2022 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants