search

paulevsGitch / BetterNether

BetterNether Mod
GNU General Public License v3.0
105 stars 72 forks source link

Incompatibility with Quilt (uses FAPI impl) #544

Closed sisby-folk closed 2 years ago

sisby-folk commented 2 years ago 
Minecraft 1.18.2
Quilt Loader 0.16.0-beta.16
bclib-1.4.5
better-nether-6.1.1
qsl-1.1.0-beta.8_qfapi-1.0.0-beta.12_fapi-0.51.1_mc-1.18.2

latest.log

Functions correctly with 

Fabric 0.14.4
fabric-api-0.51.1+1.18.2.jar

Suggested reason: mod uses bits from FAPI's impl package (perhaps erroneously)

paulevsGitch commented 2 years ago

BetterNether is a Fabric mod, it was not made for Quilt

msparkles commented 2 years ago

Doesn't mean one should touch the impl?

sisby-folk commented 2 years ago

@paulevsGitch While I understand this - The vast majority of fabric mods work with Quilt by default, and the reason for this incompatibility is very much an edge case that explicitly cant be fixed on QSL's end. Can you confirm specifically whether you won't support the mod in this way / whether you'll accept relevant pull requests?

paulevsGitch commented 2 years ago

There are no plans to make specific Quilt-compatible version at this moment

sisby-folk commented 2 years ago

Could you please clarify further on the other parts of the above question? No "quilt compatible version" of any fabric mod should need to exist, as outlined.

paulevsGitch commented 2 years ago

BetterNether doesn't use impl package. BetterEnd require specific classes from that package

LambdAurora commented 2 years ago

BetterNether doesn't use impl package. BetterEnd require specific classes from that package

@paulevsGitch excuse me? We very much know how to read import of net.fabricmc.fabric.mixin.object.builder.AbstractBlockSettingsAccessor which is an impl class.

Second, any mixin accessor in Fabric API is considered implementation, you're supposed to recreate the mixin yourself. Or use an access widener, both works. This is an edge-case where for some reason Fabric doesn't support modifying the material of a block settings for an unknown reason.

Finally, your mods regularly cause issues with other mods, it's time to consider a proper cleanup of the codebase.

paulevsGitch commented 2 years ago

We very much know how to read

First of all - don't be rude, no one insulted you. I didn't write code that used AbstractBlockSettingsAccessor and I'm not developing BetterNether since 1.16.5.

Second, any mixin accessor in Fabric API is considered implementation, you're supposed to recreate the mixin yourself

Second - any part of the API can be used, that's why it is an API. If there is something that requires usage of API, for example accessors - it can be used, that's why API exists - it used to share same code between mods.

Finally, your mods regularly cause issues with other mods

Any mods can cause issues with any other mod. All known issues are listed on Git. If you have more issues - just add them on tracker.

it's time to consider a proper cleanup of the codebase.

If you know people that want to rewrite 4+ years codebase of dozen thousands lines of code for free in 5/7 mode - you can invite them, I don't know any. All code that exists at this moment is maintained by quiqueck for his own will for free. We don't have any more resources to do this.

Finally - this mod was developed to be used with existing Fabric API, not with any side APIs that tries to replicate it. There are no plans to adapt code to work with other APIs in the nearest future.

Patbox commented 2 years ago

Second - any part of the API can be used, that's why it is an API. If there is something that requires usage of API, for example accessors - it can be used, that's why API exists - it used to share same code between mods.

If that was the case, fabric wouldn't split it into api and impl packages. Everything outside of api is intended only for fabric api internal usage. One main version bump and it won't work (without any changes for api)

LambdAurora commented 2 years ago

The only actual API is what are in the api packages.

The rest is implementation only, which means it can change at any time and have no guarantee of stability, this goes for the impl and mixin package. And at least, Fabric API was made this way to ensure mods could work on re-implementations of the API.

msparkles commented 2 years ago

I don't get what's so hard to get about not touching the impl when using an API.

LambdAurora commented 2 years ago

I don't get what's so hard to get about not touching the impl when using an API.

To be fair, there is valid use cases of using implementation details, but they only highlight the fact that the API fails to provide what modders need.

felinusfish commented 2 years ago

I don't get what's so hard to get about not touching the impl when using an API.

If it's not hard, why don't you fix it then? This is old code. Rewrite if you so wish.