-
Notifications
You must be signed in to change notification settings - Fork 35
Update to support EventBus7 #129
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
base: 5.x
Are you sure you want to change the base?
Conversation
Do old Forge versions (1.21.1-1.21.5) use EB7? This could be a breaking change that I don't want to merge into KFF 6.x |
But, it also breaks support for MC 1.21.6-1.21.7 |
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'll review the KotlinModContainer class later, I'm suspicious of the module reflection, but knowing LexForge, it's probably just how they decided to do 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.
Why all of these random Gradle options? If these changes are just for your setup, please remove them from the PR
|
||
min_mc_version = 1.20.6 | ||
unsupported_mc_version = 1.22 | ||
|
||
# KOTLIN FOR FORGE VERSION | ||
kff_version=5.9.0 | ||
kff_max_version=6.0.0 |
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 should be 7.0.0, not 6.1.0
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've changed that
@@ -132,9 +133,9 @@ public object AutoKotlinEventBusSubscriber { | |||
|
|||
private fun registerTo(any: Any, target: Mod.EventBusSubscriber.Bus, mod: KotlinModContainer) { | |||
if (target == Mod.EventBusSubscriber.Bus.FORGE) { | |||
target.bus().get().register(any) | |||
target.bus().get()?.register(MethodHandles.lookup(), any) |
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.
Why is the bus supplier now nullable? Should this log an error if the bus is null?
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.
Should I use !! or ?
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 compiler 2.2.0 won't let me go without !! or ?
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.
Please remove this change from the PR
Is it ok now? |
ok
…On Sun, Jul 13, 2025 at 6:39 AM thedarkcolour ***@***.***> wrote:
***@***.**** requested changes on this pull request.
I'll review the KotlinModContainer class later, I'm suspicious of the
module reflection, but knowing LexForge, it's probably just how they
decided to do it...
------------------------------
On gradle.properties
<#129 (comment)>
:
Why all of these random Gradle options? If these changes are just for your
setup, please remove them from the PR
------------------------------
In gradle.properties
<#129 (comment)>
:
>
min_mc_version = 1.20.6
unsupported_mc_version = 1.22
# KOTLIN FOR FORGE VERSION
-kff_version=5.9.0
-kff_max_version=6.0.0
This should be 7.0.0, not 6.1.0
------------------------------
In
src/kfflang/forge/kotlin/thedarkcolour/kotlinforforge/AutoKotlinEventBusSubscriber.kt
<#129 (comment)>
:
> @@ -132,9 +133,9 @@ public object AutoKotlinEventBusSubscriber {
private fun registerTo(any: Any, target: Mod.EventBusSubscriber.Bus, mod: KotlinModContainer) {
if (target == Mod.EventBusSubscriber.Bus.FORGE) {
- target.bus().get().register(any)
+ target.bus().get()?.register(MethodHandles.lookup(), any)
Why is the bus supplier now nullable? Should this log an error if the bus
is null?
------------------------------
On
src/kfflang/forge/resources/META-INF/services/net.minecraftforge.forgespi.language.IModLanguageProvider
<#129 (comment)>
:
Please remove this change from the PR
—
Reply to this email directly, view it on GitHub
<#129 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BK6AZOOB5WOF5UYTEULFC733IHWK5AVCNFSM6AAAAACBL6TRCKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTAMJTHA4DCOJSGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Updated KotlinForForge to support MC 1.21.7 and the new EventBus7 introduced in Forge 57.x.
Mod containers now use BusGroup for event dispatching, keeping up with the Forge changes.