-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/gh 3/title support #12
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
Conversation
| jvmToolchain(21) | ||
| } | ||
|
|
||
| val shadowJarOnly: Boolean = project.property("shadowJarOnly")?.toString()?.toBoolean() ?: false |
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 don't anticipate this being something that gets toggled on and off in this plugin, but I wanted this to be something I could move into the template (which actually should in part be a Gradle plugin...)
| import com.fasterxml.jackson.databind.ser.std.StdSerializer | ||
| import kotlin.time.Duration | ||
|
|
||
| class DurationSerializer : StdSerializer<Duration>(Duration::class.java) { |
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.
This class isn't strictly necessary to read (deserialize) the config, but including it is helpful for testing. For example, to create the default config I constructed a SimpleAnnounceConfig object and got the yaml out via a test class with println(objectMapper.writeValueAsString(config))
| import org.bukkit.plugin.Plugin | ||
| import org.simplemc.simpleannounce.config.SimpleAnnounceConfig.AnnouncementConfig.Title | ||
|
|
||
| class TitleSender( |
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.
The "main" "feature" of this PR is only ~30 lines out of almost 1000 changed 😬
| override val includesPermissions: List<String> = emptyList(), | ||
| override val excludesPermissions: List<String> = emptyList(), | ||
| @field:JsonAlias("message") override val messages: List<BossBarMessage>, | ||
| @field:JsonUnwrapped val barConfig: BarConfig = BarConfig(), |
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 could have used some other kotlin config library like hoplite instead of jackson and avoided a small portion of the pain (with an old version of jackson being included with spigot), but would have still had to deal with the kotlin-reflect problems and would lose the ability to unwrap properties like I do here.
...this makes it so the announcement configuration for a boss-type announcement has the barConfig fields directly (eg hold) instead of nested inside a barConfig field/object. Same with them being on the messages themselves.
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.
May or may not be interesting for you, but Configurate is a popular config library for modern plugins (Paper uses it)
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.
hoplite is simpler and easier to use IMO, but both would lose some of the jackson niceties (although Configurate seems to use jackson for its JSON support). Configurate also uses kotlin-reflect for its kotlin data class support, so many of the same things would need to be addressed anyway.
Adds title sender
The main motivation was adding a title announcement type, but obviously there's a bit more here per the CHANGELOG.
config.get...calls pretty awkward60s,10m,1h) rather than implicitly being either minutes or secondslibrariesdoesn't isolate the classpath at all...Spigot/craftbukkit/etc ship with jackson (an old version...) so having jackson pulled vialibrariesin theplugin.ymldoesn't work (old version is on classpath, resolved first)libraries:) and the regular "offline"-mode shadow jar with everything. This resulted in some weirdness/conflicts with what could be picked up in the jar vs out. Very much not worth it for the smaller download size...kotlin-reflect(jackson kotlin module dependency) being required on the classpath has some problems requiring workarounds...kotlin.Anyisn't a real class contained in thestdlibjar, but a virtual/builtin type. Shadow relocations references to it, but the builtin doesn't change and thus it's not found.Completes #3