Conversation
|
Not exactly a fan of the prefix behavior with I honestly also don't think we need the string substitution here, let's keep it simple and have IPC names be somewhat compile-time constant. |
|
Done on both counts. Now it's required and will throw if not set when you override. I was originally gonna argue for |
To clarify here, [IpcPrefix("Commands")]
public class IpcCommands {
[Ipc] public void DoFoo(); // Resolved: MyPlugin.Commands.DoFoo
[Ipc("Bar")] public void DoBar(); // Resolved: MyPlugin.Commands.Bar
[Ipc("Nya.Baz")]
public void DoBaz(); // Resolved: MyPlugin.Commands.Nya.Baz
[Ipc("Qux", applyPrefix: false)]
public void DoQux(); // Resolved: Qux
[Ipc("MyPlugin.Corge", applyPrefix: false)]
public void DoCorge(); // Resolved: MyPlugin.Corge
}If so, I'd suggest making that still valid if possible since it brings in the principle of least surprise.
There's also the following form, which I think explains better: public class Utilities {
[Ipc($"{nameof(Utilities)}.Test", applyPrefix: false)]
public void DoTest(); // Resolved: Utilities.Test |
Yes since you don't set applyPrefix and that's what I thought you meant.
Sure.
True, I just like the brevity/lack of needing to repeating |
|
Pushing to @goaaats for feature approval, I think this API surface looks good as-is. |
Goal of this was to emulate EzIpc from ECommons, but in a better way and native to dalamud so one wouldn't need that dependency. The way that (ec)/this works is providing convenience for ipc handling by offloading most of it onto the lib/dalamud.
This is what a lot of my ipc files look(ed) like normally
It's a bit verbose (IMO) with the initialisation and due to a c# limitation, having a helper function is almost required since <T1, T2> can't be named on the subscriber. It's also helpful when you want to safely invoke the ipc. This system would reduce the same thing to the following:
and basically the same for providers
The names are all reflected unless you want to pass/override them manually. The instances are also all stored in the interface and are auto disposed on plugin unload so one only has to register it unless they need to dispose sometime before the plugin lifecycle ends. This of course means you'd have to follow the same names as the plugin you're subscribing to to utilise the reflection, but if you don't, you can override it, like:
If you wanna disable the ipcs outside of plugin disposal, you can if you keep track of the
IpcRegistrations. Otherwise, you don't need to and can let dalamud call it when it disposes of your plugin normallyAI disclosure: It generated the docs (I'm not particularly good at those), though I went back and edited some as needed and checked they sounded fine. I also used it for quick making the additional generics, e.g. made IpcAction, it made IpcAction<T1>, IpcAction<T1, T2> ... since that was just copying boilerplate.
Pending a "LGTM" review I can go back and hand write all the docs. I just kind of don't expect much out of this PR.