-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
add enchantment abbreviations as slot text #1056
base: master
Are you sure you want to change the base?
add enchantment abbreviations as slot text #1056
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like there should be a better way of reporting duplicates but I can't think right now.
for (Map.Entry<String, String> entry : ID_ABBREVIATION_MAP.entrySet()) { | ||
final String put = otherDirection.put(entry.getValue(), entry.getKey()); | ||
if (put != null) { | ||
throw new IllegalArgumentException("Duplicate key [%s, %s] for value %s".formatted(put, entry.getKey(), entry.getValue())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to throw here, logging an error is sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explicitly decided to throw an exception, since you may not notice a simple log. And since the exception will only be throw in a debug/development environment I assumed it should be fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this would be more sufficient as a unit test. This way we also don't ship test code.
will this add to weapons/armors? e.g. for terminator,used to tell if it's SE or duplex |
return Text.empty(); | ||
} | ||
|
||
final int color; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace this with a switch
No description provided.