AntiqueAtlasTeam / AntiqueAtlas

A Minecraft mod that adds a fancy interactive map item.
http://www.minecraftforum.net/topic/2045745-164forge-antique-atlas
Other
251 stars 67 forks source link

Crashes / Reload-Loops on Quilt (Async resource loading) #438

Closed sisby-folk closed 1 year ago

sisby-folk commented 1 year ago

Describe the bug When loading the mod on Quilt with QFAPI, the game crashes on startup on most boots (it's possible for the game to boot successfully). In heavier mod configurations (possibly with mods that guard against these crashes), we've also observed the game enter a reload-loop and not reach the start screen.

To Reproduce Steps to reproduce the behavior:

  1. Open the game with the minimum quilt mod set - Cloth, QFAPI, Architectury, Antique Atlas
  2. Wait and cross your fingers

Expected behavior Game starts up correctly.

Logs Game reaches the start screen successfully on Quilt (Rare): https://mclo.gs/MgKSL8E Game crashes on start on Quilt: https://mclo.gs/tI2ReAH Game reaches the start screen successfully on Fabric (with the giant modlist that would cause reload-loops): https://mclo.gs/JAm1e58

Version information (please complete the following information):

Additional context This is likely a bug/oversight with QFAPI's replication of FAPI behvaiour, and can either be fixed their side or simply tweaked to work around on this side - Further info will need to be provided here by QSL developers.

From what we know already reworks for resource loading have occurred on QFAPI recently, and the mod worked on quilt when we tested the Sep 7 version - so it's likely the parts of the Architectury Commit responsible for asynchronous resource loading that are stressing QFAPI here.

We're primarily opening this issue for visibility and tracking - especially for users that may not understand why issues like this occur and erroneously believe this is a bug with Antique Atlas.

sisby-folk commented 1 year ago

Looking at a possible fix now šŸ‘ looks like a typo on AA's side that fabric just wasn't kicking a fuss about by luck.

tyra314 commented 1 year ago

The issue is that AA needs a particular loading sequence. First textures, then texture sets and last but not least tiles. Iā€™m honestly surprised the current code works on forge and fabric, because that dependency is only defined weakly in the order of the resource loader registering.

While fabric can have these dependencies explicitly defined, forge does not. So Iā€™m yoloing it here šŸ¤·ā€ā™‚ļø

Am 15.09.2022 um 12:44 schrieb Sisby @.***>:

ļ»æ Looking at a possible fix now šŸ‘ looks like a typo on AA's side that fabric just wasn't kicking a fuss about by luck.

ā€” Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.

sisby-folk commented 1 year ago

Thanks for the extra info - The first thing we were alerted to by one of the QSL devs was this, which while helpful unfortunately doesn't seem remedy the issue, likely for the reason you're describing.

I'll keep an ear open for possible ways on getting around that and put them here or in a PR.

sisby-folk commented 1 year ago

I've been a bit busy to dig into this one, but I should probably share the advice we were given -

They really must use the dependency stuff on Fabric, even if they need to mixin their own class Architectury API is incredibly bad on this one

This would mean (as we understand it) mixing in implements IdentifiableResourceReloadListener onto the existing listeners, in order to fill out getFabricDependencies() (here) and then doing... something.

We've been a little busy recently, but we're starting to tinker around with this on our fork - we're missing a few pieces of info required to get this to work, but it's a start.

sisby-folk commented 1 year ago

Update - Architectury has added the optional dependencies argument in the latest release (v4.10.86 for 1.18.2)