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

Fix vehicle zones not invalidating on move #62108

Merged
merged 1 commit into from
Nov 14, 2022

Conversation

irwiss
Copy link
Contributor

@irwiss irwiss commented Nov 6, 2022

Summary

None

Purpose of change

Fixes #55091

Describe the solution

Rebuild vehicle zones after vehicles move

Describe alternatives you've considered

Testing

Scenario in linked issue

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) astyled astyled PR, label is assigned by github actions labels Nov 6, 2022
@irwiss irwiss marked this pull request as ready for review November 6, 2022 01:50
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Nov 6, 2022
@NetSysFire NetSysFire added Vehicles Vehicles, parts, mechanics & interactions Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones labels Nov 6, 2022
@Fris0uman
Copy link
Contributor

Does rebuilding the zone after every move have any performance impact?

@irwiss
Copy link
Contributor Author

irwiss commented Nov 11, 2022

It probably does but not in a perceivable way.

It's hard to catch the function in profiler since it's too fast to produce reliable samples, but it's possible to do a naive measurement using high_resolution_clock, replace the call at map.cpp:598 with

    const auto t1 = std::chrono::high_resolution_clock::now();
    // refresh vehicle zones for moved vehicles
    zone_manager::get_manager().cache_vzones( this );
    const auto t2 = std::chrono::high_resolution_clock::now();
    const std::chrono::duration<int64_t, std::nano> ns = t2 - t1;
    add_msg( "cache_vzones in vehmove took %d ns", ns.count() );

add #include <chrono> on top

It measures around 20000ns for no vehicles moving or around 190000ns (0.19 millisec) for moving vehicle with 100 bound zones

Don't think there's much to optimize in vehicle movement here, worst case autodrive is ever so slightly slower.

There is a spot to optimize - the zone_manager::cache_vzones() itself, it is called from all over the place including per-turn activities and is super weird; It calls map::get_vehicle_zones which then calls zone_manager::cache_vzones() once more for some weird reason

But that's a bigger change than a small fix

@dseguin dseguin merged commit 5833428 into CleverRaven:master Nov 14, 2022
@irwiss irwiss deleted the fix-veh-zone-move-invalidation branch November 14, 2022 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Inventory / AIM / Zones Inventory, Advanced Inventory Management or Zones json-styled JSON lint passed, label assigned by github actions Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vehicle-based auto-drink and auto-eat zones stop working after I move the vehicle
4 participants