Update ClassLoaderFixer to v2 #29
Conversation
| public RelaunchClassLoader() { | ||
| super(new URL[0], FMLRelauncher.class.getClassLoader()); | ||
| this.sources = new ArrayList<>(); | ||
| this.parent = this.getClass().getClassLoader(); |
There was a problem hiding this comment.
Tbh, get off your high horse. If you think this is unnecessary, just close this PR without any comment. But don't spend your time writing such comments if the explanation is already there in the premise of the PR.
| this.addTransformerExclusion("cpw.mods.fml.common.asm.SideOnly"); | ||
| this.addTransformerExclusion("cpw.mods.fml.common.Side"); | ||
| this.addTransformerExclusion("fr.catcore.fabricatedforge."); | ||
| this.addClassLoaderExclusion("com.llamalad7.mixinextras."); |
There was a problem hiding this comment.
Did these become static? If not, just leave it be...
| // Class<?> cl = super.findClass(name); | ||
| // cachedClasses.put(name, cl); | ||
| // return cl; | ||
| // try |
There was a problem hiding this comment.
What's the point in adding yet more commented out code?
There was a problem hiding this comment.
To keep the codebase consistent. Explains the working principle behind ClassLoaderFixer. If it's unnecessary, then this entire commented out code snippet should have already been removed ages ago
| @@ -14,13 +14,11 @@ | |||
| package cpw.mods.fml.relauncher; | |||
|
|
|||
| import java.net.*; | |||
| import java.security.CodeSigner; | |||
| import java.security.CodeSource; | |||
| import java.util.jar.Attributes.Name; | |||
There was a problem hiding this comment.
Seems like an unnecessary style chane, to change this to an inner class import.
There was a problem hiding this comment.
Unnecessary? Might be. Just copy-pasted from my ClassLoaderFixer source code.
| public void addURL(URL url) { | ||
| super.addURL(url); | ||
|
|
||
| FabricLauncherBase.getLauncher().addToClassPath(UrlUtil.asPath(url)); | ||
|
|
||
| this.sources.add(url); |
There was a problem hiding this comment.
Again, why would you remove this? It's making the type of call clearer.
There was a problem hiding this comment.
Same as one of the above here
|
There was a problem hiding this comment.
Overall, feel free to just close this PR without any comment or feel free to merge it. It's your decision. As already said in the description of this PR: Incrementing the FIXER_VERSION to 2 is the only important change in this PR. The rest would just have made the codebase more consistent with the original ClassLoaderFixer.
And as already said, just add the FIXER_VERSION line to all the versions between 1.3.1 and 1.4.5 (since the first Forge version for 1.4.6 already fixed it and makes ClassLoaderFixer unnecessary).
| @@ -14,13 +14,11 @@ | |||
| package cpw.mods.fml.relauncher; | |||
|
|
|||
| import java.net.*; | |||
| import java.security.CodeSigner; | |||
| import java.security.CodeSource; | |||
| import java.util.jar.Attributes.Name; | |||
There was a problem hiding this comment.
Unnecessary? Might be. Just copy-pasted from my ClassLoaderFixer source code.
| public RelaunchClassLoader() { | ||
| super(new URL[0], FMLRelauncher.class.getClassLoader()); | ||
| this.sources = new ArrayList<>(); | ||
| this.parent = this.getClass().getClassLoader(); |
There was a problem hiding this comment.
Tbh, get off your high horse. If you think this is unnecessary, just close this PR without any comment. But don't spend your time writing such comments if the explanation is already there in the premise of the PR.
| this.addTransformerExclusion("cpw.mods.fml.common.asm.SideOnly"); | ||
| this.addTransformerExclusion("cpw.mods.fml.common.Side"); | ||
| this.addTransformerExclusion("fr.catcore.fabricatedforge."); | ||
| this.addClassLoaderExclusion("com.llamalad7.mixinextras."); |
| // Class<?> cl = super.findClass(name); | ||
| // cachedClasses.put(name, cl); | ||
| // return cl; | ||
| // try |
There was a problem hiding this comment.
To keep the codebase consistent. Explains the working principle behind ClassLoaderFixer. If it's unnecessary, then this entire commented out code snippet should have already been removed ages ago
| public void addURL(URL url) { | ||
| super.addURL(url); | ||
|
|
||
| FabricLauncherBase.getLauncher().addToClassPath(UrlUtil.asPath(url)); | ||
|
|
||
| this.sources.add(url); |
There was a problem hiding this comment.
Same as one of the above here
|
Thanks for the contribution. |
|
Perfect, thank you very much and good luck! :) |
Today I released ClassLoaderFixer 2. Most changes here are not really necessary, but I still applied some of them to be consistent :)
By the way, this exact same class also works for every version up to 1.4.5 - so feel free to incorporate it in every version up until then.
At least
public static String FIXER_VERSION = "2";would be important to add everywhere to tell Block Helper to shut up (otherwise it warns the user everytime that he needs a properRelaunchClassLoaderimplementation - even though it is indeed fine)