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

Show remembered vehicles across z levels #56747

Merged
merged 4 commits into from
Apr 21, 2022

Conversation

haveric
Copy link
Contributor

@haveric haveric commented Apr 11, 2022

Summary

Bugfixes "Show remembered vehicles across z levels"

Purpose of change

Fixes #48897

Describe the solution

Track the z-level for remembered vehicles and show the symbol for the closest z-level. Shows "^" if vehicle is above, "v" if vehicle is below, or "c" if on the current level.

Describe alternatives you've considered

None

Testing

Loaded a game. Spawned some vehicles on varying z-levels near a silo and bridge. Examined each and set "Remember vehicle position" on them. Saw symbols showing the direction of the vehicle when standing on other z-levels.

Spawned vehicles at the top and bottom of a silo. Saw the symbol change (in the minimap) from "c" at the top, "^" below that, "v" one level above the ground, and then back to "c" at the ground.

Additional context

Screenshots

Map:
image

Minimap:
image

Graphical Overmap:
image
image

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Apr 11, 2022
@Maleclypse Maleclypse added the Vehicles Vehicles, parts, mechanics & interactions label Apr 11, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Apr 11, 2022
@haveric haveric mentioned this pull request Apr 12, 2022
Copy link
Member

@dseguin dseguin left a comment

Choose a reason for hiding this comment

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

Tested this locally and it works as advertised. Nice work! Spoke too soon, see edit below

I see that you added support for the overmap in the sidebar, but the "custom" sidebar uses a different function for drawing the overmap:

std::string display::colorized_overmap_text( const avatar &u, const int width, const int height )

Could you implement the same behaviour there? No problem if not, it's not a showstopper.


Edit: Looks like the z-level indicator only works with graphical overmap disabled:

notiles

Enabling it uses the previous behaviour:

tiles

@haveric haveric marked this pull request as draft April 20, 2022 12:53
@haveric
Copy link
Contributor Author

haveric commented Apr 20, 2022

I'll take another look at those when I get a chance. Moving this to draft for now.

@github-actions github-actions bot added Info / User Interface Game - player communication, menus, etc. Map / Mapgen Overmap, Mapgen, Map extras, Map display labels Apr 21, 2022
@haveric haveric marked this pull request as ready for review April 21, 2022 17:02
@haveric
Copy link
Contributor Author

haveric commented Apr 21, 2022

@dseguin I've updated it to support both. I'm not sure how to test the custom sidebar, but it should work the same way. I also decided to move the shared functionality to a function; it was getting to be in too many places and this should prevent accidental changes to one and not the others. Let me know if they belong outside of overmapbuffer though.

Also, is there anything I need to do to notify graphical overmaps of the new names?

Copy link
Member

@dseguin dseguin left a comment

Choose a reason for hiding this comment

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

Tested:

  • Overmap, non-graphical
  • Overmap, graphical
  • Overmap widget, default sidebar
  • Overmap widget, custom sidebar

Looks like it's all good!


For the future, you can swap the sidebar template to "custom" in the panel manager (} by default):

panel_adm

As for the new overmap tile ids, my understanding is that the tileset repo has scripts for discovering tile ids. If not, I would just post a comment in the graphics channel on Discord with the new ids just to be sure.

@dseguin dseguin merged commit e453626 into CleverRaven:master Apr 21, 2022
@haveric haveric deleted the remember-vehicle-z-level branch April 21, 2022 22:50
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` Info / User Interface Game - player communication, menus, etc. 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.

Remembered vehicle location is displayed at the ground level instead of the proper one
3 participants