Support split client/server intermediary for old Minecraft versions #912
Conversation
|
Tested against modified babric intermediaries and it works flawlessly. |
src/main/java/net/fabricmc/loader/impl/game/GameProviderHelper.java
Outdated
Show resolved
Hide resolved
src/main/java/net/fabricmc/loader/impl/launch/MappingConfiguration.java
Outdated
Show resolved
Hide resolved
|
I've externalized the code defining source namespace to game provider. |
|
I'll wait for the next TR release before updating the code accordingly. |
| VersionPredicate mergedVersionPredicate = VersionPredicate.parse(">=1.3"); | ||
|
|
||
| if (!mergedVersionPredicate.test(Version.parse(getNormalizedGameVersion()))) { | ||
| sourceNamespace = envType == EnvType.CLIENT ? "clientOfficial" : "serverOfficial"; |
There was a problem hiding this comment.
(Leaving a comment, as requested by @thecatcore) Treating an enum value as a boolean feels wrong, even if it will always realistically be one or the other.
There was a problem hiding this comment.
I think in this case it's fine, even if strictly something like a switch statement would be better. I don't see that enum getting another value.
src/main/java/net/fabricmc/loader/impl/util/mappings/TinyRemapperMappingsHelper.java
Outdated
Show resolved
Hide resolved
…ed-intermediary-support
…ingProviders instead of doing it ourselves
|
I've updated TR to take advantage of FabricMC/tiny-remapper#128. |
thecatcore
requested review from
modmuss50,
NebelNidas,
SpaceWalkerRS and
halotroop2288
| VersionPredicate mergedVersionPredicate = VersionPredicate.parse(">=1.3"); | ||
|
|
||
| if (!mergedVersionPredicate.test(Version.parse(getNormalizedGameVersion()))) { | ||
| sourceNamespace = envType == EnvType.CLIENT ? "clientOfficial" : "serverOfficial"; |
There was a problem hiding this comment.
I think in this case it's fine, even if strictly something like a switch statement would be better. I don't see that enum getting another value.
sounds good |
…ed-intermediary-support # Conflicts: # build.gradle
|
Fixed merge conflicts and fully replaced TinyRemapperMappingsHelper with TR's TinyUtils. |
| try { | ||
| VersionPredicate mergedVersionPredicate = VersionPredicate.parse(">=1.3"); | ||
|
|
||
| if (!mergedVersionPredicate.test(Version.parse(getNormalizedGameVersion()))) { |
There was a problem hiding this comment.
Could this be a system property to enable (or disable?)? You should still be able to easily set when installing loader.
There was a problem hiding this comment.
Do you mean a system property for "merged intermediaries" or for the source namespace?
There was a problem hiding this comment.
I think its important for there to still be a way to still be able to use the previous "merged" intermediaries on these older versions. Another option could be to just check what source namespaces are present in the current mapping set.
There was a problem hiding this comment.
what should be the default then? previous or new "merged"?
There was a problem hiding this comment.
You could probably just check the namespaces, if it has official then use that, if not use one of the others. Removes the need for the version comparision as well.
…namespace is missing
modmuss50
changed the title
Loader follow up to FabricMC/fabric-loom#1026, allowing to use the same intermediary format in production as well.