sideeffects / HoudiniEngineForUnreal

Houdini Engine Plugin for Unreal Engine.
http://www.sidefx.com/unreal
Other
1.36k stars 373 forks source link

Purpose of IHoudiniEngineEditor and IHoudiniEngine? #33

Closed JaapSuter closed 6 years ago

JaapSuter commented 8 years ago

These two interfaces comprise the current public interface of their respective modules, but the methods they provide are as far as I can tell only useful inside the Houdini plugin itself.

(...for additional thoughts on what I'd like to see in a useful public API, see this forum thread: Access from C++ (plugin API surface)).

Unless I'm overlooking their purpose, I suggest you make these methods no longer available publically.

I believe at one point, the Unreal build system insisted every module had at least one public file it. That's no longer the case. (somewhat similar to how the Classes folder is no longer necessary for public UObject implementations, they can live anywhere nowadays -- provided the folder's accessibility matches the object's external use, but alas, I digress).

It's totally fine to have module with only a single Private folder that contains all of its code.

Moreover, the editor module currently adds the runtime module's 'Private' header folder to it's own include path. Given the tight coupling between editor and runtime module in a typical plugin architecture, I think that's entirely reasonable.

Given all of this, I think you can get rid of the Public folders (and their contents) in both modules entirely. Case where the editor module uses IHoudiniEngine, it should just use FHoudiniEngine -- which it does in many other places already anyway.

That kills two public interfaces, two header files, and bunch of code.

Less code, less bugs... ;-)

Of course, my next issue is going to be about adding stuff to the public interface, hah!

ttvd commented 8 years ago

I think originally UE4 plugins required the presence of interface files (Class folder as well, but I believe this was removed around 4.6, or so). But we don't really use them, you are correct. I think we now put HAPI headers in the Engine public folder, but the other one can definitely go.