I am continuing to speak to my rubber ducky and wanted to share what I think might be an unintentional oversight in the TOnlineAsyncOp API.
After thinking about all the problems I raised above, it occoured to me that perhaps all of the “Common” APIs like FOnlineServicesCommon, FAuthCommon, etc… are all technically optional. The only code that must be implemented are the actual interfaces themselves so IOnlineServices, IAuth, etc.
Glancing at the code it seems feasible, so I started down that route and while it is more boilerplate for me to write, I can implement my service components registration just how I want them, totally data driven and I have a lot more control over how things are handled which works for me.
This almost works perfectly but there is one fly in the ointment, TOnlineAsyncOp. It takes a reference to FOnlineServicesCommon as a construction parameter. After looking at the code for that class, I don’t think this hard reference is really needed, let’s take a look at where it is used.
- TOnlineAsyncOp(FOnlineServicesCommon& InServices, ParamsType&& Params)
- The constructor stores the reference to FOnlineServicesCommon but I see no reason why this couldn’t be a IOnlineServicesPtr instead.
- FOnlineServicesCommon& GetServices() { return Services; }
- This function does not appear to be called at all, I tried changing the name of it and no compiler errors for a missing function were raised so, if it’s not used now, surely it could either be deleted or changed to IOnlineServicesPtr instead.
- DECLARE_MULTICAST_DELEGATE_FourParams(FOnOnlineAsyncOpCompleted, const FString& OpName, const FOnlineServicesCommon& OnlineServicesCommon, const UE::Online::FOnlineError& OnlineError, double DurationInSeconds)
- As far as I can tell again, this function isn’t used directly anywhere but again, couldn’t it use IOnlineServicesPtr instead?
It really feels like the inclusion of FOnlineServicesCommon in TOnlineAsyncOp is a mistake here as it breaks the implementation agnosticism of all the online interfaces and forces developers to derive their classes from FOnlineServicesCommon which feels unnecessarily restrictive.
If you could confirm the following as true then I can probably mark this whole question as answered.
- The “Common” (e.g. FOnlineServicesCommon, FAuthCommon, etc) APIs are optional.
- The hard referencing of FOnlineServicesCommon in TOnlineAsyncOp should either be deleted or should be changed to IOnlineServicesPtr instead.
- If so, could you let me know what the preference is so I can implement this in our version of the Engine.
Cheers!