Add shift and control modifiers to close window button

[ccahiggins]

Add keyboard modifiers to the window close widget.
Pressing shift while closing the window leaves that window open, but closes all other windows, e.g. close all staff windows leaving ride construction open.
Pressing control while closing the window closes all windows of the current window class, e.g. close all guest widows.

[IntelOrca]

You should try to avoid needing this logic in every window and see if you can make the logic generic and part of the core window logic.

[ccahiggins]

You should try to avoid needing this logic in every window and see if you can make the logic generic and part of the core window logic.

Oh that sounds like a good idea. Remove duplicating the input check each time. Would it be confusing if Close() has the check in it? Probably need more testing if it affects all windows rather than specific ones, just in case it causes some weirdness.

[IntelOrca]

@ccahiggins It could go in Close, but that is used in other circumstances. I think the close button widget uses its own widget type, CloseBox, so you could put the check logic in the generic code. Set a flag to say next close call for what specific window owning the widget should do your special logic.

[ccahiggins]

@IntelOrca Good point, could lead to some confusion if Close() is called while shift happens to be pressed. Ok thanks, I'll have a look at CloseBox and see if I can do something with that.

[ccahiggins]

Ok, so one way to do this could be to use MouseInput and check on LeftReleased that it was a CloseBox that was pressed. A new field in rct_window can be set to shift/ctrl/none, and checked inside Close().

[IntelOrca]

Ok, so one way to do this could be to use MouseInput and check on LeftReleased that it was a CloseBox that was pressed. A new field in rct_window can be set to shift/ctrl/none, and checked inside Close().

Only one window will ever have this flag set at one time, so no need for it to be stored in every window. Instead have a global that stores the (window class, number) that should close with special logic.

[ccahiggins]

Ok yeah that makes sense. C++ is definitely not my first language, too used to Java, so this is some fun learning, thanks :)

How's this sound?
src/openrct2/interface/Window.h:

• Enum for modifier types (shift, control, none)
• Struct for rct_windownumber, WindowClass, modifier Enum
• extern to struct defined in Window.cpp

src/openrct2/interface/Window.cpp:

• define struct for last window modifier

src/openrct2-ui/input/MouseInput.cpp:

• in InputStateWidgetPressed, check when state == LeftRelease
• if widget == CloseBox, and shift/ctrl pressed, set last modifier to shift/control

src/openrct2-ui/interface/Window.cpp:

• in Close(), check if this is last window with close modifier
• close other window/windows of class type, depending on last modifier
• reset last modifier to none
• maybe also set none if number/class don't match, just in case close is called by something else to avoid random issues

Notes:

Instead of defining a new Enum, reuse the PLACE_OBJECT_MODIFIER Enum?

With the MouseInput, two ways to test for this:

• new if statement for state == LeftRelease
• use existing switch, case MouseState::LeftRelease: - however this would need a fallthrough otherwise inputs are very broken

Also, I've noticed RightPress state gets set even when I don't press the right button, is that normal? And released is set a lot (is that release for right when I have left pressed?)

Not all windows are refactored to the new openrct2-ui Window class, options:

• use this solution, understanding that some windows won't have this functionality until they are refactored
• wait until all have been refactored so that all windows can use this
• add additional code to all classes implementing old Window class, which can be removed when they are refactored
• don't think this is a good one: also add modifier check to window_close so other windows also can use this

[IntelOrca]

Sounds good. It is a bit hacky, but the best that can be done for now until we are able to make the code for window shims more common.

Struct for rct_windownumber, WindowClass, modifier Enum

I think there is already a struct for class, number and widget index, so use that - even if it is in a new structure that is together with the enum.

Instead of defining a new Enum, reuse the PLACE_OBJECT_MODIFIER Enum?

I would make a new enum with more meaningful identifiers.

Also, I've noticed RightPress state gets set even when I don't press the right button, is that normal? And released is set a lot (is that release for right when I have left pressed?)

The input logic is very messy, so I am not surprised by this. I am not really sure what the right behaviour is.

use this solution, understanding that some windows won't have this functionality until they are refactored

I think this makes the most sense.

[ccahiggins]

Changes have been pushed now.

And this is a demo of the new behaviour (image links to a YouTube demo)

[IntelOrca]

Looks good for the most part, a changelog entry should be added for this.

[tupaschoal]

@IntelOrca can you review this again and see if it's ready for merge? Seems like the changes you requested were incorporated by the OP

@ccahiggins tip for the future: whenever you implement suggestions as they are by reviewers, make sure to mark the conversations as resolved, so that the PR history becomes cleaner and easier to re-review.

[tupaschoal]

@ccahiggins can you rebase so that the changelog entry sits on the 0.4.4 version, since 0.4.3 was already released?

Also I don't know if we can do better on the changelog to explain what the modifiers do?