Hi, some general comments ;-)
The State's 'none' access for the chunks was intentional because you can not replicate anything useful with the generic interface. E.g. if you do: state->genericAddChunk(texObj0); state->genericAddChunk(NULL); state->genericAddChunk(texObj2); this should result in a State with texture objects 0 and 2 set. Currently you will get 0 and 1 set. So writing and reading using the generic interface is not symmetrical. But as State is always a derived element I would not bother with the generic interface at all and just disable it, hence the none access. Similar the node children should be NULL checked as no NULL child is allowed, and other parts of the code rely on this. I hope I got all the semantic side effects ;-) On Wed, 2008-06-11 at 14:22 -0500, Carsten Neumann wrote: > Hi, > > Carsten Neumann wrote: > > Carsten Neumann wrote: > as mentioned earlier, here is a new patch. Some things were I'd be > especially interested in feedback are: > > - I had to disambiguate the generated remove/replace functions that take > an index from those that take a pointer [1]. I choose the following > replaceObjIn<FieldName> and removeObjFrom<FieldName> for the functions > that take a pointer. Adding a ByObj, ByObject or ByIndex suffix would be > alternatives, I don't have a strong opinion on what is nicer though ;) > > - for the same reason I had to change some .fcd files that already named > functions different from the default, here I used the ByObj suffix, > because that required less changes. I'm happy to improve consistency > here, provided we can decide on a naming scheme ;) > > - wrt the generated functions there are basically three pieces of > information that are relevant: > * is there a function that provides the operation ? > * is it generated in base or hand-written in the class ? > * what's its name ? > > Therefore the .fcd has now supports the following: > generatePushToField = "true" / "false > hasPushToField = "true" / "false" > pushToFieldAs = "myAddFunctionName" > > generate<funcname> implies has<funcname> > <funcname>As implies has<funcname> > > Of course the first two are only relevant for fields with > ptrFieldAccess="custom", otherwise the functions are generated either > with the given or default name. why is anything generated for the custom case ??. I would treat it all or nothing. If custom is selected all functions must be implemented inside the class and nothing remains in the base class. I don't see a reason to implement one half and have the other half generated. So I don't see the need for the generate part. The 'has' part if fine to select if the generic interface should support a particular function. Another thing, I don't quite understand the meaning of EditMFieldHandle<FieldContainerPtrMFieldBase>::removeObject EditMFieldHandle<FieldContainerPtrMFieldBase>::replaceObject As they are marked as pure virtual they can only be called through a derived object, but this does seem not happen. kind regards, gerrit ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ Opensg-core mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensg-core
