Dalamud icon indicating copy to clipboard operation
Dalamud copied to clipboard

Add interface support to dalamud/plugin services

Open kalilistic opened this issue 2 years ago • 4 comments

Adding interface support to Service<T>, starting with dalamud services and then plugin services.

For example, you have ICondition that defines the API, and Condition that implements it. Plugins can request ICondition instead, which will be satisfied by Condition, but can also be satisfied by e.g. MockCondition.


I'm taking a stab at this and will share a draft PR for feedback.

kalilistic avatar Apr 21 '22 01:04 kalilistic

I'm stealing this now, plfti

karashiiro avatar May 14 '22 01:05 karashiiro

I've been busy with other things, but I'm still working on this. I'm probably going to implement this in phases to avoid breaking changes until we can actually plan an overhaul. The most significant issues are:

Many plugin services have their public interfaces coupled with types such as SeString, and these aren't dependencies that can be easily isolated without a huge refactor and causing breaking changes. In short: we aren't moving these to a separate assembly anytime soon.

Many plugin services have delegate types declared inside of their concrete types, so extracting an interface without creating breaking changes requires a circular reference, which leads back to the first problem.

Many plugin services would benefit from having their interfaces split into multiple small interfaces with well-defined purposes. UiBuilder is one example of this: I think it should have its interface split into several to handle image loading, font loading, etc. This would be easily-solvable if we could register several interfaces in the service container for a single concrete type, except...

The Service<T> class is a static class with one generic type argument. Adding interface support to it requires a second generic type argument with a constraint where T : TInterface. Because you cannot narrow generic constraints of generics that have already been defined in an outer context, we need to add a second class Service<TInterface, TImplementation> to allow for that constraint. We would need to add a third etc. class to handle allowing two interfaces to be registered for a type, and more for greater numbers of interfaces. This is a problem with the static Service class, and not with the service container inside of it, which can be extended much more easily.

For the time being, I'm just adding a single Service<TInterface, TImplementation> and extracting interfaces. We can progressively clean things up from there until we get to the point where we need breaking changes to go any further. At that point, we'll probably go into DIP territory (if we're not already there?).

karashiiro avatar May 17 '22 19:05 karashiiro

Any update? The window system and imgui helpers needs interfaces too.

kalilistic avatar Oct 31 '22 00:10 kalilistic

No updates, I'm willing to toss the potato back to you.

karashiiro avatar Oct 31 '22 01:10 karashiiro