Hi Julian, Thanks for revising the submission. Changes look good, now merged and submitted to git master. I will now update the SO version number to reflect the new API, this will also enable any changes to the serializers to utilize this new SO version number.
Robert. On 11 June 2016 at 09:37, Julien Valentin <[email protected]> wrote: > Hi Robert > I haven't fully understood all you wrote in your last post so I reinjected my > changes in files from current master git without changing any indentation. > Is that what you want? > > (After few tests it appears the block l753 in ObjectWrapper.cpp > > Code: > std::string libName = std::string( name, 0, posDoubleColon ); > > ObjectWrapper* found=0; > std::string nodeKitLib = > osgDB::Registry::instance()->createLibraryNameForNodeKit(libName); > if ( > osgDB::Registry::instance()->loadLibrary(nodeKitLib)==osgDB::Registry::LOADED > ) > found= findWrapper(name); > > std::string pluginLib = > osgDB::Registry::instance()->createLibraryNameForExtension(std::string("serializers_")+libName); > if ( > osgDB::Registry::instance()->loadLibrary(pluginLib)==osgDB::Registry::LOADED ) > found= findWrapper(name); > > pluginLib = > osgDB::Registry::instance()->createLibraryNameForExtension(libName); > if ( > osgDB::Registry::instance()->loadLibrary(pluginLib)==osgDB::Registry::LOADED ) > found= findWrapper(name); > > if(found)found->setupAssociatesRevisionsInheritanceIfRequired(); > return found; > > > seams to be called multiples times -even when plugin already loaded?!- so I > re-added the bool flag I removed before > (_isAssociatesRevisionsInheritanceDone) > > > > robertosfield wrote: >> Hi Julian, >> >> I am currently reviewing the osgDB changes. You've made lots of >> improvements to the indentation of the files besides the ones just for >> the associates - while I like these as they fix lots of poor formating >> that was in the original code and out of step with the rest of the >> OSG, I don't like to have commits that tackle unrelated issues. I >> like to keep a commit focused on just what it needs to so that it's >> easier to follow what was done for what reason and easier to cherry >> pick or revert individual sets of changes. >> >> I can try to merge these changes by hand, either the cosmetic changes >> first then the associate versioning or the other way around. Is there >> any change that you've made separate commits yourself so I can take >> these rather than attempt to split them myself? >> >> Robert. >> >> >> On 3 June 2016 at 13:15, Julien Valentin <> wrote: >> >> > There was mistakes in the submission >> > I test it using >> > { >> > UPDATE_TO_VERSION_SCOPED( 1430 ) >> > ADDED_ASSOCIATE("osg::Node") >> > } >> > in Drawable serializer >> > and it seams to work (not outputing node attributes for any subclasses) >> > >> > >> > >> > mp3butcher wrote: >> > >> > > Based on the observation that flagging associates should be repeated in >> > > all derived classes: >> > > In the use case of Drawable's "new" associate Node, it force to repeated >> > > the associate revision tag >> > > >> > > Code: >> > > { >> > > UPDATE_TO_VERSION_SCOPED( 143 ) >> > > ADDED_ASSOCIATE("osg::Node") >> > > } >> > > >> > > >> > > >> > > in all derived class serializer derivated from Drawable :Geometry, >> > > ShapeDrawable, RigGeometry ..and so on... >> > > (not very great) >> > > >> > > So, I added a mecanism (in ObjectWrapper and IN/OUTputStream cpps) in >> > > order to simulate inheritance of associate revisions tags. >> > > I can't find any use case that would obliterate my reasoning but I'm >> > > opened to critics >> > > >> > > >> > > >> > > mp3butcher wrote: >> > > >> > > > >> > > > >> > > > >> > > > mp3butcher wrote: >> > > > >> > > > > I add some debug output (perhaps they can be merged too) >> > > > > and tests I have done seams to work. >> > > > > >> > > > > >> > > > > robertosfield wrote: >> > > > > >> > > > > > Hi Julien, >> > > > > > >> > > > > > We are getting close :-) >> > > > > > >> > > > > > Still passing strings as strings rather than const string&, but I >> > > > > > can >> > > > > > changed that :-) >> > > > > > >> > > > > > I'm inclined to change the split function to directly add the >> > > > > > strings >> > > > > > into the associate list rather than put them in a StringList then >> > > > > > copy >> > > > > > it as this will be more efficient and require less code. Again >> > > > > > this is >> > > > > > small tweak that I can implement. >> > > > > > >> > > > > > Have you tested out the feature yet? >> > > > > > >> > > > > > Robert. >> > > > > > >> > > > > > >> > > > > > On 2 June 2016 at 12:45, Julien Valentin <> wrote: >> > > > > > >> > > > > > >> > > > > > > Here's the new submission >> > > > > > > Hope I didn't miss a thing >> > > > > > > >> > > > > > > >> > > > > > > robertosfield wrote: >> > > > > > > >> > > > > > > >> > > > > > > > Hi Julien, >> > > > > > > > >> > > > > > > > On 2 June 2016 at 11:46, Julien Valentin <> wrote: >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > > I may not be clear enough: >> > > > > > > > > If you change the inheritance of a class (such Drawable >> > > > > > > > > inheriting from Node) the base class serializer is already >> > > > > > > > > used by other (Node serializer is already used in Group >> > > > > > > > > etc...) so you don't want to tag the wrapper with a version >> > > > > > > > > (tagging Node serializer with newversion would make no sense >> > > > > > > > > as it's already present in previous version).So You need >> > > > > > > > > finer version tagging control... >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > Yes, now makes perfect sense. Given this constraint my >> > > > > > > > suggestion of >> > > > > > > > embedding the supported versions into the wrapper itself isn't >> > > > > > > > workable. >> > > > > > > > >> > > > > > > > You approach looks to be the best way forward. >> > > > > > > > >> > > > > > > > Your changes aren't ready to merge as is yet as the coding >> > > > > > > > style >> > > > > > > > (indents/spacing/placement of {), are at odds with the rest of >> > > > > > > > code >> > > > > > > > around them. If we merge changes that use different coding >> > > > > > > > styles the >> > > > > > > > OSG code base would end up a bit of a mess and less readable >> > > > > > > > and >> > > > > > > > maintainable for it. Could you adjust your code to fit in >> > > > > > > > better. >> > > > > > > > >> > > > > > > > Key things are: >> > > > > > > > >> > > > > > > > open { on a a separate line to if etc. statement. >> > > > > > > > four spaces for indentation within {} block >> > > > > > > > no spaces between object->method >> > > > > > > > passing objects by const& rather than as a straight objects. >> > > > > > > > i.e. >> > > > > > > > const std::string& rather than std::string. >> > > > > > > > >> > > > > > > > There will be little ones besides these, general rule If the >> > > > > > > > code >> > > > > > > > looks like it's written be different developers then it's a >> > > > > > > > something >> > > > > > > > that grates. >> > > > > > > > >> > > > > > > > Cheers, >> > > > > > > > Robert. >> > > > > > > > _______________________________________________ >> > > > > > > > osg-submissions mailing list >> > > > > > > > >> > > > > > > > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org >> > > > > > > > >> > > > > > > > ------------------ >> > > > > > > > Post generated by Mail2Forum >> > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > ------------------ >> > > > > > > Read this topic online here: >> > > > > > > http://forum.openscenegraph.org/viewtopic.php?p=67346#67346 >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > _______________________________________________ >> > > > > > > osg-submissions mailing list >> > > > > > > >> > > > > > > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > _______________________________________________ >> > > > > > osg-submissions mailing list >> > > > > > >> > > > > > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org >> > > > > > >> > > > > > ------------------ >> > > > > > Post generated by Mail2Forum >> > > > > > >> > > > > >> > > > > >> > > > >> > > > >> > > >> > > Code: >> > > >> > > >> > > >> > > >> > >> > >> > ------------------ >> > Read this topic online here: >> > http://forum.openscenegraph.org/viewtopic.php?p=67386#67386 >> > >> > >> > >> > >> > _______________________________________________ >> > osg-submissions mailing list >> > >> > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org >> > >> > >> _______________________________________________ >> osg-submissions mailing list >> >> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org >> >> ------------------ >> Post generated by Mail2Forum > > > ------------------ > Read this topic online here: > http://forum.openscenegraph.org/viewtopic.php?p=67572#67572 > > > > > _______________________________________________ > osg-submissions mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org > _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
