Added time input calculator#2189
Open
NamelessJu wants to merge 5 commits into
Open
Conversation
Author
|
My bad, I only now found out about the checkstyle gradle tasks. Should be fine now (hopefully) |
Author
|
And we try again (sorry), I ran a full successful build on my end now (don't know why I didn't do this earlier) |
Collaborator
kevinthegreat1
left a comment
kevinthegreat1
left a comment
There was a problem hiding this comment.
I don't particularly like the extra config, but I'll see if anyone else has anything to say before approving. Also some unit tests would be nice.
Comment on lines
+53
to
+57
| if (numberString.startsWith("-")) { | ||
| seconds = -1; | ||
| error = Component.translatable("skyblocker.config.uiAndVisuals.timeInputCalculator.invalidNumberError", numberString).withStyle(ChatFormatting.RED); | ||
| return; | ||
| } |
Collaborator
There was a problem hiding this comment.
Nice negative time (your regex doesn't allow this btw).
Comment on lines
+39
to
+40
| if (message.equals(lastInput)) return; | ||
| lastInput = message; |
Collaborator
There was a problem hiding this comment.
Do you need to check time type as well instead of just last input?
LifeIsAParadox
added
merge conflicts
Collaborator
It is a bit redundant but I don't really mind |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently the AH duration input uses the regular input calculator, which shows errors on valid time inputs such as "2d".
This time calculator fixes this by displaying the entered amount of time instead:
(Correctly uses the time unit selected in the AH menu by default, so either hours or minutes)
It also allows for inputting multiple time units along with comma values:
Also converts whatever is entered into a cleaner format:
(I did a quick test and the auction house actually allows and correctly uses values like this, even though it only shows the highest unit entered in the auction creation screen)
+ I added config values for this new calculator:
I also added a German translation for this, but I am not sure how exactly how you handle translations so I made it a separate commit to disconnect it from the core feature implementation