Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Start to make vehicle coordinates make sense #79457

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

PatrikLundell
Copy link
Contributor

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.

@github-actions github-actions bot added Map / Mapgen Overmap, Mapgen, Map extras, Map display Vehicles Vehicles, parts, mechanics & interactions Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` Appliance/Power Grid Anything to do with appliances and power grid json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 31, 2025
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Feb 1, 2025
@Maleclypse Maleclypse merged commit 8c76186 into CleverRaven:master Feb 1, 2025
29 checks passed
@PatrikLundell PatrikLundell deleted the vehicle branch February 1, 2025 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Appliance/Power Grid Anything to do with appliances and power grid astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants