widelands

Merge lp:~peter.waller/widelands/menu-fullscreen-hotkey into lp:widelands

  1. menu-fullscreen-hotkey
  2. Merge into trunk
Proposed by Peter Waller
Status: Rejected
Rejected by: SirVer
Proposed branch: lp:~peter.waller/widelands/menu-fullscreen-hotkey
Merge into: lp:widelands
Diff against target: 35 lines (+14/-0)
2 files modified
src/ui_fsmenu/base.cc (+13/-0)
src/ui_fsmenu/base.h (+1/-0)
To merge this branch: bzr merge lp:~peter.waller/widelands/menu-fullscreen-hotkey
Reviewer Review Type Date Requested Status
SirVer Disapprove
Review via email: mp+102973@code.launchpad.net

Description of the change

Added a handle_key function to Fullscreen_Menu_Base which just handles "SLDK_f" when the key is pressed down.

(tested) This provides the ability to toggle fullscreen from all of the widelands fullscreen menus.

There is some discussion about the SDL_WM_ToggleFullScreen function going away in SDL 1.3, in which case Graphics::toggle_fullscreen will need to be modified. But for the moment this just gives the same functionality in the menu as ingame.

To post a comment you must log in.
Revision history for this message
SirVer (sirver) wrote :

Thanks for your work pwaller! However, I am against merging this. It is *not* an official feature of SDL 1.2 - just a hack that happens to work on Linux. It *will* be gone in SDL 1.3. This feature is therefore deprecated with the moment we merge it into the core.

review: Disapprove
Revision history for this message
Peter Waller (peter.waller) wrote :

Hi SirVer. You're going to remove the ability to switch out of fullscreen altogether? When SDL changes, surely only Graphics::toggle_fullscreen needs to be modified, and the code in this merge would remain the same.

If work needs to begin to figure out how to have Graphics::toggle_fullscreen-like behaviour without the use of SDL_WM_ToggleFullScreen, then I might investigate that some day. Is there an open issue for it on launchpad?

Revision history for this message
SirVer (sirver) wrote :

>Hi SirVer. You're going to remove the ability to switch out of fullscreen altogether? When SDL changes, surely only Graphics::toggle_fullscreen needs to be modified, and the code in this merge would remain the same.
Very likely yes. The only way to *truly* support fullscreen <-> window
switching is to reinitialize the graphics system which might mean
reloading all graphics. Coding is non trivial.

>If work needs to begin to figure out how to have Graphics::toggle_fullscreen-like behaviour without the use of SDL_WM_ToggleFullScreen, then I might investigate that some day. Is there an open issue for it on launchpad?
Not as far as I know. If you could make this happen it would be much
appreciated.

Revision history for this message
SirVer (sirver) wrote :

I set this to rejected for now - this is not the proper way to do it unfortunately.

Unmerged revisions

6347. By Peter Waller

Add handle_key to Fullscreen_Menu_Base, for now only f to toggle fullscreen (see #682351)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/ui_fsmenu/base.cc'
2--- src/ui_fsmenu/base.cc 2012-03-02 13:21:49 +0000
3+++ src/ui_fsmenu/base.cc 2012-04-21 15:18:20 +0000
4@@ -101,6 +101,19 @@
5 }
6
7
8+bool Fullscreen_Menu_Base::handle_key(bool const down, SDL_keysym const code)
9+{
10+ if (down)
11+ switch (code.sym) {
12+ case SDLK_f:
13+ g_gr->toggle_fullscreen();
14+ return true;
15+ default:
16+ return false;
17+ }
18+ return false;
19+}
20+
21 uint32_t Fullscreen_Menu_Base::gr_x() {
22 return g_options.pull_section("global").get_int("xres", XRES);
23 }
24
25=== modified file 'src/ui_fsmenu/base.h'
26--- src/ui_fsmenu/base.h 2012-02-15 21:25:34 +0000
27+++ src/ui_fsmenu/base.h 2012-04-21 15:18:20 +0000
28@@ -40,6 +40,7 @@
29 ~Fullscreen_Menu_Base();
30
31 virtual void draw(RenderTarget &);
32+ virtual bool handle_key(bool down, SDL_keysym code);
33
34 public:
35 ///\return the size for texts fitting to current resolution

Subscribers

People subscribed via source and target branches

to status/vote changes: