search

enjarai / recursive-resources

Minecraft mod that enhances the Resource Pack menu with folder support.
Mozilla Public License 2.0
20 stars 8 forks source link

redirect instead of overwrite #11

Closed KingContaria closed 1 year ago

KingContaria commented 1 year ago

use a redirect to replace only the packscreen creation instead of the whole method

enjarai commented 1 year ago

Why is this change necessary?

The reason I used an overwrite is because I'm completely overwriting the initialization of the resource packs screen. This is inherently incompatible with any other mods attempting to do something similar, so I don't see how using a redirect would improve anything here.

KingContaria commented 1 year ago

Since you are only replacing the Screen, not the parameters, it makes sense to only replace the screen (which would still be incompatible if another mod tried to do it aswell) in my opinion.

lets say for example someone wanted to just change the title of the resourcepack screen with a @ModifyArg that would not be possible with the overwrite, however it would be when using a redirect, as the args are still created by the original Minecraft code and then just passed on to your redirect

KingContaria commented 1 year ago

idk maybe this is just personal preference if you want to keep it as is thats of course your decision

enjarai commented 1 year ago

Alright, I can see where you're coming from, and you're right that this could probably fix some rare situations.

However, if compatibility problems do become a concern, I'd rather revamp the way the custom screen is injected completely, and do it properly. Though as it stands currently, I haven't encountered any other mods doing something similar.

As this PR is mostly a bandaid fix for a currently nonexistent problem, I'm closing it.