Railcraft

Railcraft

34M Downloads

Reflection Cleanup

Speiger opened this issue ยท 13 comments

commented

Since i did use Railcraft in my testpack with ic2c i noticed a bug with optifine.
And that got fixed from optifine but it left a question behind why did that happen.
By asking around people said me what Optifine changed. They moved a field from the top to the bottom.

My request @CovertJaguar is that you look through your code and check every Reflection call you make and look if it is not needed. If a field that you access is private/nonePublic then i would like you to check if it is still accessable in some way of form. Because i know only extreme rare cases where you would need reflection. And nothing gets covered by Railcraft if i remember correctly.
I do not care on which milestone you put it but please make that check.
(Mod Intigration only if there is a case matching here)

commented

For #924, it is a forge bug in its nature. For other cases such furnace cart fuels, solutions may be opening pull requests to Forge to add Access Transformer entries.

commented

@liach thats why i do not use forge to generate models. xD Even if i use a NoneJson system i dont use forges stuff at all since it comes sooner or later to Minecrafts IBakedModels... And besides helper tools is forge not needed to make that stuff work...

commented

The only case where it would not be necessary are some cases that could be replaced with Access Transformers. But since I don't really understand how to set that up I'll be sticking with reflection unless someone wants to PR the Access Transformer framework into the mod/gradle.

commented

@Speiger you mean as opposed to vanilla models?

The reason why is because the vanilla format doesn't support texture remapping.

commented

@CovertJaguar remapping? You mean Texture Copying?
If you mean that. I use only vanilla models (no forge at all) and CF + Obscurator works 100% fine.
And anything IC2 had in 1.4.7 and more works 100% fine.
Even cables work fine even so i had to trick a bit. But that has something to do that i have a JsonDestroyer (code generated models)

commented

I mean being able to define textures in the BlockState JSON instead of the Model JSON. This greatly reduces the number of files needed.

And not having to define Item JSONs for blocks because Forge does that for me.

commented

@CovertJaguar An access transformer is really like a coremod. Forge allows you to put "FML_AT" attribute in jar to specify the file for access transformer to read.

commented

@CovertJaguar @liach the model bug i saw was because of using forge models...
To be honest i dont understand why you use those in the first place...
But everyone does what he likes

commented

yeah that is an upside. Better would be if you dont have to do the json stuff at all (which is possible)

commented

No, that is NOT better. What the hell makes you think that? You're one of those people that refuse to move to the new system? I'd rather not base my entire render base on unsupported hacks thank you.

commented

First of all its not a hack. Second yeah its a none supporting system on the other hand JsonFiles can not control boundingboxes of any kind or effect the effects of these blocks when they have a colision effect so its not a bad move to do that.
On top of that this system saves a lot of ram on my side and what you do in this system just generate the models via code and inject them in a Event which gets thrown by the model baker.
Note on that one: Even Minecraft has Some JsonFiles which are generated in that case. These are called buildIn

commented

Non-supported system == hack.

And why the hell would you want JSON files to control bounding boxes? That has always been handled by the Block class.

commented

FYI see #1418 for access transformers. They are added to 1.12.