Changes look mostly good to me. A few comments in the diff, but nothing
serious. In think there is a doubled function call to scroll_to_field() but
that can be fixed.
I couldn't figure out where the new focus() of the panel or the scrolling for
boxes are used. I wasn't able to find a box with scrollbars that is not already
a list or textarea, maybe there simply is non in the current game. Also I
didn't notice any changes due to the focus() call. Adding a printf() call shows
that the code is reached but I found no behavior changes.
Diff comments:
>
> === modified file 'src/ui_basic/box.cc'
> --- src/ui_basic/box.cc 2017-03-11 17:10:33 +0000
> +++ src/ui_basic/box.cc 2017-12-09 19:38:06 +0000
> @@ -135,6 +135,23 @@
> layout();
> }
>
> +bool Box::handle_mousewheel(uint32_t which, int32_t x, int32_t y) {
Is there some GUI where this is used? All scrollbars I found were with
textareas or lists.
Not that I am against adding this here.
> + if (scrollbar_) {
> + assert(scrolling_);
> + return scrollbar_->handle_mousewheel(which, x, y);
> + }
> + return Panel::handle_mousewheel(which, x, y);
> +
> +}
> +bool Box::handle_key(bool down, SDL_Keysym code) {
> + if (scrollbar_) {
> + assert(scrolling_);
> + return scrollbar_->handle_key(down, code);
> + }
> + return Panel::handle_key(down, code);
> +}
> +
> +
> /**
> * Adjust all the children and the box's size.
> */
>
> === modified file 'src/ui_basic/panel.cc'
> --- src/ui_basic/panel.cc 2017-09-04 15:02:05 +0000
> +++ src/ui_basic/panel.cc 2017-12-09 19:38:06 +0000
> @@ -501,7 +501,10 @@
> *
> * \return true if the mouseclick was processed, flase otherwise
flase -> false.
Okay, not really a problem of this branch.
> */
> -bool Panel::handle_mousepress(const uint8_t, int32_t, int32_t) {
> +bool Panel::handle_mousepress(const uint8_t btn, int32_t, int32_t) {
> + if (btn == SDL_BUTTON_LEFT) {
> + focus();
Where is this used / what it is doing?
In trunk when I have two windows and click on the "lower" one, it is brought to
the front and its (potential) button is clicked. This would have been the
behavior I expected through this change but seemingly it is not required.
> + }
> return false;
> }
>
>
> === modified file 'src/ui_basic/scrollbar.cc'
> --- src/ui_basic/scrollbar.cc 2017-05-18 21:26:29 +0000
> +++ src/ui_basic/scrollbar.cc 2017-12-09 19:38:06 +0000
> @@ -438,6 +438,75 @@
> return true;
> }
>
> +bool Scrollbar::handle_key(bool down, SDL_Keysym code) {
> + if (down) {
> + if (horizontal_) {
> + switch (code.sym) {
> + case SDLK_KP_6:
> + if (code.mod & KMOD_NUM) {
> + break;
> + }
> + FALLS_THROUGH;
> + case SDLK_RIGHT:
> + action(Plus);
Maybe make Area an enum class?
> + return true;
> +
> + case SDLK_KP_4:
> + if (code.mod & KMOD_NUM) {
> + break;
> + }
> + FALLS_THROUGH;
> + case SDLK_LEFT:
> + action(Minus);
> + return true;
> + default:
> + break; // not handled
> + }
> + } else {
> + switch (code.sym) {
> + case SDLK_KP_2:
> + if (code.mod & KMOD_NUM) {
> + break;
> + }
> + FALLS_THROUGH;
> + case SDLK_DOWN:
> + action(Plus);
> + return true;
> +
> + case SDLK_KP_8:
> + if (code.mod & KMOD_NUM) {
> + break;
> + }
> + FALLS_THROUGH;
> + case SDLK_UP:
> + action(Minus);
> + return true;
> +
> + case SDLK_KP_3:
> + if (code.mod & KMOD_NUM) {
> + break;
> + }
> + FALLS_THROUGH;
> + case SDLK_PAGEDOWN:
> + action(PlusPage);
> + return true;
> +
> + case SDLK_KP_9:
> + if (code.mod & KMOD_NUM) {
> + break;
> + }
> + FALLS_THROUGH;
> + case SDLK_PAGEUP:
> + action(MinusPage);
> + return true;
> + default:
> + break; // not handled
> + }
> + }
> + }
> + return Panel::handle_key(down, code);
> +}
> +
> void Scrollbar::layout() {
> if ((2 * kSize + get_knob_size()) > static_cast<uint32_t>((horizontal_
> ? get_w() : get_h()))) {
> buttonsize_ = kSize / 2;
>
> === modified file 'src/wui/story_message_box.cc'
> --- src/wui/story_message_box.cc 2017-01-25 18:55:59 +0000
> +++ src/wui/story_message_box.cc 2017-12-09 19:38:06 +0000
> @@ -19,68 +19,84 @@
>
> #include "wui/story_message_box.h"
>
> +#include "base/i18n.h"
> #include "graphic/graphic.h"
> +#include "logic/game_controller.h"
> +#include "logic/save_handler.h"
> #include "ui_basic/button.h"
> #include "ui_basic/multilinetextarea.h"
> #include "ui_basic/textarea.h"
> -
> -/**
> - * The message box itself
> - */
> -StoryMessageBox::StoryMessageBox(UI::Panel* const parent,
> +#include "wui/interactive_player.h"
> +
> +namespace {
> +constexpr int kPadding = 4;
> +}
> +
> +StoryMessageBox::StoryMessageBox(Widelands::Game* game,
> +
> const Widelands::Coords coords,
> const std::string& title,
> const std::string& body,
> - const std::string& button_text,
> - int32_t const gposx,
> - int32_t const gposy,
> + int32_t const x,
> + int32_t const y,
> uint32_t const w,
> uint32_t const h)
> - : UI::Window(parent, "story_message_box", 0, 0, 600, 400, title.c_str()) {
> - UI::MultilineTextarea* message_text = nullptr;
> - int32_t const spacing = 5;
> - int32_t offsy = 5;
> - int32_t offsx = spacing;
> - int32_t posx = offsx;
> - int32_t posy = offsy;
> -
> - set_inner_size(w, h);
> - message_text = new UI::MultilineTextarea(
> - this, posx, posy, get_inner_w() - posx - spacing, get_inner_h() -
> posy - 2 * spacing - 50);
> -
> - if (message_text)
> - message_text->set_text(body);
> -
> - int32_t const but_width = 120;
> - int32_t space = get_inner_w() - 2 * spacing;
> - space -= but_width;
> - space /= 2; // center button
> - posx = spacing;
> - posy = get_inner_h() - 30;
> - posx += space;
> - UI::Button* okbtn = new UI::Button(this, "ok", posx, posy, but_width,
> 20,
> -
> g_gr->images().get("images/ui_basic/but5.png"), button_text);
> - okbtn->sigclicked.connect(boost::bind(&StoryMessageBox::clicked_ok,
> boost::ref(*this)));
> -
> - center_to_parent();
> -
> - if (gposx != -1)
> - set_pos(Vector2i(gposx, get_y()));
> - if (gposy != -1)
> - set_pos(Vector2i(get_x(), gposy));
> -
> + : UI::Window(game->get_ipl(), "story_message_box", x, y, w, h,
> title.c_str()),
> + main_box_(this, kPadding, kPadding, UI::Box::Vertical, 0, 0,
> kPadding),
> + button_box_(&main_box_, kPadding, kPadding, UI::Box::Horizontal, 0,
> 0, kPadding),
> + textarea_(&main_box_, 0, 0, 100, 100, ""),
> + ok_(&button_box_, "ok", 0, 0, 120, 0,
> g_gr->images().get("images/ui_basic/but5.png"), _("OK")),
> + desired_speed_(game->game_controller()->desired_speed()),
> + game_(game) {
> +
> + // Pause the game
> + game_->game_controller()->set_desired_speed(0);
> + game_->save_handler().set_allow_saving(false);
> +
> + // Adjust map view
> + if (coords != Widelands::Coords::null()) {
> + game_->get_ipl()->map_view()->scroll_to_field(coords,
> MapView::Transition::Jump);
This call is also done in the lua function above. Remove it in the lua function?
> + }
> +
> + // TODO(GunChleoc): When all campaigns and scenarios have been
> converted, simply add the body text above.
> + try {
> + textarea_.force_new_renderer();
> + textarea_.set_text(body);
> + log("Story Message Box: using NEW font renderer.\n");
> + } catch (const std::exception& e) {
> + log("Story Message Box: falling back to OLD font
> renderer:\n%s\n%s\n", body.c_str(), e.what());
> + textarea_.force_new_renderer(false);
> + textarea_.set_text(body);
> + }
> +
> +
> + // Add and configure the panels
> + main_box_.set_size(get_inner_w() - 3 * kPadding, get_inner_h() - 2 *
> kPadding);
> +
> + main_box_.add(&textarea_, UI::Box::Resizing::kExpandBoth);
> + main_box_.add(&button_box_, UI::Box::Resizing::kFullSize);
> + button_box_.add_inf_space();
> + button_box_.add(&ok_);
> + button_box_.add_inf_space();
> +
> + ok_.sigclicked.connect(boost::bind(&StoryMessageBox::clicked_ok,
> boost::ref(*this)));
> +
> + if (x == -1 && y == -1) {
> + center_to_parent();
> + }
> move_inside_parent();
> + textarea_.focus();
> }
>
> -/**
> - * Clicked
> - */
> void StoryMessageBox::clicked_ok() {
> + // Manually force the game to reevaluate its current state, especially
> time information.
> + game_->game_controller()->think();
> + // Now get the game running again.
> + game_->game_controller()->set_desired_speed(desired_speed_);
> + game_->save_handler().set_allow_saving(true);
> +
> end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
> }
>
> -/*
> - * Avoid being closed by right click
> - */
> bool StoryMessageBox::handle_mousepress(const uint8_t btn, int32_t mx,
> int32_t my) {
> if (btn == SDL_BUTTON_RIGHT)
> return true;
--
https://code.launchpad.net/~widelands-dev/widelands/story_message_box/+merge/335003
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/story_message_box into lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
