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

Some fixes to autodrive and map memory behavior #1918

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

olanti-p
Copy link
Contributor

Summary

SUMMARY: Bugfixes "Fixed a couple issues with autodrive and map memory"

Purpose of change

  1. Fix some rare occurences of Called map_memory::get_submap() before initializing map memory region.
  2. Make autodrive work at night when not using tiles

Describe the solution

  1. Preparing memory map region is now not a hard requirement, but rather an optimization.
    Where before the mm_submaps were searched/loaded/allocated only on preparation step, and get_tile()/get_symbol() would only return data from prepared region, now these heavy operations can potentially happen on every get_* call. This makes it harder to reason about performance, but less prone to breakages.
  2. Map memory was never designed as part of game state. It's a graphical overlay, it's state is used by map drawing routines and the occasional "unmemorize vehicle when it moves" hack. Using it for autodrive is hackish, but works. Solution was to encapsulate the hack, and check for either memorized tile or symbol, not just tile (original author probably assumed it was terrain tile id?).

Describe alternatives you've considered

  1. It was a bad initial design choice on my part, should've gone with this solution back then.
  2. Rewriting map memory from grounds up, to be updated on every turn or avatar action, and memorize actual map data?

Testing

  1. I tried reproducing these before, but there's no precise repro steps whenever it gets reported. I'm fairly confident this will solve the issue, but can't verify beyond commenting out prepare_map_memory_region part in cata_tiles.cpp.
  2. Autodrive a car in a straight line at day, do a u-turn, set time to night, try autodriving back.
    Without fix: works on tiles mode, fails on text mode. After fix: works on both versions.

@Coolthulhu Coolthulhu self-assigned this Sep 27, 2022
@@ -96,6 +96,8 @@ class avatar : public player
/** Returns last stored map tile in given location in curses mode */
int get_memorized_symbol( const tripoint &p ) const;
void clear_memorized_tile( const tripoint &pos );
/** Returns last stored map tile in given location in tiles mode */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the comment was for something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, copied it but forgot to change afterwards.

@@ -71,25 +70,37 @@ map_memory::map_memory()
clear_cache();
}

const memorized_terrain_tile &map_memory::get_tile( const tripoint &pos ) const
const memorized_terrain_tile &map_memory::get_tile( const tripoint &pos )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this lose const?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because get_submap() is no longer const. It's either this, or hacky const_cast, or spamming mutable, and this felt least bad.

@Coolthulhu Coolthulhu merged commit 941cf12 into cataclysmbnteam:upload Sep 29, 2022
@olanti-p olanti-p deleted the mm-autodrive-hack-fix branch October 11, 2022 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants