Draft: dig-now plugin: remove dependencies on MapCache module.#5832
Draft
SilasD wants to merge 9 commits into
Draft
Draft: dig-now plugin: remove dependencies on MapCache module.#5832SilasD wants to merge 9 commits into
SilasD wants to merge 9 commits into
Conversation
this is intended to be a naive direct-replacement with minimal changes to algorithms. this commit is the easy stuff: remove MapExtras::MapCache &map parameter from functions change DFCoord to df::coord change MapCache::ensureBlockAt to Maps::ensureTileBlock change MapCache::BlockAtTile to Maps::getTileBlock change MapCache::tiletypeAt to *Maps::getTileType change MapCache::getDesignationAt to *Maps::getTileDesignation change MapCache::setDesignationAt to *Maps::getTileDesignation change MapCache::occupancyAt to *Maps::getTileOccupancy flag MapCache::layerMaterialAt for further work flag MapCache::baseMaterialAt for further work flag Block::veinTypeAt for further work flag Block::setStoneAt for further work flag Block::setSoilAt for further work flag DesignationJobs for further work add several now-necessary headers.
this is a naive-but-correct implementation with no caching.
cherry-pick from branch dig-now-test
ready to start code review.
turns out I'm not allowed to have unused functions.
so much for forward planning.
also, apparently 0 is signed! who would have known?
In file included from /home/runner/work/dfhack/dfhack/library/include/Error.h:31,
from /home/runner/work/dfhack/dfhack/library/include/BitArray.h:26,
from /home/runner/work/dfhack/dfhack/library/include/DataDefs.h:37,
from /home/runner/work/dfhack/dfhack/library/include/DataIdentity.h:38,
from /home/runner/work/dfhack/dfhack/library/include/DataFuncs.h:30,
from /home/runner/work/dfhack/dfhack/plugins/dig-now.cpp:5:
/home/runner/work/dfhack/dfhack/library/include/MiscUtils.h: In instantiation of ‘T clip_range(T, T1, T2) [with T = long unsigned int; T1 = int; T2 = long unsigned int]’:
/home/runner/work/dfhack/dfhack/plugins/dig-now.cpp:108:23: required from here
/home/runner/work/dfhack/dfhack/library/include/MiscUtils.h:553:11: error: comparison of integer expressions of different signedness: ‘long unsigned int’ and ‘int’ [-Werror=sign-compare]
553 | if (a < minv) return minv;
| ~~^~~~~~
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I request code review on this draft PR, with a focus on
dig-now.cpplines 60 to 380 or so, which contain new code, and thedig_type()function in lines 624-682, which is a complete reimplementation.I would like this code to be up to modern C++ standards, and I know that my C++ knowledge and skills are not the best.
The remainder of the file is old code updated with different types, e.g.
df::coordinstead ofDFCoord, and removal of all references to theMapExtras::MapCache &mapobject.There are also a couple of minor bugfixes, in particular in the
dig_tile()functionChannelcase.I don't think that part of the plugin needs full review yet, but I do welcome comments.
LuaLS annotations were added to
dig-now.lua. No code changes.General notes:
This PR is intended to be a naive-but-correct reimplementation of dig-now using the Maps module instead of the MapCache module. Note that it does not have the caching that MapCache provided. This results in a lot of unnecessary info lookups.
This PR is known to have bugs. These bugs already existed in the old version. They will be fixed in the near future.
This code has been verified to be almost bug-for-bug compatible with the old version. It results in the same map state with these exceptions:
map_block.flags.designated. This new version replicates the buggy behavior, but it properly clears that flag. I can't be bothered to track down the exact line and state to replicate that tiny bug, when I expect to fix the larger bug soon.clean_ramp()function. This has been seen on tower-cap cap ramps, but has not been verified for all undiggable ramps.Intended future directions, roughly in this order:
Maps::cuboid::forBlock(). Currently it is a naivelayer-by-layer, column-by-column, row-by-row scan.