refactor: make EquipmentManager slots an enum #1510
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1510 +/- ##
============================================
+ Coverage 34.90% 34.98% +0.07%
- Complexity 17555 17585 +30
============================================
Files 1065 1067 +2
Lines 163430 163590 +160
Branches 34938 34950 +12
============================================
+ Hits 57043 57224 +181
+ Misses 96828 96817 -11
+ Partials 9559 9549 -10
Continue to review full report at Codecov.
|
midgleyc
force-pushed
the
equipment-manager-enum
branch
from
e547e2a
to
a7027a3
Compare
| import java.util.function.Function; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class EquipmentSlot { |
There was a problem hiding this comment.
Why contain slot within equipment slot? Can you store the sets as statics on Slot?
There was a problem hiding this comment.
I could do, but I think it looks better to only have CAPITALS be the enum values, and anything else to be out of it. So e.g. Slot.NONE is a slot, but Slot.SLOTS would not be, so I've put the latter at EquipmentSlot.SLOTS.
There was a problem hiding this comment.
I think there are ways to achieve that goal without making Slot a subclass. It's annoying DX that I want to move away from in (for example) AscensionPath.Path
There was a problem hiding this comment.
Why do you think it's annoying DX? I rather like it.
midgleyc
force-pushed
the
equipment-manager-enum
branch
5 times, most recently
from
e7a7fe3
to
f56077b
Compare
Pieces of code using (int slot = EquipmentManager.HAT; slot <= EquipmentManager.FAMILIAR + 1; ++slot) were changed to omit the crown of thrones. UnequipCommand was hat -> sticker3, also changed to omit crown
midgleyc
force-pushed
the
equipment-manager-enum
branch
from
f56077b
to
73d45df
Compare
midgleyc
added
the
enum-conversion
Pieces of code using
were changed to omit the crown of thrones. I don't think it's necessary (and I don't know why they were written this way to begin with).
UnequipCommand was hat -> sticker3. It was also changed to omit the crown.
There are a couple of outstanding things to check on the Maximizer (e.g. CLI calls that equip as they go along) but this should be mostly done.