So I finally managed to refactor monster spells into a data table (or
three, in fact). It taught me a lot. As takkaria notes in the first post, the thinking in the original code is not always easy to divine, as layer upon layer of change over the years either obscures it or undermines it (if it was there in the first place).What we had was half the spells being resolved in monster2.c (the non-projectable ones), and the other half being resolved by the projection code in spells1.c. We now have a separate mon-spell.c which will handle all spells and their side effects, calling out to project() where necessary.
Monster spells, by the way, means anything that isn't a melee attack - so all breaths, balls and bolts as well as the non-damaging spells like teleportation, confusion, summoning etc. Anything that's on an S: line in monster.txt and has an RSF_ flag in the code.
This work led me to two conclusions: the first is that data tables and nice flavourful text output don't mix well. It was hard enough doing all the different verbs for slay_table, but I think it's worth moving to an edit file for the spell text. If I understand takkaria correctly the list-foo.h data tables are a stepping stone towards some sort of scriptable edit file setup anyway, and elly's brilliant parser API makes writing a new parser possible for the syntactically challenged. This way we can keep (and even enhance) the flavour of the different in-game messages without cluttering the list-* files.
The second conclusion is that we ought to go back to first principles and have a think about object flags before we make too many more decisions which are constrained by the existing setup. Object flags are fundamental to the game, because they are used by p_ptr->state to determine resistances and other abilities granted to @ by objects. (Perhaps really going back to first principles would challenge this way of tracking @'s state, but that's a separate essay - for now I'll start from here.)
At the moment object flags are all lumped together in a single group, and we use a load of #defines to create "masks" for particular flag types (slays, brands, obvious flags, etc.). We recently expanded OF_SIZE because we went beyond the limit of 96 flags when the new HATES_FOO flags were introduced in object_base.txt. We can now have up to 255 object flags before we need to change savefiles again. (takkaria has written a new block-based savefile format, which should mean much better savefile compat in future.)
But there are already two different types of flags, and should probably be more. There are binary flags (you either have fire immunity or you don't), and there are pval flags (you have an object which affects your STR, but the precise effect is determined by a pval).
Before I added multiple pvals, there was no structural governance of which flags were connected with an object's pval and which weren't - it was all done by hard-coded rules. Now we have a set of o_ptr->pval_flags for each pval, which simply indexes each flag which is related to that pval. This may seem memory-inefficient at 32 bytes for each pval, but it allows object flag operations to use a single flagset instead of searching over N pval flagsets (+1 for the non-pval flags). It also allows a binary flag to become a pval flag with a minimum of hackery.
As I was acquainting myself with the ID code last year, I was gratified to learn that o_ptr->known_flags is exactly what you expect it to be: a bitflag which tells you which object flags @ knows to be present on the object. I assumed that o_ptr->flags was the actual flags present on the object - and it is in a way, but it's almost never used directly.
Instead a function will create a bitflag f[OF_SIZE] and call object_flags(f). This copies into f the flags of the base object type, those of the object kind, then flags from any ego type or artifact. Instead of being done once at object creation, this is done repeatedly throughout the code. If there was ever a reason for this I can't find it - the object list is global, so the memory for all the o_ptr->flags of all the objects is already in use at any point.
Of course, the object list isn't a single list. There's o_list, which has all the items on the current dungeon level, including those carried by monsters. There's p_ptr->inventory, which is everything @ is carrying and wearing. There's st_ptr->stock, which is all the objects in a store. But they're all global.
In fact, the only flags which *aren't* copied over every time object_flags is called are the curse flags - so when you remove a curse from an object, it stays removed. Now, why doesn't this apply to every other flag too? That way all sorts of spells and effects in the game could add or remove flags from objects, opening up all sorts of possibilities (forging, anyone?) - including the long-gestated new curses. Recent discussion on Oook came down against allowing effects to remove flags from objects, but dispelling timed buffs was considered tolerable.
So we come to the issue of timed flags. The TMD_ flag set is quite recent (2008) and the MON_TMD_ group are even more so. We will have to be careful to keep these separate from object flags, and test for both timed and object flags if necessary. In calc_bonuses we copy TMD_OPP_CONF to p_ptr->state, and likewise with TMD_AFRAID. We should soon be able to do without this, as the new check_for_resist function returns nonzero if either is present. But at the moment it only works for effects whose temporary and permanent effects are linked in gf_table, so something needs redesigning if it is to be more broadly useful.
My suggestions so far:
1) Make o_ptr->flags the definitive source of an object's flags (call object_flags at creation, but not subsequently). This would allow us to do lots of interesting things to add flags before upsetting people by taking any away (but some of the additions could be curses ...)
2) Synchronise OF_FOO and TMD_FOO so that they are linked pairs. This should enable a single check for any flag in the player's state, regardless of whether it comes from an object or a temporary spell effect.
3) Add o_ptr->timed_flags[OF_MAX] so that each object flag can be added or removed temporarily as well as permanently. This would add about 600k to the memory footprint (256x u16b per object, or more if we want timers over 2^16), but should still leave us about two orders of magnitude short of memory-limiting an average smartphone.
4) Create of_table[OF_MAX] to replace all the #defines for OF_FOO_MASK. This would also allow the encoding of other info about a particular flag (e.g. whether it could be found as a random ego power, or on a randart).
None of this directly changes gameplay, but it opens up possibilities that the current code cannot offer. Even if we don't want to use them, variant maintainers might.
