Skip to content

Conversation

@robotgryphon
Copy link

Currently the mod loader will reject valid Access Transformer files in game libraries, even though the locator can find the entries in their default location. This means that dev-time application of ATs from game libraries works while production has no effect, causing easily missed bugs.

@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@lukebemish
Copy link
Contributor

Hmm. I'm not really sure about this -- dev-time discovery of ATs from LIBRARYs would also work and have no effect in prod. As would dev time use of an AT that's just not bundled, or not at the default location and not declared in the mods.toml. And... etc.; I feel like at some level this isn't really necessary? After all -- if you're providing an AT, why can't you just use a mod instead of a GAMELIBRARY? The two act basically identical -- in fact, all that's needed to make a given GAMELIBRARY a mod is (a) remove the FMLModType manifest entry, (b) provide a mods.toml in the same source set with only a license, mod ID, and version (plus loader info on older MC versions), and call it a day. There's no case where this is a non-trivial change that I'm aware of. I feel like if you're using any "mod" feature, like an AT, your thing should probably be a mod to begin with?

Also as an aside, I'm not entirely against this -- after all, mixin configs are still loaded from GAMELIBRARYs for some reason? And consistent behaviour in this regard feels sensible -- but if this is implemented can we please just include the game libraries in the original loop instead of duplicating the whole thing? Feels a bit wasteful.

Copy link
Member

@Technici4n Technici4n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is for Gander, it should probably just be made a mod. The only practical difference is that mods appear in the mod menu. Might be an XY fix?

We kept the mixin manifest attribute because that's what MixinExtras uses, and we didn't want to show it in the mod list as part of a "clean" NeoForge distribution. It is also fairly established in the broader mixin ecosystem.

@lukebemish
Copy link
Contributor

As noted on discord, making gander use mods instead of GAMELIBRARYs is a ~30 line change or so, so yeah, at least in that case that seems like a cleaner solution -- ATs are a mod feature after all, this solution feels kinda like, I dunno, letting @EventBusSubscriber work in GAMELIBRARYs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants