Overmapbuffer function refactoring #32300
Merged
Conversation
This file contains 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
A bunch of map / overmap functions take x, y, z coordinates. Add overloads to (some of) those taking a point or tripoint. For now, those overloads just forward to the original function, but the ultimate goal is to remove the originals and only have the new overloads.
Remove almost all (x, y, z) APIs and replace wityh point / tripoint APIs. This is a first step towards making these APIs type-safe (CleverRaven#32017). A couple of APIs remain because they're more complex to change and this commit was already very big.
ZhilkinSerg
added
[C++]
KorGgenT
pushed a commit
to b0urgeoisie/Cataclysm-DDA
that referenced
this pull request
Jul 14, 2019
* Typo * Add some point / tripoint overloads for APIs A bunch of map / overmap functions take x, y, z coordinates. Add overloads to (some of) those taking a point or tripoint. For now, those overloads just forward to the original function, but the ultimate goal is to remove the originals and only have the new overloads. * Overhaul overmapbuffer APIs Remove almost all (x, y, z) APIs and replace wityh point / tripoint APIs. This is a first step towards making these APIs type-safe (CleverRaven#32017). A couple of APIs remain because they're more complex to change and this commit was already very big.
This was referenced Jul 24, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
SUMMARY: Infrastructure "Use point/tripoint rather than individual coordinates in overmapbuffer functions"
Purpose of change
Working towards #32017. If we want to use type-safe
points, the first step is to usepointat all, rather than individual coordinates. This PR does that for theovermapbuffermember functions.Describe the solution
Change almost all the functions and refactor all the places that call them.
I skipped
get_existing_om_globalandget_om_globalbecause they require changed return type, which was more complex refactoring and this PR was already too big. I intend to deal with those in a follow-up PR.Most of the refactoring improves to code clarity, I think.
Along the way I discovered some bugs in map extra coordinate transformations.
mx_roadblock,mx_bandits_block,mx_minefield, andmx_roadworkswere all using the wrong coordinate system to check adjacent terrain types, so they were probably not behaving as intended. More evidence that #32017 is needed, but I fixed these examples for now.Describe alternatives you've considered
This is a big chunk of changes in one go. If it's too intimidating to review and I should split it up further, let me know.
Additional context
Takes advantage of the new features I added recently in #32242 and #32269.
I tested this a little bit in-game. I found (and fixed) one bug in my changes to
overmap_ui, which is by far the most complex function I refactored. I suspect there are other bugs I'm introducing here; it's such a broad range of game mechanics being affected it's not practical to test thoroughly.If you are testing this PR before merge, pay special attention to the minimap and overmap UI; those are the most likely places for bugs to have crept in.