search

PatchworkMC / patchwork-api

An attempt to reimplement the Minecraft Forge API on Fabric
GNU Lesser General Public License v2.1
283 stars 48 forks source link

Publicized MinecraftClient#textureManager #190

Closed YTG1234 closed 4 years ago

YTG1234 commented 4 years ago

Just saw this on #125 so I decided to do it.

YTG1234 commented 4 years ago

Maybe we could also move all Forge's access modifications that are already in Patchwork to this new module, idk how it's going to be possible to "load" the access widener when developing other modules. Any thoughts?

TheGlitch76 commented 4 years ago

Moving access wideners to their own module isn't the right solution yet because nobody has made a jar processor to make access wideners transitive. I think instead of its own module, for now this should either be in an appropriate module that already exists (Check the Forge source code and see if it's used anywhere), or failing that dropped in patchwork-fml.

YTG1234 commented 4 years ago

I think it may be used in the forge CloudRenderer: mc.textureManager.bindTexture(texture);. I'm not sure

YTG1234 commented 4 years ago

Yep. It's only used in CloudRenderer

YTG1234 commented 4 years ago

I will put it in the FML module

YTG1234 commented 4 years ago

Moved it to the FML module for now. I didn't bump the version - as the commit msg says.

YTG1234 commented 4 years ago

Turns out I put a /. Fixed it to a ., it should run now.

YTG1234 commented 4 years ago

Updated the title so it's less confusing

YTG1234 commented 4 years ago

@famous1622 I changed things up a little, can you review again?

rikka0w0 commented 4 years ago

Moving access wideners to their own module isn't the right solution yet because nobody has made a jar processor to make access wideners transitive. I think instead of its own module, for now this should either be in an appropriate module that already exists (Check the Forge source code and see if it's used anywhere), or failing that dropped in patchwork-fml.

I think in the future the rendering stuff should have their own package. But for now I would agree that this aw should be in patchwork-fml. Sometimes mc.textureManager is used by mods in their tileentity render and entity render, I don't there is a direct reference to that field in Forge.

YTG1234 commented 4 years ago

It's interesting that in most places Forge uses getTextureManager(), so I don't understand why the field has been made public

cittyinthecloud commented 4 years ago

Was probably public forever ago and they don't want to break mods

YTG1234 commented 4 years ago

Was probably public forever ago and they don't want to break mods

I thought Forge was the opposite of backwards-compatibility

TheGlitch76 commented 4 years ago

Forge is more than just Lex and all of them do try to maintain back-compat for some things.