Hi Gerrit,

Gerrit Voss wrote:
some general comments ;-)

thanks for reviewing this.

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.

you are right, the generic interface can not reproduce it correctly; I changed it back to access='none'.

Similar the node children should be NULL checked as no NULL child
is allowed, and other parts of the code rely on this.

Changed as well. Can we consider moving to a generated Node class ?

I hope I got all the semantic side effects ;-)
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.

I removed that part.

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.

you mean why are there implementations of these functions ? That is a leftover I had forgotten to remove, when I added the callbacks for removeObject (my original idea was to implement removeObject always in terms of find and remove - but then decided against it as it would introduce a case where the handle deviates from the container).

Thanks for catching these mistakes. A new patch [1] is attached, which I plan to commit in a few days, just yell if I should hold on ;)

        Thanks,
                Carsten

[1] it's rebased against r1305, I hope it applies cleanly

Attachment: generic_interface-2008-06-16.diff.gz
Description: GNU Zip compressed data

-------------------------------------------------------------------------
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

Reply via email to