Re: Here is the patch

[Jens Granseuer <jensgr@gmx.net>]

On 11.03.2004 05:08, Gil Pinheiro wrote:
> >1) with the changes with regards to campaigns, difficulty levels, etc.
> >currently discussed, much of the code (almost everything outside of
> >the ai* files themselves) would have to be reworked anyway. That's why
> >I think it is better to wait for those changes to happen. As a result
> >some of the interface adjustments you made will fit in even easier.  
>
> Things I think are in need of improvement:
>     game abstraction (rules, and the like should be encapsulated away 
> from the UI routines, so really new ideas can be tried).
>     event handling (discussed before)


I'll add composite widgets to the list.


>     Comet -- why the branch of players, etc?  This effectively doubles 
> my consistency checking. Maybe doing a subclass like you did for mission 
> would be better.


Unfortunately, I have noone to blame for this but myself. The main reason
for keeping things separate was that crimson and comet sometimes do very
different things with the data, so where crimson is happy with an array
comet might badly need a linked list or so. By now this turned out to
cause much more harm than good, so I agree that subclassing would have
been a saner path. (as a sidenote, mission is not a subclass, either.
You may be talking about cfed.)


> >2) I have a few (mostly minor) objections concerning various
> implementation
> >details. (The one that hit me first when trying to resolve the linking
> >error yesterday was the AI stuffed into the Player class and the circular
> >reference this resulted in. Something like that is usually a sign of
> >design errors. I know that I did the same in a few cases but that doesn't
> >mean new code is allowed to repeat that mistake ;-) In this case it looks
> >like it can easily be avoided.) If you like we can discuss the points in
> >detail but should probably take this off-list then.  
> >
> It is a pretty standard practice to forward declare classes, when 
> necessary.


"when necessary" is the important part here...


> The short version, is that I considered player to store data 
> on the AI, so that it can be read/written to file in a consistent manner 
> (no need for any conflict between the two player paradigm, and the idea 
> of a computer controller), so naturally player needs to know about AI. 
> The problem stems from the fact that the AI needs knowledge of the game 
> state, which is in mission.h (or game.h, really, but same loop). 
> mission.h includes player.h. Thus the loop. Each of the three require 
> knowledge of the other. Likely the forward declaration would have been 
> ignored if I had not commented it.


I understand and agree with the reasoning but the Player doesn't actually
need to know the AI classes. Game does, and it would suffice to just
include the AI id's in Player.


> I included a picture of crimson-0.4.1's include graph, so that you can 
> get an idea of hard it can be to add to the hierarchy without buggering 
> something - this was the clearest rendering I could make.


Goes a long way to show what I should do better when starting my next
project ;-)


> As to the other details, feel free to critique. I've been away from C++ 
> since the bad old watcom days, naturally I'm rusty on what is considered 
> correct these days.


I'm pretty sure I am not authoritative on what is considered correct these
days in C++. I can only speak for myself. Whether it makes sense to discuss
the details now depends on the approach you choose, I think. I assume
you'll be working on the AI parts, mainly, and the interface looks fine.
It's the integration of the new feature into the game that bothers me
at a few places, but as I said it's nothing major, so we could sort this
out later.


> >2.5) I'd like to have the mapper select the AI best suited for a
> scenario,
> >not the player. Thinking about it, the "null" AI might actually be quite
> >useful for the tutorial campaign.  
> >
> Basically, support for it, in principal, is already there.


I noticed. Using it is pretty straightforward.


> >In general, the idea is nice and I hope we can put it to good use.
> >Speaking of which, what is it specifically that you aim for in your AI
> >project?
>
> If I'm not getting back from the community interaction, it doesn't make 
> any sense to spend any more time chasing the CVS. I'd rather just spend 
> my time working on my branch.


That's understandable...


> I'm not ticked off or anything like that 
> (maybe a little disappointed) -I'll stick around on the list, but don't 
> count on my work anytime soon. Maybe after my deadline, I could publish 
> my paper on the list. Basically this will allow me to fix what I 
> consider necessary, and gives me the latitude to do strange things like 
> link with a scripting language, without worrying about a merge. 
> Effectively, I'm saying I'll worry about it later (i.e. not never, just 
> later).


... and one of the reasons I was asking about your goals was that I have
no idea what kind of community feedback you'd like to have. At this
point I don't know what you are trying to accomplish so it's difficult to
say whether we can provide some useful input, maybe even without merging
your code.


I don't know whether this general AI interface is vital to your project
or if it just helps to make testing etc. easier. I don't know the big
picture. I don't know where you are going, what you are expecting, or
what you were hoping for. You didn't really tell. I'm kind of clueless.


Jens