MoreMcmeta (Forge)

MoreMcmeta (Forge)

158k Downloads

Incompatibility with Continuity

xandercreates opened this issue ยท 6 comments

commented

There appears to be an incompatibility with MoreMcMeta and Continuity that prevents Continuity from being able to load their built in resource packs properly

To reproduce
Steps to reproduce the behavior:

  1. Have MoreMcMeta
  2. Install Continuity
  3. Load the Built-in Packs from Continuity
  4. Latest.log Snippet whilst loading said packs

Expected behavior
Connected Textures from the built in packs
Example:
2021-10-13_22 16 14

Screenshots or videos
See Continuity Issue 15 and take a look at the first post

Which Minecraft versions does this bug affect?
1.17.1

Mod loader
Which mod loaders does this bug affect?
Fabric

Mods list
image

Resource pack
They're Built-in, So here is a screenshot of the loaded packs
image

Additional context
The Errors in the Log Snippet vanished once disabling/removing MoreMcMeta

commented

Version 1.17.1-3.0.1, which contains a fix for this issue, has been released and approved for Forge and Fabric on CurseForge and Modrinth.

Changelog for this version:

  • Fixed incompatibility with Continuity on Fabric.
  • Fixed an issue that prevented non-block sprites from being animated.
  • Unused mipmaps are now freed, reducing memory usage by approximately 25% for most textures.
  • Updated file licenses to 2022.
  • See PR#6 for explanation on most technical changes.

Corresponding versions for 1.16.5 will be released Wednesday or Thursday pending final testing.

commented

I don't have information about this currently, but I want to note that I've seen this. Due to a death in the family and a lot of other things going on, I haven't had time to look into the issue. Hopefully, I will be able to investigate it next week.

commented

I have finally been able to identify the cause of this. Continuity is using a mixin on Minecraft's resource manager. In short, mixins alter Minecraft's code when the game runs, and these are usually tricky to debug. The seemingly-gibberish file names in the logs you are seeing show up because Continuity's mixin that transforms file names is not being applied to MoreMcmeta's custom resource manager.

Technical explanation: Continuity's ReloadableResourceManagerImplExtension interface is applied to MoreMcmeta's SizeSwappingResourceManager, preventing the SimpleReloadableResourceManager (AKA ReloadableResourceManagerImpl) from receiving redirects/mappings. Therefore, Continuity's sprite identifier mappings are disabled, and the hashed file names are not found.

I'm deciding how I want to implement a fix.

commented

Hello. Please let know if something can be done on Continuity's part to make this easier to resolve as well. I know the redirect system I'm using isn't great, but it's the only way I know of to do it that prevents invalid identifiers and textures being outside the textures/ directory.

commented

Hey, thanks for reaching out @PepperCode1.

I've been experimenting with different options for improving compatibility with mods that use mixins on the resource manager. I have Continuity and MoreMcmeta working together in my dev environment, but the methods I've been using are extremely fragile or create resource leaks and other issues. The other option is to use mixins/coremodding, but that obviously creates more compatibility concerns. I've been trying to look for other options instead of requiring other mods to change their mixins when issues like this come up, but I've mostly exhausted the options besides coremodding over the last few days.

It's not really the redirects themselves that cause the problem. Continuity's redirects are added to the map of MoreMcmeta's resource manager wrapper. But MoreMcmeta's wrapper for the resource manager delegates to Minecraft's original one--the wrapper doesn't call its own getResource method. Therefore, the original manager has no knowledge of the redirects.

I believe the simplest fix on Continuity's end would be to make the map you use to store the redirects static so that they are shared between both resource managers. Another option would be to remove the mixin and create a similar wrapper for the resource manager, replacing the existing manager with reflection. The wrappers would then simply delegate to each other and Minecraft's original manager.

I just searched through GitHub for other mods that mixin to the resource manager. There are several, but I don't think they would conflict with MoreMcmeta's wrapper if I kept that. The risk is that I might have to revisit this again if MoreMcmeta doesn't change and something similar pops up in the future.

Please let me know if you want to apply a fix on Continuity's end. I can submit a PR with one of those solutions if that would be helpful.

commented

Closed as an alternative fix I found for this has been merged--I will reply to this thread again once a new version has been formally released.