implement Maps::forEachTile - #5963
Conversation
API for bulk operations over portions of the map
There was a problem hiding this comment.
some quick thoughts on the API; it generally looks powerful and flexible.
I'll look at the code tomorrow / soon. not sure what I can spot in C++ code, but I'll look.
Edit: I've committed the review, but I've had a couple more thoughts.
- it might be nice to have a way to set the z traversal direction.
several of the uses Squamous will have for this will benefit from coming down from the top of the sky and stopping when the scan hits a block that is all rough walls. for example, changing the material of a keep to a custom material CONCRETE.
other use cases would benefit from starting at the bottom and working up until they hit a block that is all OpenSpace with designation.light, or designation.outside, or not designation.subterranean. for example, the bulk layer material replacement that kick-started this.
- in a similar vein, it might be nice to receive a couple of flags indicating that a block is 'uninteresting' -- it is composed completely of
OpenSpacetiles or of{Stone,Mineral,Lava,Feature}Walltiles. possibly mixed with a flag indicating that the entire block is hidden. or have the caller pass a flag to just skip the callbacks in those cases.
maybe that should be done in a block-oriented traverser like Maps::cuboid::forBlock().
None of this is well-thought-out, these are just initial thoughts.
| required values, e.g. ``{hidden=false, subterranean=true}``. | ||
| Multi-bit fields such as ``dig`` or ``flow_size`` take integer values. | ||
| - ``occupancy``: same, for ``df.tile_occupancy`` fields. | ||
| - ``filter``: a function ``fn(x, y, z, block, tiletype)`` evaluated last |
There was a problem hiding this comment.
relevant both for filter and for actions.callback:
one of the most common operations Squamous does is calculating x % 16 and y % 16. this is dog-slow if you don't know to use the bitmath equivalent x & 15 and y & 15. I think passing those in for convenience would provide a decent speedup in some common use-cases.
fn(x, y, z, block, localx, localy, tiletype) .
(edited for typo, used & bit-and where I meant % modulus.)
another common operation is checking the neighboring tiles for something. I'm not at all sure how to speed that up; I an guessing that forEachTile expects to traverse a large swath of the map, and that setup costs will largely preclude fn from making many, many calls to a second level of forEachTile with a tiny cuboid of (x-1, y-1, z), (x+1, y+1, z). only a guess, as I haven't even glanced at the code yet.
There was a problem hiding this comment.
Implemented the first. Haven't implemented the second - will have to think about it
| ``item_type`` (number or ``df.item_type`` name), ``item_subtype``, | ||
| ``mat_type``, ``mat_index``, ``flags`` (a table of | ||
| ``df.construction_flags`` field names to values, e.g. | ||
| ``{no_build_item=true}``), and ``tiletype`` (a tiletype to write to the |
There was a problem hiding this comment.
I suggest requiring that the replacement tiletype be one of those that are constructions, i.e. the C++ equivalent of df.tiletype.attrs[n].material == df.tiletype_material.CONSTRUCTION.
if the user needs something else, they could use the callback to force the tiletype.
Edit: you might also want to require that exactly one of flags.no_build_item and flags.top_of_wall is set. that's because there won't be any blocks/boulders/whatever buried in that tile.
and maybe require that flags.reinforced is clear. You don't set, or don't document setting, the hacky fields sec_i_sc1 and sec_i_sc2 used for reinforced walls. (Also I kind of suspect that flags.reinforced doesn't interoperate with flags.no_build_items; just guessing.)
OTOH any trouble with no-build-item reinforced walls probably only crops up when the wall is deconstructed. so maybe it's all an ignorable issue.
There was a problem hiding this comment.
Still thinking about this one, you might want to expand your comments some
added an optional tiletype replacement map added passing block-local coordinates to callbacks
|
I'm going to move this to ready-to-merge status because i want to get it into the upcoming beta so Squamous (and others) can experiment with it and give me feedback more readily. I've added a note to the documentation noting that the API should not be considered stable. |
2 participants
API for bulk operations over portions of the map