Re: [simspark-devel] integrated_agent_v3

[Markus R. <rol...@un...>]

Hi,

Ben schrieb:
> I have make a update of integrated_agent[1], please have a look, 
> thank you!

thanks Ben for your work. I looked at the source code and it's getting 
better ;-)

I have some comments:

Concerning the base behavior class.

- I wonder if it's better to give the prdicate list directly to the 
think method of the behavior class. In the current implementation the 
predicates are collected, then converted into a string. The behavior 
class then parses the string back into predicates. That's much overhead.

The TrainControl::EndCycle() method get's the senselist from the 
agentaspect. This could be passed on to the behavior class directly.

- Some technical issues. If you hold a reference to another node in the 
system please use the CachedPath template together with the 
RegisterCachedPath() method (for the mTrainControl reference). Same for 
the mTrainControl reference in the TrainControl class.

- I think we should change the behavior class to cach the command string 
it sends. I.e. move the mCmdMsg member and the SetCmdMsg() member to the 
base class and change the think(...) to return void. In this way it is 
possible to use the ruby interface for behavior classes that are not 
derived from the soccerbehavior, i.e make it available to other simulations.

Concerning the base behavior class.

- You are using cerrs and stdouts to give agent responses instead of the 
Logserver (i.e. GetLog()->Normal() << "msg"). Maybe we should introduce 
a new log channel for agent responses?

- don't run the debug.rb script from SoccerbotBehavior. This should only 
be done from classes that derive from SoccerbotBehavior. I.e. it is 
specific to your agent implementation.

Concerning the TrainControl class

- I think we could use a std::set<shared_ptr<Client> TClientSet instead 
of the TClientList. This makes searching for a client instance easier 
(operator == and < are defined for shared_ptr) and makes sure that each 
behavior instance is registered only once.

- I think we should rename the mClientSize member that reflects its 
purpose, maybe to mNextClientId?

> I have a question: can we make the library zeitgeist, kerosin, oxygen
> as dll in windows?

I don't think it is possible without some changes. The problem is that 
the different libraries are dependent on each other and access 
implementation details from each other. However on windows only symbols 
(i.e. function names and members) that are explicitly declared are 
exported (i.e. visible from outside the .dll). So declaring all required 
symbols as DLL-Export is quite a lot of work.

In contrast Linux and MacOs work the other way round. By default _all_ 
symbols are visible. On these platforms it's therefore no problem to 
load our libraries dynamically.

cheers,
Markus

> [1] www.apollo3d.cn/download/integrated_agent_v3.tar.gz