GH-71 Support 1.21.3 bukkit platform#71
Conversation
Rollczi
changed the title
Jakubk15
left a comment
Jakubk15
left a comment
There was a problem hiding this comment.
Wygląda sensownie, dużo nowych rzeczy...
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
multification-core/src/com/eternalcode/multification/notice/resolver/NoticeResolverRegistry.java (1)
49-49: CatchingThrowableis very broad. Try catching a more specific exception instead to avoid unnecessary error handling.multification-bukkit/src/com/eternalcode/multification/bukkit/notice/resolver/sound/SoundAccessor.java (1)
27-34: valueOf fallback
This is straightforward. Consider checking for invalid names before reflection if you want friendlier errors.multification-core/src/com/eternalcode/multification/notice/Notice.java (1)
243-244: Consider calling these params volume then pitch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
buildSrc/src/main/kotlin/Versions.kt(1 hunks)examples/bukkit/build.gradle.kts(3 hunks)examples/bukkit/src/main/java/com/eternalcode/example/bukkit/ExamplePlugin.java(2 hunks)examples/bukkit/src/main/java/com/eternalcode/example/bukkit/command/ReloadCommand.java(1 hunks)examples/bukkit/src/main/java/com/eternalcode/example/bukkit/config/ConfigurationManager.java(3 hunks)examples/bukkit/src/main/java/com/eternalcode/example/bukkit/config/MessagesConfig.java(1 hunks)multification-bukkit/src/com/eternalcode/multification/bukkit/notice/resolver/sound/SoundAccessor.java(1 hunks)multification-bukkit/src/com/eternalcode/multification/bukkit/notice/resolver/sound/SoundBukkitResolver.java(3 hunks)multification-cdn/test/com/eternalcode/multification/cdn/NoticeComposerTest.java(6 hunks)multification-core/src/com/eternalcode/multification/notice/Notice.java(4 hunks)multification-core/src/com/eternalcode/multification/notice/resolver/NoticeResolverRegistry.java(1 hunks)multification-core/src/com/eternalcode/multification/notice/resolver/sound/SoundAdventure.java(1 hunks)multification-core/src/com/eternalcode/multification/notice/resolver/sound/SoundAdventureResolver.java(3 hunks)multification-okaeri/src/com/eternalcode/multification/okaeri/MultificationNoticeSerializer.java(2 hunks)multification-okaeri/test/com/eternalcode/multification/okaeri/NoticeSerializerTest.java(6 hunks)
✅ Files skipped from review due to trivial changes (2)
- buildSrc/src/main/kotlin/Versions.kt
- multification-okaeri/src/com/eternalcode/multification/okaeri/MultificationNoticeSerializer.java
🔇 Additional comments (70)
multification-core/src/com/eternalcode/multification/notice/resolver/sound/SoundAdventure.java (5)
10-11: Nice choice for unset markers
Using special constants for unset pitch and volume is a clear approach.
13-14: Default values are handy
Providing defaults makes it easy to ensure consistent playback settings.
16-18: Simple fallback logic
Nice and clean way to use a default if pitch is unset.
20-22: Volume fallback looks good
This mirrors the pitch logic nicely.
24-26: Sound category default
Using MASTER as a default ensures a safe fallback sound source.multification-bukkit/src/com/eternalcode/multification/bukkit/notice/resolver/sound/SoundAccessor.java (3)
8-22: Reflective approach
Grabbing methods by reflection is creative. Just keep an eye on future API changes.
24-25: Private constructor
Neat approach to prevent instantiation.
36-43: name method
Simple approach to fetch enum name.examples/bukkit/src/main/java/com/eternalcode/example/bukkit/command/ReloadCommand.java (4)
11-12: Command definition
Clear command name, easy to remember.
14-15: Field arrangements
Nicely grouped for clarity.
17-20: Constructor
Straightforward dependency injection. Looks good.
22-38: Reload functionality
Great job reloading configs and sending a message. Very user-friendly.examples/bukkit/src/main/java/com/eternalcode/example/bukkit/config/ConfigurationManager.java (5)
7-8: Extra imports
HashSet and Set keep the config list flexible.
17-17: Configs collection
Storing reloadable configs in one place is streamlined.
36-37: Config registration
Adding each config to the reloadable set is a good move for quick reloading.
41-46: Reload method
Reloading each config in a loop is neat and clear.
48-49: Record for config
A small record is a tidy way to hold the file name and instance together.examples/bukkit/src/main/java/com/eternalcode/example/bukkit/config/MessagesConfig.java (1)
40-44: Cool new reload message!
This straightforward approach nicely informs users about config reloads.examples/bukkit/src/main/java/com/eternalcode/example/bukkit/ExamplePlugin.java (2)
4-4: Nice import.
Including ReloadCommand completes your set of commands neatly.
43-43: Great addition of the reload command.
This makes refreshing configs quick and simple for everyone.multification-core/src/com/eternalcode/multification/notice/resolver/sound/SoundAdventureResolver.java (4)
15-15: Helpful constant.
Keeps formatting logic concise and clear in your code.
32-32: Straightforward playSound.
Neat single-line approach for playing sounds with defaults.
38-41: Good fallback for pitch/volume.
Using a simple format when they're unset is a nice touch.
67-69: Neat single-value handling.
Wonderful job simplifying how you handle one-element inputs.multification-bukkit/src/com/eternalcode/multification/bukkit/notice/resolver/sound/SoundBukkitResolver.java (7)
7-7: Importing Locale is a good idea.
Excellent for ensuring correct sound name casing.
44-44: Simplified minimal format.
Return statement is nice for single-argument scenarios.
48-48: Helper method usage.
Neatly keeps your code organized and easy to follow.
55-55: Complete format string.
Ensures all parameters are included in one place.
62-74: Helpful doc and method.
Good job handling modern and legacy sound formats together here.
87-87: Simple single-length handling.
Nicely defaults pitch and volume when only one argument is provided.
94-94: Dynamic sound resolution.
Using SoundAccessor to interpret the sound name is a clean approach.multification-core/src/com/eternalcode/multification/notice/Notice.java (10)
23-23: This import looks good!
96-98: Nice addition for string-based sound.
100-102: Great overload with volume and pitch.
104-106: Helpful method for adding a category.
108-109: Good key-based overload for sound.
118-122: Consistent approach for sound usage.
227-229: Builder method for string-based sound is clear.
231-233: Double-check the parameter order (pitch, volume).
235-237: Category-based variant is straightforward.
239-241: Key overload keeps things flexible.multification-cdn/test/com/eternalcode/multification/cdn/NoticeComposerTest.java (13)
26-28: Added imports look fine.
35-35: BeforeAll annotation usage is good.
38-39: Mockito imports are set up correctly.
53-59: Mocking approach is neat.
60-72: Good job handling multiple sound keys in the mock.
243-243: Sound format 'block.anvil.land MASTER 1.0 1.0' is consistent.
249-251: Test checks look accurate.
267-267: Without category format is awesome.
273-274: Assertions here are clear.
292-292: Compact sound notation is valid.
298-300: Volume and pitch defaults look well-tested.
357-360: Additional test class is well-defined.
361-382: Covers missing volume/pitch scenario nicely.multification-okaeri/test/com/eternalcode/multification/okaeri/NoticeSerializerTest.java (12)
4-6: New Bukkit imports are good.
32-32: BeforeAll is well-placed.
35-36: Mockito static mocking is set up smoothly.
44-65: Detailed mock behavior is solid.
234-234: Sound string usage aligns with the new format.
240-243: Test logic for volume/pitch checks out.
258-258: No category sound string is consistent.
264-266: Verifies category is null correctly.
283-283: Block sound key is correct.
289-291: Validation for default volume/pitch is good.
348-350: Simple config class usage looks good.
352-372: Test for missing volume/pitch covers real scenario.examples/bukkit/build.gradle.kts (4)
3-3: Shadow plugin upgrade looks fine.
5-5: Run-Paper version updated for new functionalities.
45-46: Commenting out relocations is understandable.
59-59: Bumping to Minecraft 1.21.4 keeps it current.
2 participants
No description provided.