Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExpectPlatform classes #5

Open
shedaniel opened this issue May 11, 2021 · 7 comments
Open

ExpectPlatform classes #5

shedaniel opened this issue May 11, 2021 · 7 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@shedaniel
Copy link
Member

Accept classes in @ExpectPlatform, may be confusing.
Library Common:

@ExpectPlatform
public class Foo implements Supplier<Integer> {
    protected int a;
    
    public Foo(int a) {
        this.a = a;
    }
    
    @Override
    public Integer get() {
        return this.a;
    }
    
    // Does not need a @ExpectPlatform, just error
    public void set(int value) {
        throw new AssertionError();
    }
}

Library Fabric:

public class FooFabric extends Foo implements FabricOnlyRandomInterface {
    // Must include the same constructor as common, Foo#<init> is redirected to here
    public FooFabric(int a) {
        super(a);
    }
    
    @Override
    public Integer get() {
        return super.get() + 5; // Custom logic overriding the common one
    }
    
    @Override
    public void set(int value) {
        this.a = value - 5;
    }
    
    public void fabricOnlyMethod() {
        System.out.println("lol");
    }
}

Library Forge:

public class FooForge extends Foo implements ForgeOnlyRandomInterface {
    // Must include the same constructor as common, Foo#<init> is redirected to here
    public FooForge(int a) {
        super(a);
    }
    
    @Override
    public void set(int value) {
        this.a = value;
    }
}

Using the library in Common:

System.out.println(new Foo(10).get()); // Prints 15 on Fabric, 10 on Forge

// -> Compiles to
System.out.println(new FooFabric(10).get());
System.out.println(new FooForge(10).get());

Using the library in Fabric / Forge:

System.out.println(new Foo(10).get()); // This will crash
System.out.println(new FooFabric(10).get()); // Code have to explicitly call FooFabric#<init>
System.out.println(new FooForge(10).get()); // Code have to explicitly call FooForge#<init>
@shedaniel shedaniel added enhancement New feature or request help wanted Extra attention is needed labels May 11, 2021
@Juuxel
Copy link
Member

Juuxel commented May 11, 2021

How would this work for libraries that are not compiled using architectury? Would they have to use the platform implementations from impl.fabric/forge instead of the common API?

@KP2048
Copy link

KP2048 commented Jan 20, 2022

Expect platform fields would be useful too

@KP2048
Copy link

KP2048 commented Feb 28, 2022

An annotation for excluding or including a method in common from being an expect platform method in the class?

@KP2048
Copy link

KP2048 commented Feb 28, 2022

Even better, the implementation classes could be subclasses of the common class and the overridden methods in the implementation classes could just be the implementation methods so all you would have to do is replace the object creation statements with the appropriate implementation class. The inheritance stuff would take care of the methods. You would also need to enforce somehow that the common class is used in common module and the implementation class is used in implementation module (the common still gets replaced with the implementation in common module)

@wagyourtail
Copy link

@Witherking25

as long as common gets replaced in implementation module, everything would work with that as common can't see the impl...

tho, I just use ServiceLoader at this point...

@KP2048
Copy link

KP2048 commented Mar 1, 2022

How would this work for libraries that are not compiled using architectury? Would they have to use the platform implementations from impl.fabric/forge instead of the common API?

as long as common gets replaced in implementation module, everything would work with that as common can't see the impl...

If you use common class in a fabric only mod it won’t work same thing for forge only mod
You must use the platform specific class

@KP2048
Copy link

KP2048 commented Mar 1, 2022

Would this work?

public ClassNode doEdit(String name, ClassNode node) {
if (!RemapInjectables.isInjectInjectables()) return node;
for (MethodNode method : node.methods) {
for (ListIterator<AbstractInsnNode> iterator = method.instructions.iterator(); iterator.hasNext(); ) {
AbstractInsnNode instruction = iterator.next();
if (instruction instanceof TypeInsnNode) {
TypeInsnNode typeInsn = (TypeInsnNode) instruction;
if (typeInsn.getOpcode() == Opcodes.NEW) {
String className = typeInsn.desc;
if (getClassNode(className).visibleAnnotations.stream().anyMatch(it -> Objects.equals(it.desc, RemapInjectables.EXPECT_PLATFORM) || Objects.equals(it.desc, RemapInjectables.EXPECT_PLATFORM_LEGACY) || Objects.equals(it.desc, RemapInjectables.EXPECT_PLATFORM_LEGACY2))) {
iterator.set(new TypeInsnNode(Opcodes.NEW, getPlatformClass(className)));
}
}
}
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants