View Single Post
Old November 1, 2019, 00:27   #2
Vanilla maintainer
Nick's Avatar
Join Date: Apr 2007
Location: Canberra, Australia
Age: 55
Posts: 8,641
Donated: $60
Nick will become famous soon enough
Interesting post, and you make some good points. Since becoming maintainer most of the work I have done on datafile parsing has been adding new stuff to parse. I have done a little re-organisation here and there, but have mainly been adding to your diagram rather than simplifying it

Anything I say below is from my limited understanding; comment from takkaria and other devs might invalidate some of it.

Now, my comments:

Originally Posted by Marco View Post
  • The stores and dungeon init/parse functions could be merged at the tail of the others array initialization and cleanup, in the pl[] array (init.c). It seems to work. The init_modules store_ and generate_ would no longer be necessary
As far as I can see, this leaves the modules for non-datafile game data, and combines all the datafile data into the arrays module. This seems like a good idea, and also suggests to me that some of the non-datafile stuff should make its way into datafiles (messages is one I've long wanted to do, but it is a big job).

Originally Posted by Marco View Post
  • All the File Parsers could be grouped in a few init-something files, like i.e. the z-something and ui-something groups, just to better separate and identify this support functionality from the main game logic. The issue, I think, is moving things away from old files, where people is used to found them, to other or new files . It is a problem to increase the number of .c/.h files?
This also seems like a reasonable idea. Originally all the parsing was in one file (well, two, but that's another story), and breaking it up as far as it currently is was probably a good idea. However, I think having init as a prefix (so init-obj rather than obj-init) is a decent plan. It's probably not a good idea to break up completely into one init file per datafile; object.txt, ego_item.txt and artifact.txt, for example, have enough things used in common (arrays of flags, etc) that it probably makes sense to keep them all in one file.

Originally Posted by Marco View Post
  • The FilePaser struct could comprise both the filename to parse (the current unused 'name' field) and the description to print (Initalizing... /Cannot initialize ...). Or both could be placed in the pl[] array? I implemented the first solution.
That seems completely sensible and much simpler.

Originally Posted by Marco View Post
  • The grab_something functions could be moved elsewhere, because they don't deal directly with datafiles, but I can't figure where. Maybe in parser.c, as ancillary parsing functions?
  • I also would like to better clarify in the code the roles/relations of (basic) Parser and FileParser, the latter sitting on top of the former one. There is something that escapes me, but I can't grasp exactly what. My fault maybe
So my understanding is this:
  • parser.c is the low level file which sets up functions like parser_getint() that the individual file parsers can use, and functions that create and destroy individual parsers;
  • datafile.c I made from pulling higher level stuff out of parser.c and lower level stuff out of the various file parsers; it's meant to be all the code that's more Angband-specific than parser.c, but applicable across lots of datafiles;
  • datafile.c also contains file handling routines for reading (usually by calling functions from parser.c) and writing datafiles;
  • Then the individual datafile parsers are the highest level.
So maybe datafile.c should contain just the file handling stuff, and the grab_*() functions could be moved to another file - init-util.c, maybe, if we're using init-*.c for intialisation files?

Thinking about this has been very helpful for me - I hope it has helped you a bit too.
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote