-
Notifications
You must be signed in to change notification settings - Fork 33
Modern config compat layers #476
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: v1
Are you sure you want to change the base?
Conversation
# Conflicts: # gradle.properties # gradle/libs.versions.toml # minecraft/build.gradle.kts # minecraft/src/main/java/org/polyfrost/oneconfig/internal/OneConfigMixinInit.java # modules/config-impl/src/main/kotlin/org/polyfrost/oneconfig/api/config/v1/internal/ConfigVisualizer.kt
# Conflicts: # gradle/libs.versions.toml # settings.gradle.kts
minecraft/src/main/java/org/polyfrost/oneconfig/internal/OneConfigEarlyMixinInit.java
Show resolved
Hide resolved
minecraft/src/main/java/org/polyfrost/oneconfig/internal/OneConfigMixinInit.java
Outdated
Show resolved
Hide resolved
minecraft/src/main/kotlin/org/polyfrost/oneconfig/internal/compat/MoulConfigCompat.kt
Outdated
Show resolved
Hide resolved
import kotlin.reflect.KClass | ||
|
||
@Suppress("UNCHECKED_CAST") | ||
internal var Property<*>.visualizer: Class<out Visualizer>? |
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 we make this public?
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.
idk your call
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.
@nextdayy what do you think
...abric/src/main/java/org/polyfrost/oneconfig/internal/mixin/compat/modmenu/Mixin_ModMenu.java
Show resolved
Hide resolved
...sions/1.16.5-fabric/src/main/kotlin/org/polyfrost/oneconfig/internal/compat/ModMenuCompat.kt
Show resolved
Hide resolved
...sions/1.19.2-fabric/src/main/kotlin/org/polyfrost/oneconfig/internal/compat/RConfigCompat.kt
Show resolved
Hide resolved
...sions/1.20.4-fabric/src/main/kotlin/org/polyfrost/oneconfig/internal/compat/RConfigCompat.kt
Show resolved
Hide resolved
minecraft/src/main/java/org/polyfrost/oneconfig/internal/OneConfigEarlyMixinInit.java
Show resolved
Hide resolved
minecraft/src/main/kotlin/org/polyfrost/oneconfig/internal/compat/MoulConfigCompat.kt
Outdated
Show resolved
Hide resolved
i don't really see the point of the relocator system - can't you just have multiple mixin targets for the mixin(s) ? |
we still need the main MoulConfigCompat class and we (meowora and i) concluded that doing it this way was the best |
I personally think all usage of kotlin reflect (including KClass) should be removed and replaced with just java methods, as the kotlin ones spin up so many methods etc that are considerably slower and unnecessary here. |
also I don't hate the compile time plugin stuff, but why not just use multi target accessors and mixins? |
# Conflicts: # gradle/libs.versions.toml # modules/config-impl/api/config-impl.api # modules/hud/api/hud.api
It would make adding new moulconfig instances a lot more complicated/annoying, I personally also feel like that would be way too much work, especially since the compile time stuff already works. |
I don't mind either, just a decision what i should do would be awesome :3 |
i would prefer us using java reflection |
Description
Adds a compat layer for some of the most popular config libraries.
Closes #457 #458 #459 #461
Checklist