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

Allow moving zone position #1364

Merged
merged 7 commits into from
Mar 5, 2022
Merged

Allow moving zone position #1364

merged 7 commits into from
Mar 5, 2022

Conversation

SaintCirno9
Copy link
Contributor

Summary

SUMMARY: [Features] "Allow moving zone position"

Purpose of change

Currently, the only way to change the position of a zone is to re-edit the zone, but in many cases it is more convenient to move the area directly than to re-edit. So it's a good idea to add a feature to move the zone directly.

Describe the solution

Add a new option below the option "Edit position", now the window looks like this:
image

You can then use the keyboard to move the zone:
GIF 2022-2-25 8-07-50

or mouse:
GIF 2022-2-25 8-14-16

If z-levels are enabled, moving zone across z-levels can also function, as shown in the gif above.

Describe alternatives you've considered

Testing

  • Create a new zone, select it and move it around, the zone saves fine.

  • Load an old save with existing zones, those zones can also be moved.

Additional context

@Coolthulhu
Copy link
Member

Nice!

Does it work with zones on vehicles? I see the following cases:

  • Moving zone from vehicle to map. Should probably "detach", maybe ask.
  • Moving zone from map to vehicle. Should probably ask to attach.
  • Moving zone from one vehicle tile to another. Should probably not detach.

@Coolthulhu
Copy link
Member

/home/runner/work/Cataclysm-BN/Cataclysm-BN/src/game.cpp: In member function ‘void game::zones_manager()’:
Error: /home/runner/work/Cataclysm-BN/Cataclysm-BN/src/game.cpp:6686:108: error: cannot bind non-const lvalue reference of type ‘tripoint&’ to an rvalue of type ‘tripoint’
                         const look_around_result result_local = look_around( false, zone_local_start_point + tripoint_zero,
                                                                                     ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~

For some reason, the center argument is not const. The function has a its own special return value where it could write the "new center", so there is no good reason to have pass-by-nonconst-ref center.
Still, if you want a quick fix, it's OK if you just add a temporary variable, preferably with a TODO: comment summarizing the issue.

@Coolthulhu Coolthulhu self-assigned this Feb 25, 2022
@SaintCirno9
Copy link
Contributor Author

Nice!

Does it work with zones on vehicles? I see the following cases:

  • Moving zone from vehicle to map. Should probably "detach", maybe ask.
  • Moving zone from map to vehicle. Should probably ask to attach.
  • Moving zone from one vehicle tile to another. Should probably not detach.

Because currently vzones only support 1x1 size, so I don't think there is any necessity to enable moving for vehicle zones, I may do that after vzones can be larger.

@Coolthulhu
Copy link
Member

/home/runner/work/Cataclysm-BN/Cataclysm-BN/src/game.cpp:6684:67: error: ellipsis preferred over three dots. [cata-text-style,-warnings-as-errors]
                        message_pop.message( "%s", _( "Moving zone..." ) );
                                                                  ^~~
                                                                  …
/home/runner/work/Cataclysm-BN/Cataclysm-BN/src/game.cpp:6979:13: error: Value stored to 'zone_blink' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
            zone_blink = blink;
            ^

src/game.cpp Outdated Show resolved Hide resolved
@Coolthulhu Coolthulhu merged commit 6f759f6 into cataclysmbnteam:upload Mar 5, 2022
@SaintCirno9 SaintCirno9 deleted the moving-zone branch March 6, 2022 02:08
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.

3 participants