krpc / krpc-core

Other
3 stars 1 forks source link

Add proper inheritance support for remote objects in clients #8

Open Genhis opened 4 years ago

Genhis commented 4 years ago

In my service, I have classes which share common functionality, so I set up an inheritance hierarchy. However, this hierarchy is not preserved when I generate client code. Instead, the common functionality is duplicated for each derived class annotated with KRPCClass which prevents me from using benefits of inheritance on the client side, such as creating a list of objects with the same base class. Although not all languages may support inheritance, it would be nice to have it at least in languages which support it.

After doing some research, I believe the following parts would need changing to support this feature:

To conclude my examples below, if a base class have KRPCClass annotation, all classes derived from it should be exported and create a correct hierarchy. The derived classes do not need to have KRPCClass annotation. This is just my opinion on how I would use the API, and I welcome further discussion.

I would love to help with the implementation but currently I am having issues with making this project compile on Windows. I think I am missing some dependencies but I don't know which ones exactly.


Examples:

// This class is internal and should not be exported to the client
// It should not contain any KRPC properties/methods, although the current implementation allows it
public class InternalBase {
    public void InternalMethod() {}
}

[KRPCClass(Service = "MyService")]
public class Base : InternalBase {
    [KRPCMethod]
    public void NormalMethod() {}
}

[KRPCClass(Service = "MyService")] // Is this necessary if parent is annotated?
public abstract class Base2 : Base {
    [KRPCMethod]
    public abstract void AbstractMethod();
}

// It is not necessary to export this class because it doesn't have any KRPC properties/methods
// but there is nothing wrong with doing so
public abstract class Base3 : Base2 {
    public overrite void NormalMethod() { ProtectedAbstractMethod(); }
    protected abstract void ProtectedAbstractMethod();
}

[KRPCClass(Service = "MyService")] // Is this necessary if parent is annotated?
public class Derived : Base3 {
    // No need to have annotation here, the abstract method is annotated.
    public void AbstractMethod() {}

    public void ProtectedAbstractMethod() {}
}

How I would use it in Java:

List<Base> list = new ArrayList<>();
list.add(service.getBase()); // Base object
list.add(service.getDerived()); // Derived object

Base2 base = service.getDerived(); // Derived object
base.abstractMethod(); // run Derived implementation
djungelorm commented 1 year ago

Duplicate of krpc/krpc#63

djungelorm commented 1 year ago

I agree this would be a good addition. I will draw up a design for this.

djungelorm commented 1 year ago

if a base class have KRPCClass annotation, all classes derived from it should be exported and create a correct hierarchy.

I think we should require explicit KRPCClass annotations on all classes that you want to be exposed to clients. That way you have complete control over what's exposed. For example, you might have base class A, and classes B, C and D that derive from it. You might want to expose A, B and C to the client but not D.

What we need to implement is:

Open questions:

Genhis commented 1 year ago

It's been a long time since I wrote this request, so I don't remember all details about how I imagined certain things to function.

djungelorm commented 1 year ago

I agree we should just try and follow C# rules. I think this will be fine for most clients that support OOP.

The interesting one would be the Cnano client. I think it'll be doable though. I'll have a go at a proof of concept implementation.