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

set ownsFloristFriar to true if it is owned #1856

Merged
merged 4 commits into from
Jul 27, 2023
Merged

set ownsFloristFriar to true if it is owned #1856

merged 4 commits into from
Jul 27, 2023

Conversation

Rinn
Copy link
Contributor

@Rinn Rinn commented Jul 27, 2023

This will allow scripts like av-snapshot or greenbox to report the florist friar as owned even if florist_available() returns false, such as a post community service run where the forest is inaccessible.

Also always set it as available if you can get into the choice instead of only on selecting choice 4

@Rinn Rinn requested a review from a team as a code owner July 27, 2023 17:44
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #1856 (509e904) into main (3877c6f) will increase coverage by 0.00%.
The diff coverage is 25.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1856   +/-   ##
=========================================
  Coverage     36.31%   36.31%           
- Complexity    18746    18747    +1     
=========================================
  Files          1081     1081           
  Lines        166310   166312    +2     
  Branches      35385    35386    +1     
=========================================
+ Hits          60390    60402   +12     
+ Misses        96056    96042   -14     
- Partials       9864     9868    +4     
Files Changed Coverage Δ
...t/sourceforge/kolmafia/request/FloristRequest.java 43.86% <25.00%> (+0.05%) ⬆️

... and 5 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@s-k-z
Copy link
Contributor

s-k-z commented Jul 27, 2023

Why not trust the property when using the florist commands?

@Veracity0
Copy link
Contributor

Veracity0 commented Jul 27, 2023

Legacy of Loathing offers a replica Order of the Green Thumb Order Form which grants in-run access to the Florist Friar.

@Rinn
Copy link
Contributor Author

Rinn commented Jul 27, 2023

arg fricking lol

@Rinn
Copy link
Contributor Author

Rinn commented Jul 27, 2023

Why not trust the property when using the florist commands?

you can't get to it if the forest is unavailable, or in standard

@s-k-z
Copy link
Contributor

s-k-z commented Jul 27, 2023

Why not trust the property when using the florist commands?

you can't get to it if the forest is unavailable, or in standard

Isn't that fine?

@Rinn
Copy link
Contributor Author

Rinn commented Jul 27, 2023

Why not trust the property when using the florist commands?

you can't get to it if the forest is unavailable, or in standard

Isn't that fine?

I don't understand? This preference won't ever be set to false in code once it's set to true. It's to denote if you own the iotm, not if the friars is currently available.

@s-k-z
Copy link
Contributor

s-k-z commented Jul 27, 2023

Why not trust the property when using the florist commands?

you can't get to it if the forest is unavailable, or in standard

Isn't that fine?

I don't understand? This preference won't ever be set to false in code once it's set to true. It's to denote if you own the iotm, not if the friars is currently available.

Last I saw, florist_available() only returns true when you launch KoLmafia with it accessible. If this property is just cosmetic rather than functional, it won't behave like the others, where toggling it on would enable attempts to use the florist friars at runtime.

@Rinn
Copy link
Contributor Author

Rinn commented Jul 27, 2023

Why not trust the property when using the florist commands?

you can't get to it if the forest is unavailable, or in standard

Isn't that fine?

I don't understand? This preference won't ever be set to false in code once it's set to true. It's to denote if you own the iotm, not if the friars is currently available.

Last I saw, florist_available() only returns true when you launch KoLmafia with it accessible. If this property is just cosmetic rather than functional, it won't behave like the others, where toggling it on would enable attempts to use the florist friars at runtime.

I intentionally didn't name this property to contain "Available", or "Always", or "Accessable" for this very reason.

@s-k-z
Copy link
Contributor

s-k-z commented Jul 27, 2023

Why not trust the property when using the florist commands?

you can't get to it if the forest is unavailable, or in standard

Isn't that fine?

I don't understand? This preference won't ever be set to false in code once it's set to true. It's to denote if you own the iotm, not if the friars is currently available.

Last I saw, florist_available() only returns true when you launch KoLmafia with it accessible. If this property is just cosmetic rather than functional, it won't behave like the others, where toggling it on would enable attempts to use the florist friars at runtime.

I intentionally didn't name this property to contain "Available", or "Always", or "Accessable" for this very reason.

Here is an example:

public static String accessible() {
if (!Preferences.getBoolean("ownsSpeakeasy")) {
return "You don't own a speakeasy";
}
return null;
}

@Rinn
Copy link
Contributor Author

Rinn commented Jul 27, 2023

honestly i don't want to deal with this i don't have time

@Rinn Rinn closed this Jul 27, 2023
@Rinn Rinn deleted the owns-florist-friar branch July 27, 2023 19:44
@gausie
Copy link
Contributor

gausie commented Jul 27, 2023

I intentionally didn't name this property to contain "Available", or "Always", or "Accessable" for this very reason.

Here is an example:

public static String accessible() {
if (!Preferences.getBoolean("ownsSpeakeasy")) {
return "You don't own a speakeasy";
}
return null;
}

That is a bad example. ownsSpeakeasy is one requirement to it being accessible. Others need to be added and will be in the future when the item is no longer in standard or is otherwise routinely unavailable.

@Rinn Rinn restored the owns-florist-friar branch July 27, 2023 20:28
@Rinn Rinn reopened this Jul 27, 2023
@Rinn Rinn merged commit 23d4dc2 into main Jul 27, 2023
13 checks passed
@jaadams5
Copy link
Contributor

coolitems in the API has nothing to say about this?

@Rinn Rinn deleted the owns-florist-friar branch July 27, 2023 20:44
@gausie
Copy link
Contributor

gausie commented Jul 27, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants