Conversation
Vankka
left a comment
Vankka
left a comment
There was a problem hiding this comment.
Thank you for contributing this, I've left comments below
And I agree that dealing with relocated & unrelocated dependencies at the same time is a pain. Last time I checked shadowJar didn't have a way to exclude source files from having relocations applied to them, unfortunately (albeit this was a long time ago).
I'm fine with the unrelocate approach for now, but I will probably try to find a neater approach in the future (maybe just a separate paper artifact that doesn't relocate adventure dependencies)
|
|
||
| if (PaperComponentCheck.IS_AVAILABLE) { | ||
| // Paper (Since 1.16) | ||
| registerIntegration("com.discordsrv.bukkit.integration.chat.CarbonChatIntegration"); |
There was a problem hiding this comment.
Please move this to the BukkitDiscordSRV class to be together with the other integrations
| static MinecraftComponent message(CarbonChatEvent event) { | ||
| return MinecraftComponent.fromAdventure( | ||
| ((com.discordsrv.unrelocate.net.draycia.carbon.api.event.events.CarbonChatEvent) (Object) event) | ||
| .message()); | ||
| } |
There was a problem hiding this comment.
This can be replaced by the PaperComponentHandle helper class,
private static final PaperComponentHandle.Get<CarbonChatEvent> MESSAGE_HANDLE = PaperComponentHandle.get(CarbonChatEvent.class, "message");
MinecraftComponent component = MESSAGE_HANDLE.getAPI(event);| discordSRV.scheduler().run(() -> discordSRV.eventBus().publish( | ||
| new GameChatMessagePreProcessEvent(null, srvPlayer, component, new CarbonGameChannel(chatChannel), | ||
| false))); |
There was a problem hiding this comment.
| discordSRV.scheduler().run(() -> discordSRV.eventBus().publish( | |
| new GameChatMessagePreProcessEvent(null, srvPlayer, component, new CarbonGameChannel(chatChannel), | |
| false))); | |
| discordSRV.scheduler().run(() -> discordSRV.eventBus().publish( | |
| new GameChatMessagePreProcessEvent(null, srvPlayer, component, new CarbonGameChannel(chatChannel), false) | |
| )); |
| if (key.value().equalsIgnoreCase(channelName) || channel.commandName().equalsIgnoreCase(channelName)) { | ||
| return channel; | ||
| } |
There was a problem hiding this comment.
Is the namespace on these keys constant? If not this should probably match the entire key as well as the value (if there are possible overlapping values)
I would prefer to drop the command name matching, or have it deprioritized so the all the keys are checked before any command names are checked (is the actual channel name hard to find for the user?)
2 participants
Lots of CarbonChat classes have been shimmed to avoid runtime
NoSuchMethodExceptionerrors due to the Adventure API being relocated (as both DiscordSRV and Carbon use it). I am not too familiar with the DiscordSRV codebase so if there's a different way you'd prefer for this changes to be made, please either leave a PR review with detailed instructions of how to do so, or simply make edits to the PR directly.Changes have been lightly tested and seem to work, however further testing is currently being done.