[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: Here is the patch





I think you're mixing a few things up here. The problems with AM_PATH_SDL
only occur when building from CVS. When using a distribution tarball
the macro is included in aclocal.m4.
Really, my bad. General protocol is to do a aclocal, then autoconf, then automake. By doing so I obliterate the PATH_SDL macro.
I would still recommend adding acinclude, though. Here is what I'm talking about:
I'm using the stock crimson-0.4.1.tar.gz up on the site, and have complied SDL with the default settings.

[root@superdell src]# cd crimson-0.4.1
[root@superdell crimson-0.4.1]# aclocal
aclocal: configure.ac: 41: macro `AM_PATH_SDL' not found in library
[root@superdell crimson-0.4.1]# cp ../SDL_ttf-2.0.6/acinclude.m4 ./
[root@superdell crimson-0.4.1]# aclocal
[root@superdell crimson-0.4.1]# autoconf
configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET
[root@superdell crimson-0.4.1]# automake --add-missing
configure.ac:42: warning: AC_ARG_PROGRAM was called before AC_CANONICAL_TARGET
configure.ac: installing `config/mkinstalldirs'
configure.ac:42: installing `config/config.guess'
configure.ac:42: installing `config/config.sub'
[root@superdell crimson-0.4.1]# ./configure && make && make install

finished cleanly

What for? In the tarballs you already have configure which in turn can
generate the Makefiles from the Makefile.ins. The purpose of autogen.sh
is to create configure and the Makefile.ins.
If changes are made to the source, it can be helpful. Not all developers immediately grab at the CVS archive. But I understand your reasoning, most will users will not want or require it. autogen.sh is just nicer to work with.
Enough said.

Yes, I know about the error. Actually I have patched my SDL to make it
go away. The sdl.m4 script contains a command that is completely
unnecessary IMO, and to get rid of the warning we'd have to distribute
two more auto* scripts (config.guess and another one which I don't
remember9 which we don't use and don't need. Therefore I'd rather live
with the warning than try to fix it. Maybe I should ask the SDL guys
about it...
Not an autoguy. I dislike the syntax, perl dependency, and general obtuseness. That is only my opinion, and I do use their facilities, but it doesn't mean I have to like it :). Enough said.

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

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

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.
my email is pinh1440@mach1.wlu.ca.

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 added AI specification to the map data files, all that is left to do is decide whether or not to allow the user to switch AIs, and stop the call to aiselector. Maybe the policy would necessarily include another gametype, campaign mode, where only the maps preferences are allowed (only way to get password to the next level?).

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

Windows bitmap