Start to make vehicle coordinates make sense #79457
Merged
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
None
Purpose of change
Start the journey to allow vehicles to behave properly outside of the reality bubble (such as explosion maps).
Describe the solution
Change a bunch of vehicle properties to use absolute coordinates rather than bub coordinates of whatever map the vehicle happens to be associated with. This was then followed by adjustment to the usages of these properties to match.
Describe alternatives you've considered
There's some rather suspect code in vehicle::get_bounding_box() originally detected during typification (if you need to change a bub position into a rel one without a clear reason for why that's reasonable, that's a warning sign). The code retains the original logic in this PR, although I've added suggested changes in comments. Before changing it, there really should be some way to test the code before/after to verify it's wrong (if it is) and it's corrected (if it is). I wouldn't mind if someone took up that task, although I may try to tackle it myself at some time.
Testing
Load save, walk up ramp, jump into car, drive through hay bales, run over a turkey, crash into stationary vehicle. Nothing odd seen.
Additional context
There's a lot to be done to ensure vehicles actually behave properly on maps other than the reality bubble one. Many operations transitively call operations that just uses get_map() to get a map reference for bub coordinates, even though the original call was done using a different map (currently some explosions and mapgen in play).
There's been some effort done to treat mapgen differently from game play, but the determination logic is weak (the map used isn't get_map()), and I don't know if the calls for the mapgen case actually manages to keep get_map() contamination out of the way transitively (the default case operations do not).
This effort is, in turn, a smaller part in an effort to get the game to be consistent in what bub coordinates refer to (i.e. which map). It's rather ugly to have map class methods that just call get_map() themselves or do so transitively, as that violates the purpose of methods on objects. Getting vehicles to behave will have to involve getting some map methods to behave, as there are a lot of calls back and forth.
I've confirmed explosion effects outside the reality bubble (but on the explosion map) do not affect vehicles properly. That's one reason behind this PR.
I made an attempt to start from the map angle, but gave up on that PR as it kept growing and I ended up with some weird trouble with the map split test, without being able to figure out where the processing might have failed. It may have been possible to push through, but I decided to start afresh with a smaller bite to try to get at the overall problem more gradually. Anyway, it was nice to see the tests actually detecting errors, rather than failing for unrelated reasons for once.