Fabric API

Fabric API

106M Downloads

Tweak the screen registy's generics

liach opened this issue · 4 comments

commented

The screen registry's generics currently has a few ? extends T where T extends .... These double extends is unnecessary and breaks java's type inference.

I.e. basically change

public static <H extends ScreenHandler, S extends Screen & ScreenHandlerProvider<H>> void register(ScreenHandlerType<? extends H> type, Factory<? super H, ? extends S> screenFactory) {
// Convert our factory to the vanilla provider here as the vanilla interface won't be available to modders.
HandledScreens.<H, S>register(type, screenFactory::create);
}

to

public static <H extends ScreenHandler, S extends Screen & ScreenHandlerProvider<H>> void register(ScreenHandlerType<H> type, Factory<? super H, S> screenFactory) {
    // Convert our factory to the vanilla provider here as the vanilla interface won't be available to modders.
    HandledScreens.<H, S>register(type, screenFactory::create);
}
commented

So this is what causes the type inference issues... 🤦

I figured it was something with the wildcards, but didn't realise that there were redundant ones. I'll make a PR to fix this in a few days.

commented

In that example the double extends is unnecessary, but you can't remove them all. For example:

<T extends Something> List<T> something(List<? extends T> a, List<? extends T> b) can be called with different types for each (something(List<Something1>, List<Something2>)) which wouldn't work if the ? extends were removed.

In those cases, it should be changed to something like this: <T extends Something, T1 extends T, T2 extends T> List<T> something(List<T1> a, List<T2> b)

commented

This may have to be a 2.0.0 bump as removing the wildcards from the parameters breaks some code in the test mod:

Using a HandledScreen<T> for a ScreenHandlerType<U> where U extends T doesn't work anymore as that screen doesn't implement ScreenHandlerProvider<U>. In the test mod's case, T is Generic3x3ContainerScreenHandler and U is BagScreenHandler which extends the vanilla class.

commented

Actually, it seems that just removing the wildcards from the factory parameter (so that Factory<? super H, ? extends S> becomes Factory<H, S>) is enough and retains source compatibility.