Big Update #42
Big Update #42
Conversation
|
Based on #24 (comment) I am not sure if the logging properties should be changed (though fixing the gametest ones definitely is good) |
build.gradle
Outdated
| if (run.project.javaToolchains.launcherFor(java.toolchain).map { it.metadata.vendor }.getOrElse("").contains("JetBrains")) { | ||
| run.jvmArgument("-XX:+AllowEnhancedClassRedefinition") | ||
| } |
There was a problem hiding this comment.
Also this isn't enough to just support DCEVM out of the box if that was your intention. It just makes it so if you are using it then it gets properly enabled. To make it enabled by default you would need to set it via the toolchain, and also update the gradle wrapper to at least 8.4 so that it can provide the jetbrains one
There was a problem hiding this comment.
My intention indeed WAS to make it so that if you are using it then it gets properly enabled
There was a problem hiding this comment.
I don't think this is necessary; I would instead suggest something like this:
// If you wish to use enhanced class hotwsapping during debug (DCEVM), uncomment this line.
// java.toolchain.vendor = JvmVendorSpec.JETBRAINS // NOTE: Requires Gradle 8.4 or above
// If you wish to use enhanced class hotwsapping during debug (DCEVM), uncomment this block.
/*
runs.configureEach {
it.jvmArgument "-XX:+AllowEnhancedClassRedefinition"
}
*/
|
got it |
build.gradle
Outdated
| systemProperty 'forge.logging.console.level', 'debug' | ||
| run.systemProperty 'forge.logging.console.level', 'debug' | ||
|
|
||
| if (run.project.javaToolchains.launcherFor(java.toolchain).map { it.metadata.vendor }.getOrElse("").contains("JetBrains")) { |
There was a problem hiding this comment.
I'm not really a fan of adding something so specific to the MDK. Using DCEVM is an advanced configuration and comes with its own set of problems. Without any documentation this also looks like a completely arbitrary check to anyone not already familiar with DCEVM. I personally would prefer having the docs mention the possibility of using DCEVM and what is required to set it up instead of having this in the MDK, especially considering that this is nowhere near the complete setup.
There was a problem hiding this comment.
Commenting the entire block out as-is is not what I meant. If it's commented out and has a comment explaining what it's for, then the check is unnecessary. It should either be what FiniteReality mentioned above with two simple, commented-out blocks or (preferably, in my opinion) removed entirely from the MDK and instead documented on our docs
|
I want to redo this |
Update to Neo stable and foojay, enable support for DCEVM and automatically add access transformers when present
EDIT: Update Parchment!
Also supersedes #38