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

Add more project window placement options #21150

Merged
merged 1 commit into from Aug 22, 2018

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Aug 18, 2018

It is now possible to use the monitor on the left or on the right of the editor to display running projects. If either the left or right end is reached, it will wrap around to the last or first monitor (respectively).

It looks like TTR() calls are not functional when used in setting hints (I tested it by setting the editor language to French, which translates the string mentioned), so I removed it in the setting hint.

This closes #20283.

screen = OS::get_singleton()->get_current_screen();
} else if (screen == 1) {
// Leftwards monitor (wrap to the other end if needed)
screen = abs((OS::get_singleton()->get_current_screen() - 1) % OS::get_singleton()->get_screen_count());
Copy link
Member

Choose a reason for hiding this comment

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

You should use Math::abs, abs is not portable.

Copy link
Member

Choose a reason for hiding this comment

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

Also, are we sure that all OSes identify monitors from left to right?

Copy link
Member Author

@Calinou Calinou Aug 19, 2018

Choose a reason for hiding this comment

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

Also, are we sure that all OSes identify monitors from left to right?

I don't think it's guaranteed as it depends on how monitors are set up, so I should perhaps rename the options.

@Calinou Calinou force-pushed the more-window-placement-options branch from 203af09 to 0a2a65a Compare August 19, 2018 08:30
screen = OS::get_singleton()->get_current_screen();
} else if (screen == 1) {
// Leftwards monitor (wrap to the other end if needed)
screen = Math::abs((OS::get_singleton()->get_current_screen() - 1) % OS::get_singleton()->get_screen_count());
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this will do what you expect when current screen is 0, modulo operator on negative operands is implementation defined.

It would be safer (and cleaner) to use Math::wrapi() for that (and the screen == 2 case too I guess for consistency), since that's its purpose.

It is now possible to use the previous or next monitor (relative to
the editor) to display running projects. If either end is reached,
it will wrap around to the last or first monitor (respectively).

This closes godotengine#20283.
@Calinou Calinou force-pushed the more-window-placement-options branch from 0a2a65a to 475a46c Compare August 22, 2018 18:24
@Calinou
Copy link
Member Author

Calinou commented Aug 22, 2018

I updated this pull request, please check it again 🙂

@akien-mga akien-mga merged commit a752e2f into godotengine:master Aug 22, 2018
@akien-mga
Copy link
Member

Looks good, thanks!

@Calinou Calinou deleted the more-window-placement-options branch January 27, 2020 18:24
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.

Run Window Placement Not Same as Editor
2 participants