PDA

View Full Version : Angband code for dummies?


Timo Pietilš
July 18, 2010, 11:02
I used to be able to interpreter vanilla angband code and what does what, but currently I find it more a uncommented spaghetti code that is impossible to follow. Is there any Angband code for dummies around? Some sort of flowchart that tells which function is doing what? Which files have which purpose? Anything?

konijn_
July 18, 2010, 14:50
I used to be able to interpreter vanilla angband code and what does what, but currently I find it more a uncommented spaghetti code that is impossible to follow. Is there any Angband code for dummies around? Some sort of flowchart that tells which function is doing what? Which files have which purpose? Anything?

Ew, can you point us to some places that you found not documented enough to or too spaghetti-like ?

T.

Timo Pietilš
July 18, 2010, 14:57
Ew, can you point us to some places that you found not documented enough to or too spaghetti-like ?

T.

Everywhere. File relations, function relations etc. For example I need to look inside three different files to figure out simple damage calculation for manastorm: what exactly does it do, and I'm not sure that is even enough. Nobody can figure out that without some help or memorizing out every single file what they contain.

Derakon
July 18, 2010, 17:10
Hm, I'm generally able to find what I need, by searching for a string I know occurs in-game, and then working back from there. For example, if I want to look up shards damage, I would search for "You are hit by something sharp!" (the message you get when hit by shards while blind), which would get me to the code that processes shards hits, and I can search for what calls that function, and find one for breathing which refers to BR_SHAR_MAX, which I can search for to find that the damage cap for shards is 500.

Ultimately it's mostly a matter of being good at using search utilities. I will freely admit that Angband would be easier for me to read if it had a more object-oriented design, mostly because that's the kind of design I'm used to dealing with. But we'd need a full-on recoding effort to make that happen. I'd recommend just learning how to navigate unfamiliar code that isn't laid out the way you expect. If you're expecting to program as part of your career, it's a skill that will hold you in good stead.

Timo Pietilš
July 18, 2010, 17:18
I'd recommend just learning how to navigate unfamiliar code that isn't laid out the way you expect. If you're expecting to program as part of your career, it's a skill that will hold you in good stead.

A flowchart of somekind would help as well as documentation what is the purpose of each file.

I already have IT career (have been over a decade now), but all I need is some scripting skills which I already have.

Magnate
July 18, 2010, 20:25
A flowchart of somekind would help as well as documentation what is the purpose of each file.

I already have IT career (have been over a decade now), but all I need is some scripting skills which I already have.It's not that difficult. Start with main.c, find the function called "main". See that it does init_stuff, makes paths, processes command line arguments, sorts out gfx and sound, then calls play_game. Now, at this point it's not clear which file play_game is in - this is a fair cop. I use a grep alias which searches recursively through src/, so "grip play_game *" tells me it's in dungeon.c (if you're a programmer then your favourite IDE will do this much more easily than using grep).

So you open up dungeon.c and find play_game, and you notice that everything else follows from there. In general, the z-* files are the really low level things - messing with numbers, strings and files. The subdirs player/, object/ and monster/ are pretty self-explanatory. I guess the tricky stuff is in the cmdX.c files, which take a bit of getting used to.

Derakon
July 18, 2010, 20:53
(if you're a programmer then your favourite IDE will do this much more easily than using grep).
My favorite IDE is grep. ¨.¨

Timo Pietilš
July 18, 2010, 22:40
It's not that difficult. Start with main.c ... it's not clear which file play_game is in - this is a fair cop. I use a grep alias ... open up dungeon.c and find play_game, and you notice that everything else follows from there

In other words in order to find something you need to memorize entire code or if not that do multiple searches for every single function in order to figure out what they do.

A spaghetti code. A very hard to figure out spaghetti code.

Except maybe if you are a professional coder.

Ycombinator
July 18, 2010, 23:03
IMO Angband is in pretty much good shape for a 20-year old C project maintained by a team of volunteers.
I think the biggest source of complexity isn't the hairy call graph (grep and ctags really rock at finding what is where), but lots of global state and inherent algorithmic complexity of some parts of the code.

Pete Mack
July 19, 2010, 01:01
Timo, I think you are wrong on this.
It used to be, the damage calculation was mixed up in the code, and if you changed how (say) darkness worked, you would have to change code in two or three places.
Now it's all in a single .h file.
It could be better (and is, in NPP and Un). But it's much better than it was.

My preference would be to have spell and breath damage in a single .txt (or .xml:)) file.

The recent complaint about poison damage is an exception to the rule: the .H file was wrong. The complaint about that was well taken.

But the code is certainly less spaghetti-like than it was in 3.0.6

nppangband
July 19, 2010, 13:02
In my humble opinion, the comments in ui-menu.c need to go in much greater detail about how it all works. Although the current Angband code is cleaner, more efficient, and on the whole impressively written, the nice thing about the Ben Harrison code was that virtually every function in the codebase was commented so well that soembody with almost no coding experience could look at it and understand how it worked without having to decipher thousands of line of code.

konijn_
July 19, 2010, 13:23
In my humble opinion, the comments in ui-menu.c need to go in much greater detail about how it all works. Although the current Angband code is cleaner, more efficient, and on the whole impressively written, the nice thing about the Ben Harrison code was that virtually every function in the codebase was commented so well that soembody with almost no coding experience could look at it and understand how it worked without having to decipher thousands of line of code.

Yeah, we used to make fun of that one line of comment per line of code, but it really helps newbies apparently.

T.

Nick
July 19, 2010, 13:57
Yeah, we used to make fun of that one line of comment per line of code, but it really helps newbies apparently.


Speaking as a newbie, hell yeah.

Timo Pietilš
July 19, 2010, 20:07
In my humble opinion, the comments in ui-menu.c need to go in much greater detail about how it all works. Although the current Angband code is cleaner, more efficient, and on the whole impressively written, the nice thing about the Ben Harrison code was that virtually every function in the codebase was commented so well that soembody with almost no coding experience could look at it and understand how it worked without having to decipher thousands of line of code.

Exactly my point. I used to be able to tell what things do. Now I usually have no clue about them, and don't even know what to look for.

Magnate
July 19, 2010, 22:18
Timo, I think you are wrong on this.
It used to be, the damage calculation was mixed up in the code, and if you changed how (say) darkness worked, you would have to change code in two or three places.
Now it's all in a single .h file.
It could be better (and is, in NPP and Un). But it's much better than it was.Praise indeed - thanks Pete!

Timo - I don't think *any* project split into this many files has an intuitively obvious system for which functions belong where - that's precisely why IDEs have developed sophisticated methods for tracking and following code. So I don't think angband is more spaghetti-like than other projects of its size (and age - we wouldn't start from here, of course).

There is a fair point that the quality of comments in the code is variable. I think perhaps, without pointing any fingers, that the significant amount of development by a large number of people over the past few years has had the unfortunate side-effect of decreasing the overall value of comments - we all have different styles and different understandings of what's required, and it's hard to try and think like someone who doesn't know the code when your priority is committing the bugfix.

nppangband
July 20, 2010, 01:36
Yeah, we used to make fun of that one line of comment per line of code, but it really helps newbies apparently.

T.

Yep, one comment for each line is perhaps overkill, but there are whole files (such as ui-menu.c) that are barely documented now. There has to be some sort of happy medium.

And I apologize for this little soapbox speech I am about to make, but I knew nothing about coding when I considered making a variant. It was only the the highly detailed commenting in the Vanilla source that made me want to go ahead and try to make NPP. And I know plenty of other maintainers who made some great variants over the years that were in the same boat. They are also where many of the ideas that are now in Vanilla were developed and perfected. I am glad that Angband is under active development again, but I believe it is the rich variety that the variants offer that keeps people playing this game for decades. And it was Ben Harrison's code cleanup that made it all possible. ::end soapbox::

Nick
July 20, 2010, 03:11
Yep, one comment for each line is perhaps overkill, but there are whole files (such as ui-menu.c) that are barely documented now. There has to be some sort of happy medium.

And I apologize for this little soapbox speech I am about to make, but I knew nothing about coding when I considered making a variant. It was only the the highly detailed commenting in the Vanilla source that made me want to go ahead and try to make NPP. And I know plenty of other maintainers who made some great variants over the years that were in the same boat. They are also where many of the ideas that are now in Vanilla were developed and perfected. I am glad that Angband is under active development again, but I believe it is the rich variety that the variants offer that keeps people playing this game for decades. And it was Ben Harrison's code cleanup that made it all possible. ::end soapbox::

No need to apologize. I think I am more in agreement with every word of this than anything else ever.

Pete Mack
July 20, 2010, 10:38
OK, OK, mea culpa; mea maxima culpa!
I will write some comments for ui-menu.c (In my defense, I put lots of comments in the .h file.)

Oh yeah, and ui-menu.c isn't "thousands" of lines; only 922 including comments.

konijn_
July 20, 2010, 14:18
OK, OK, mea culpa; mea maxima culpa!
I will write some comments for ui-menu.c (In my defense, I put lots of comments in the .h file.)

Oh yeah, and ui-menu.c isn't "thousands" of lines; only 922 including comments.

;) Thanks Pete.

T.

Nick
July 20, 2010, 15:03
OK, OK, mea culpa; mea maxima culpa!
I will write some comments for ui-menu.c (In my defense, I put lots of comments in the .h file.)

Oh yeah, and ui-menu.c isn't "thousands" of lines; only 922 including comments.

Don't be too hard on yourself - I managed to work out how to use your menus, so it can't have been that bad. And there are other files where you pretty much have to make do with function names for comments (game-event.c, I'm looking at you).

In any case, development has been rapid lately, and it's not all that surprising that documentation hasn't kept up. It's a pity V doesn't have fully comprehensive doxygen support, like some variants...

nppangband
July 21, 2010, 13:10
OK, OK, mea culpa; mea maxima culpa!
I will write some comments for ui-menu.c (In my defense, I put lots of comments in the .h file.)

Oh yeah, and ui-menu.c isn't "thousands" of lines; only 922 including comments.

Pete - It is some great code. I just used it as an example because I had just spent some time getting NPP features like store services to work alongside the store inventory inside the new interface. In terms of ui-menu.c, most of it worked automatically. Ui_menu added the services right alongside the store inventory and I could highlight them with a mouseclick without having to de-bug or fix anything. I half expected it to have to make major changes, and I didn't. I am a big fan of how it works, even if I don't quite understand it yet.

For example: two of the store services in NPP are increase stat and restore stat, kind of like buying the potions and quaffing them. Currently, the player can purchase the service by mouseclick, but when it comes to picking a stat to restore/increase, they have to use the keyboard, because it isn't apparent to me how to create and display a list of the stats that need to be restored or increased, and have the player pick from the display with the mouse. No big deal, it is on my to-do list and I am sure I will figure it out when I get to it. But unless I missed it there is nowhere that says "here are the steps to create a menu, display it on the screen, and erase it after the user selects from a list." I don't think the code needs painstaking line-by-line commenting, just a quick paragraph or two on how to create an interactive list from scratch.

Nick had a better example, with game-event.c. I haven't looked too closely at it yet, but it isn't apparent to me why that was added, what was wrong with the code it replaced, or how it makes the game work better.

Nick
July 21, 2010, 14:05
Nick had a better example, with game-event.c. I haven't looked too closely at it yet, but it isn't apparent to me why that was added, what was wrong with the code it replaced, or how it makes the game work better.

My understanding is that it is an interface between the game and the UI, so the game says "this happened" and the UI works out how to draw it. But I certainly don't have the detail down.