search

OreCruncher / DynamicSurroundingsFabric

Dynamic Surroundings mod for Minecraft
MIT License
62 stars 25 forks source link

[1.20.1] [Sinytra] Conflict w/ModernFix registry modifications #68

Closed liangmoe closed 5 months ago

liangmoe commented 5 months ago

The know missing feature: bow pulling sound and steam columns effect

0.2.2 steam columns: 2024-01-27_04 19 45

0.3.0 steam colums: 2024-01-27_04 15 36

Fabric side 0.3.0 steam colums: image

0.2.2 latest.log: https://mclo.gs/MLju2jw

0.3.0 latest.log: https://mclo.gs/AHuOgtu

OreCruncher commented 5 months ago

What modpack? I just loaded up my Forge test instance and its working. Also, is this integrated server or connected to a remote server? Lastly, try the command /dsreload and see if it fixes it.

OreCruncher commented 5 months ago

image

It looks like something is interfering with my scanning of my mods resources. Going to need the mod pack.

liangmoe commented 5 months ago

Cant drop mrpack here, can i contact you on discord? modrinth page's discord link is invaild

OreCruncher commented 5 months ago

yeah - ping me directly on discord.

liangmoe commented 5 months ago

ive send friend request to you, cant find the working discord link and also cant find you in connector's discord

OreCruncher commented 5 months ago

Ok - looks to be a conflict with ModernFix. I changed the method of how I scan for resource pack resources to a different API in the ResourceManager. You can turn off the resource pack performance tweaks of ModernFix by editing the modernfix-mixins.properties file and add "mixin.perf.resourcepacks=false" to the bottom.

OreCruncher commented 5 months ago

I do not see this issue with ModernFix for Fabric on 1.20.4.

liangmoe commented 5 months ago

I do not see the issue with modernfix on fabric 1.20.1 either

OreCruncher commented 5 months ago

OK - beat my head on this one quite a bit. I think it is specifically ModernFix in a forge setup with Sinytra. Only way to work around it is to disable the perf for resources for ModernFix. On the plus side I made resource loading is much faster. With ATM9 Forge using Sinytra and my mod loading was about 70 seconds. Got it down to about 300 or so milliseconds.

embeddedt commented 5 months ago

Could you explain what you changed about resource access in 0.3.0 and how it works now? There is a fix coming in 5.13 for some resource pack issues and I want to ensure this will also be addressed.

OreCruncher commented 5 months ago

It was the method for resource access. My mod scans loaded mods and resource packs looking the assets it needs. I made a tweak to take into account external resource packs. Though I don't know for certain, I think there is logic somewhere (at what layer/aspect that I do not know) that makes an assumption about how a given mod would interact with resource packs.

Regarding tags, these things live in the /data path of a resource pack. In 0.3.0 I moved my tagging into /assets. Minecraft will not load my tag information since it is not in /data, which is what I want - I am a client-side mod after all. I have special scanning logic that will look in /assets and /data for tag information and initialize its internal tag cache. This is important if a user uses fabric + clientside mods to connect with a Vanilla server. The server has no notion of forge or c tags, so my tag cache has to do the crawl through client side /asset and /data folders looking for the tag information.

Link to the code that I am currently using to scan for pack resources: https://github.com/OreCruncher/DynamicSurroundingsFabric/blob/ac451d1376e3b88f98f73404da7fd235736e57fe/src/main/java/org/orecruncher/dsurround/lib/resources/ModConfigResourceFinder.java#L23

One other thing I noticed when scanning for sounds.json, when ModernFix resource mixin was enabled it would only return "minecraft:sounds.json". When disabled it gave me all the mods that have sounds.json files. Relevant code location: https://github.com/OreCruncher/DynamicSurroundingsFabric/blob/ac451d1376e3b88f98f73404da7fd235736e57fe/src/main/java/org/orecruncher/dsurround/lib/resources/ClientResourceFinder.java#L32.

To be clear the above code has not been released yet. I am planning on releasing in the next day or so - need to do more validation.

embeddedt commented 5 months ago

Thanks for the info & hints.

Can someone confirm that https://nightly.link/embeddedt/ModernFix/workflows/gradle/1.20/Package.zip works without requiring mixin.perf.resourcepacks to be disabled?

liangmoe commented 5 months ago

Thanks for the info & hints.

Can someone confirm that https://nightly.link/embeddedt/ModernFix/workflows/gradle/1.20/Package.zip works without requiring mixin.perf.resourcepacks to be disabled?

Can confirm that

OreCruncher commented 5 months ago

@embeddedt Yep - looks good. I dropped the Forge version into ATM9 w/Sinytra and my mod and it scans things correctly.

OreCruncher commented 5 months ago

Posted 0.3.1 versions to CurseForge/GitHub. Though there is no specific fix for this particular issue I did rework asset handling to improve performance. Tested the fixes with ATM9 and the fixes made to ModernFix.