Adventure Mode Site Helpers: SC_NEW_MAP_AVAILABLE state change and World::GetCurrentSiteIdsWithExtraRange#5475
Conversation
| DFHACK_EXPORT df::unit * getAdventurer(); | ||
|
|
||
| DFHACK_EXPORT int32_t GetCurrentSiteId(); | ||
| DFHACK_EXPORT void GetCurrentSiteIdsWithExtraRange(std::vector<int32_t>& ids, const int32_t x_range = 0, const int32_t y_range = 0, const uint32_t max = UINT_MAX); |
There was a problem hiding this comment.
Needs to be documented in docs/dev/Lua API.rst
There was a problem hiding this comment.
My apologies for the delay in getting back to this. I had not added this function to the Lua API; would adding it be desirable, then?
I see that GetCurrentSiteID() is also not documented, despite being exported (and used to implement the actual documented dfhack.world.getCurrentSite()), so I am not sure what the criterion should be here.
I am willing to implement any changes as desired to reach the best code possible, my apologies for any confusion.
There was a problem hiding this comment.
The criterion is very simple: every visibly exported function should be documented. The fact that some things have slipped under the radar in the past is not a justification for doing it going forward.
After doing some research on the ways to properly get access to site IDs in Adventure Mode, I realized the
SC_MAP_LOADEDstate change only triggers on transitioning from no map to having a map loaded. The logic can easily be expanded to also consider the cases when there is a map and the map changes. For this, I propose a new state change, which I have tentatively calledSC_NEW_MAP_AVAILABLE.In the same vein, while
World::GetCurrentSiteIdshould correctly return the current site ID (or -1 if no site), some practical use cases may benefit from being able to get all sites in a caller-provided range around the adventurer. For this reason, I propose the introduction ofWorld::GetCurrentSiteIdsWithExtraRange, which extends the logic of the former function to take anstd::vectoras an in-parameter and return all applicable sites, optionally with extra range in x and y and a maximum number of sites to be return (which would be ordered by ID and not distance...).