From: <ah...@un...> - 2010-01-29 04:14:47
|
Hi, First of all, thanks for the suggestions. 1) I admit that the paper is currently misleading about these "invisible" implementation objects. The only problem that was directly caused by this was that storing variables inside the implementation objects created the possibility of unwillingly using a different object where that variable is still zero or a null pointer. This has been completely ruled out by not storing anything inside the implementations. The problem with the Joint classes is caused by a pretty tricky design, which defines two pure virtual methods, SetParameter and GetParameter. It implements a load (around 20) other methods that call SetParam and GetParam with ODE-specific constants. Subclasses of Joint then inherit the implemented methods and overwrite SetParam and GetParam so it will work. However, since the methods that call GetParam and SetParam use engine-specific constants, I have to implement them in ODEJoint. But I don't know in ODEJoint whether I will have to call ODEBallJoint::SetParam or ODEHingeJoint::SetParam etc. Try looking at joint.cpp, hingejoint.cpp and odejoint.cpp to see what is going on, I admit that this is VERY hard to explain. I thought it was impossible to solve this in any other way than the one I use currently, but now that I think about it, deriving ODEBallJoint from Joint instead of ODEJoint (which I think is what you suggested) might actually solve the problem. I'll have to test if this has any side effects, but I'll definitely try it out. 2) That's some great input. Maybe I can get rid of the StaticPhysicsMethods class completely by declaring the pointers to the implementation objects as static, which I should do anyway now that I think about it. Then, I can use these within the static methods and implement these with the bridge pattern as usual. I'll try this out. 3) So, if I understand correctly, you want me to add the ImpFactory into the scenegraph and then retrieve the parent ImpFactory node of a physics object instead of using a GetInstance method? Joschka has already said that with the Factory I made this week, it should be possible to write a ruby script that allows choosing the engine at runtime. I have too little knowledge of both zeitgeist and ruby to judge the likeliness of that possibility, though. However, Joschka and I planned to look at this anyway, so I'll keep this at the back of my head. And yeah, I isolated the engine-specific code pretty well at this point. thanks again, Andreas > Thanks a lot for all your efforts in developing the APL. > I have a few comments regarding the paper and also your recent commits. > 1. (This might be out dated with regard to the recent commits) About the > Implementation classes: In the paper, you've said that some classes have > 2 instances of their parent implementation class (e.g. SphereColliderImp > has two instances of ColliderImp class): one of them through direct > inheritance and another one inherited from the Collider class. My > suggestion is to eliminate the inherited one and use the inherited > Collider instance instead (as it functions as a gateway for the internal > Implementation class) or get the Implementation object of the Collider > class if the implementation object itself is needed and the > functionality is not provided through the Collider class (I might have > missed some points as I'm not aware of the code very much). > It seems that you've changed the design and now there is only one object > of each Imp. class globally. If it is possible (so the Imp. classes > probably doesn't have any variables), so this is a good solution too. :P > And their case will be similar to the case of static functions > (explained below). > > 2. The Static Functions: if I've understood correctly, there are a set > of static methods with engine specific implementation. Instead of having > a class with different implementations based on the selected physics > engine, my proposal is to either use a single object of an engine > specific implementation and call its member functions (the object > reference can be obtained from either from the Zeitgeist framework or > having a static GetSingleton() function in the StaticMethods class), or > by using the bridge pattern for the StaticMethods class too: this class > could have a static private refrence to an implmentation class which is > used by the static functions. > > 3. In your recent commit, you've introduced a factory class which > currently contains the instances of Imp. classes. I've two comments > about it: first, the same pattern have been used throughout the code but > with a slightly different style: references are stored in the zeitgeist > hierarchy and accessed using its methods. You might use the same style > for more consistency (or might not!). > Second, the instance of the factory class itself is usually obtained > using Zeitgeist framework too, instead of calling a static method like > TheFactory::GetInstance(). Beside from consistency point of view, this > style provides the flexibility to create the desired object in .rb > scripts (the actual engine is selected there). (Notice that I've NOT > checked to see where you create the actual factory instance, so your > solution might be as flexible as this one!). > Also you could use this factory to create any engine specific objects > and not only storing single instances (but apparently you do not have > any such engine specific classes and all of them are single instance > classes?!). > > Finally, I feel that you are reaching a point where you can completely > detach the ODE specific code into a separate plugin rather than being in > the Oxygen library itself. This would be nice! :) > > Thanks a lot, > Hedayat > > > >> thanks, >> >> Andreas >> >> >> ------------------------------------------------------------------------------ >> Throughout its 18-year history, RSA Conference consistently attracts the >> world's best and brightest in the field, creating opportunities for >> Conference >> attendees to learn about information security's most important issues >> through >> interactions with peers, luminaries and emerging and established >> companies. >> http://p.sf.net/sfu/rsaconf-dev2dev >> _______________________________________________ >> Simspark Generic Physical MAS Simulator >> simspark-devel mailing list >> sim...@li... >> https://lists.sourceforge.net/lists/listinfo/simspark-devel >> > > |