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

Several improvements to GridMap. #12488

Merged
merged 1 commit into from
Nov 16, 2017
Merged

Conversation

djrm
Copy link
Contributor

@djrm djrm commented Oct 29, 2017

closes #10697

also changed the appearance of the editor:

screenshot from 2017-10-29 14-36-56,

of course thats up for discussion, but i think the change makes more sense, and should be also implemented in tilemap.

@@ -54,8 +56,18 @@ void GridMapEditor::_configure() {

void GridMapEditor::_menu_option(int p_option) {

bool using_freelook = Input::get_singleton()->is_mouse_button_pressed(BUTTON_RIGHT);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there a way to know that without hardcoding an input check? @Zylann is about to add support for toggling freelook with a hotkey.

Copy link
Contributor

@Zylann Zylann Oct 29, 2017

Choose a reason for hiding this comment

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

I agree that you shouldn't imply RMB means freelook, that could also mean there is a design issue to solve before that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes thats why i added the flag, its true, that freelook mode could be toggled in different ways.

Copy link
Contributor

@Zylann Zylann Oct 29, 2017

Choose a reason for hiding this comment

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

That means we would need to change Gridmap if freelook changes... which isn't good design (not DRY and no SoC), especially because the toggle mode I'm adding would not be implementable with such flag in Gridmap, unless Gridmap also captures the involved shortcut...
Currently the only true way to access this info is with SpatialEditorViewport::is_freelook_active() though...
Isn't it possible to handle this by making the proper plugins consume events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Zylann that flag is only used to avoid rotation of tiles, if frelook, or any other shortcut / mode, changes grid map and other stuff will need to change (that flag might even be irrelevant on such cases), because of the possible conflicts, so this is not about design, godot works this way, maybe later we could have a better input handling mechanism.

Copy link
Contributor

@Zylann Zylann Oct 29, 2017

Choose a reason for hiding this comment

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

Note that when I say "consume events", Godot already supports this. If using that feature isn't enough then I don't know... but like @akien-mga says it will fail as soon as I PR the feature proposed in #10463 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Zylann also consider that such navigation mode, will make grid map editing almost unusable, or very hard at least (it will also override QWER keys for spatial gizmos), but yeah, im not sure how people will use it, also im not sure how to fix that in a smarter way, i mean there will be something here that requires knowledge of the general viewport shortcuts.

@reduz
Copy link
Member

reduz commented Oct 30, 2017

Some questions:

  1. would this not affect Maya style keybindings?
  2. why not something like ctrl-mouse button to erase or optionally adding a brush selection icon menu? (paint, erase, select)?

@reduz
Copy link
Member

reduz commented Nov 6, 2017

any hopes of fixing this before the beta?

@djrm
Copy link
Contributor Author

djrm commented Nov 6, 2017

@reduz im a bit lost, i cant figure out a way to capture the events produced by spatial editor so they dont propagate to the gridmap shortcuts or input events, thats the only thing im missing here.

@reduz
Copy link
Member

reduz commented Nov 6, 2017 via email

@djrm djrm force-pushed the pr_gridmap_fixes branch from 933fec6 to eea1e87 Compare November 9, 2017 22:14
@djrm
Copy link
Contributor Author

djrm commented Nov 9, 2017

i updated this to consider when freelook is active.

Fixed crash when undoing.
More ergonomic shortcuts.
Fixed freelook navigation.
@djrm djrm force-pushed the pr_gridmap_fixes branch from eea1e87 to 1b7f99d Compare November 9, 2017 22:19
@akien-mga akien-mga merged commit bb1d191 into godotengine:master Nov 16, 2017
@djrm djrm deleted the pr_gridmap_fixes branch November 17, 2017 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Freelook mode does not play well with Gridmap editor
4 participants