Are you still having problems with the unit death event? I was reading back and you said it was missing some. I haven't touched it in a long time so it's probably still broken but it might have magically fixed itself with some DF change. edit: It's a pretty cheap test so you can probably afford to check it every frame without slowing FPS too much.
I haven't checked in awhile, but I am not sure if it's that it isn't triggering, or that the last attacker doesn't include death by bleeding or something. I should do some more thorough tests to see the efficiency.
Chaining: Suppose you want to shoot a magic bolt that splits at the target and bounces around. A spell that immediately bounces around seems fairly straightforward but if you need the bolt to actually hit the target in order to bounce you might have to use item-trigger or projectile-trigger in a tricky way. It's a weird case. I don't actually need it so if it's not possible don't worry about it.
That is a good idea, right now chaining just works by replicating the spell and messing with the source and target id's. It would be cool if it relied on actually hitting the first target. I would have to think of a clever way to handle that. I think it would involve projectile-trigger and some trickyness, but I think it could be done.
Looking over everything. Overall I think the code looks pretty great. A few quibbles:
There was a createitem script which overlaps with modtools/create-item. I think the only difference was yours includes an optional duration.
Yes, that is basically the only difference between the two, it might be worth it to just include the optional duration in the modtools version, and then I can scrap my version.
When at least one of several is required, I find it best to say -flowType [magma/water] instead of -magma or -water.
Technically only -magma is needed for that, it defaults to -water, but I understand your point
dfhack.run_script('blah',table.unpack('-arg1','asdf','-arg2',asdf2)) is equivalent to
dfhack.run_script('blah -arg1 asdf -arg2 ' .. asdf2)
It's a stylistic choice. Both are fine.
Hmmm, I like the second way more than my way, I didn't realize it was equivalent, I'll change them over.
asdf.name can be used in place of asdf['name'] and it generally looks better.
I was having difficulties with the . convention. Sometimes it would work fine, othertimes it would not. Is that because for something like asdf.end, end is a special function in lua, and so it gets upset?
Falling and projectile and propel are similar enough they should probably be combined unless I misunderstand what they do. They should also delegate to dfhack.items.createItem or just work with a pre-existing item. If you want to simultaneously create and project an item you could do something like
multicmd
modtools/create-item -item WEAPON:ITEM_WEAPON_PICK
falling -item lastCreated -blah blah
Yes, they are all three very similar, propel works just on units, falling is just items falling from the sky, and projectile is items moving from one x,y,z to another. The reason they are three separate scripts now is because they started as three separate spells that I expanded on (cyclone, blizzard, and multi-shot respectively). Falling and projectile could easily be combined, it might be nice to keep propel separate though so people know that one is for items and one is for units.
I like your idea of having the item one work on pre-existing items, and simply creating items if need be. That simplifies the code greatly, and allows for using actual ammunition on a unit to perform a special shot. I will look into it.
createCallback: you generally don't need to do this. You can just say something like
dfhack.timeout(100,'ticks', function () blah end)
I don't know why I never tried that, its much simpler. Is there any drawback to using the createCallback though?
When checking if a syndrome has any of a list of syn_classes you technically should be doing the O(n) algorithm with hash tables instead of the O(n^2) algorithm of all pairs comparisons, but most syndromes have few syn_classes and most commands list few syn_classes so it should be fine in practice.
Hmm, good point, it could get computationally expensive pretty quick if someone has syndromes with gigantic lists of syn_classes.
On a side note, add-syndrome is currently able to remove a syndrome based on the SYN_NAME, would it be possible to have to remove syndromes based on SYN_CLASS too? That way you could remove multiple syndromes all with different names, with the same command.
Looking at the way you use persistent variables I think there might be an easier way of doing persistent tables than I thought. Something like
local valueStr = dfhack.persistent.tableLookup(tableName,keyName)
dfhack.persistent.tableUpdate(tableName,keyName,valueStr)
In the implementation I'd have the C++ side of things cache the indices of the actual histfigs it's using so that it would get O(1) performance instead of linear performance. It would only make your code slightly cleaner but it would be much faster for looking up stored data.
That would be very awesome if you could do that, also because it would allow me to store the different text files that the classes and civilization scripts use in a persistent table, which would mean they wouldn't have to be read each time you want to do an action with them.
Still need to do no-target interactions in EventManager. I think I'll just allow it to trigger when the number of defenders isn't exactly one. It could lead to problems, so anyone who uses it would have to check whether the number of targets is appropriate or not.
I mainly work with with targeted interactions, even my interactions that are self targeted use the USAGE_HINT:ATTACK so that the combat log isn't spammed with Urist McDwarf casts Stone Skin when he is off growing plants, and is only triggered in combat. With the wrapper script this is as easy as changing !TARGET to !SOURCE. I suppose I could see it being useful to have the ability for interaction-trigger to work on a no-target system, it would definitely be useful for a multi-target system, although the wrapper script can also handle that too.