ofxAssimp - uniformity on addon name, include name

[dimitre]

Other addons usually have the library name, include name and object the same, like

ofxSvg
ofxSvg.h
ofxSvg svg;

It would be great if ofxAssimp could be at least the include with the same name of the library
now it is:

ofxAssimp
ofxAssimpModel.h
ofx::assimp::Model model;

[ofTheo]

Good point @dimitre - I wanted to bring this up too.

We were toying with the ofx::assimp::Model approach for addons, but in practice it makes using them less intuitive and I'd maybe lean towards keeping the old approach for now. 😬

cc @NickHardeman

[dimitre]

I think a rename for ofxAssimp.h and a typedef would be great

[NickHardeman]

@dimitre yes that makes sense!
After using ofx::assimp::Model, I think I would also prefer an older naming approach similar to ofxSvg, like ofxAssimp instead of ofx::assimp::Model or ofxAssimpModelLoader. Naming it ofxAssimp and not including "loader" or "Model" allows for expansion in the future, like saving; where ofxAssimpModelLoader with a save function gives conflicting messages.

That being said, a larger naming convention discussion is in order. :)

I recently ran into issues with the likes of ofx::svg::Ellipse; on Windows an Ellipse, Circle, Rectangle class caused issues, so you are forced to type ofx::svg::Ellipse; applying using namespace ofx::svg; would leave the ofx::svg::Ellipse class as Ellipse and caused conflicts with the Windows implementation. At which point it was much easier to just use ofxSvgEllipse.

Changing the filename now would break previous projects, but would be an easy fix.
Since it has already been released as ofx::assimp::Model, the typedef you mentioned could be a quick fix in the meantime.

[artificiel]

how about expanding the view on prior work with different addons, see how things fit as a high-level guideline for future addon work? there are 3 sub issues:

• .h naming; how about a "base package" ofxAssimp.h that would in turn include ofxAssimpLoader.h, allowing future assimp stuff (like ofxOsc does) while maintaining filename==classname. so you can include the whole, or choose just ofxOscSender. also the approach for some complex addons such as https://github.com/bakercp/ofxHTTP/blob/master/src/ofxHTTP.h

• namespace structure... looking at existing addons there are many variations taken, probably all of them haha, and it's evident there is gain to namespace stuff, but I wonder of the ofx:: layer has a use? (seems a bit administrative for no functional purpose). ofxAssimp::Loader is enough to put the Loader in its namespace. for instance ofxHap does that. and it removes a :: if you go full path. ofxSvg::Ellipse is not a big visual or finger-typing struggle vs ofxSvgEllipse.

• ofOldSchool: it is worth the trouble of implementing things in their own namespace, and as @dimitre suggests implementing ofx(::)Svg::Ellipse and typedef ofxSvgEllipse. this is the approach taken with the random distributions, so you can use of::random::poisson() or ofRandomPoisson(), in that case it's more about style than conflict, but it's good to support both styles, as they both have their reasons to be used, and as much as I prefer typing the :: the ofOldSchool usage won't be going away soon.

[NickHardeman]

The ofx:: layer does not seem to be beneficial in the case of addons. So I agree that it could be dropped / merged to ofxAssimp, etc..

The current name could be changed to ofxAssimp::Model. I realize it is currently ofx::assimp::Model, but this deviates from the previous approaches of ofXml, ofJson and ofxSvg which do not have a subclass. Should the user be interfacing with ofxAssimp or ofxAssimp::Model? If it is ofxAssimp as the main class, then the namespace ofxAssimp will have some clashes.

The ofxAssimp namespace would be in contrast to the current ofxSvg class. If the namespace were ofxSvg, the main interface class would need to be something like ofxSvg::Document. And then the ofxSvg.h would need to include the ofxSvgDocument.h file.
The ofxOsc, ofxOpenCv, ofxNetwork could take to this new naming convention easier since the classes have unique names from their addon name.

• ofOldSchool: I agree that the old school naming approach should be implemented in some manner.

Looking for a consensus on this so I can update ofxAssimp and ofxSvg with a cohesive naming convention for now and the future.
#8266

[artificiel]

you raise a good point in the differences between single-class and multiclass. the addons guideline could be "progressive" so not to encumber simple cases:

• type A: simple single-class addons: ofxThing directory contains ofxThing.h which defines ofxThing. mostly like now.

• type B: addons with multiple classes: ofxThing dir -> ofxThing.h -> #include ofxThingX.h, Y..., which defines ofxThing::X with using directives providing ofxThingX. everything is within ofxThing:: namespace. user's choice to include ofxThing.h or ofxThingX.h, and use ofxThingX vs ofxThing::X.

this brings back type A ofxThing in the global namespace. but i don't think we aim for purity here, but for a reasonably useable compromise with is consistent (and looking forward, the real "postmodern C++" approach will be to present addons as modules, which will solve the namespace problem as it is possible to load module ofxThing that defines and ofxThing class and expose the class itself, which means a simple name, yet with namespace protection (that is, when compilers/buildsystems/IDE/everyone is on the same page with modules support...)).

for current multi-class cases backward-fixing ofxOsc::Sender is not a usefull/required investment of energy (until perhaps some refactoring work is done for some other reasons with the class). but it should be the norm for new work.

[NickHardeman]

I keep going back and forth on ofxAssimp; should it be ofxAssimp as the main interface? or ofxAssimp::Scene?
ofxAssimp currently uses ofx::assimp::Model which loads an assimp scene, which seems confusing. ofxAssimp::Scene is more appropriately named in relation to its functionality with Assimp's aiScene*.
ofxAssimp is a clear entry point for the addon, but muddies the namespace convention. ofxAssimp::Scene is more robust but is less obvious that is the main class to use.
Currently leaning towards class ofxAssimp::Scene and ofxAssimp.h as an import due to the number of files in ofxAssimp.

The old school approach would account for all of these naming discrepancies other than being verbose. :)

Or rename the addon to ofxAssimpScene /

[NickHardeman]

Submitted the following PR #8367

[ofTheo]

closed by #8367