diff --git a/include/sfw/InputWidget.hpp b/include/sfw/InputWidget.hpp index 9c04ed2..5404593 100644 --- a/include/sfw/InputWidget.hpp +++ b/include/sfw/InputWidget.hpp @@ -7,18 +7,59 @@ namespace sfw { template class InputWidget : public Widget -/*============================================================================== - This "CRTP utility class" is an intermediate level in the widget class - hierarchy, solely to overcome some C++ limitations (i.e. to support templated - functions that would otherwise be unable to deduce their "actual widget type" - parameter (W)). - - NOTE: - "Interactive" widgets (that can have user callbacks, support the update(...) +/***************************************************************************** + Base class of widgets that are intended to receive user input (data) + + (This is implemented as a CRTP class solely to overcome a C++ limitation to + support templated functions that would otherwise be unable to deduce their + "actual widget" template parameter (W).) + + Notes on the "input thing": + + - Input widgets (that can have user callbacks, support the update(...) API etc.) should derive from this class (rather than Widget directly). -===============================================================================*/ + + - Every input widget has a dedicated value of a corresponding type that + the widget represents (models, stores, displays etc.). + + This value can be set via user input actions, or programmatically by the + constructor, and a dedicated set(...) method (and its overloads, if any). + + The current value can be queried with the widget's get() method. + (Unfortunately C++ doesn't support overloading on return types, so for + outward type conversions, separate getXxx() methods may be defined. + However, it's better to define the widget's data type explicitly, with + conversions in both directions (converting ctors and "cast operators"), + and then just use a single set/get method.) + + - Input widgets are typically configured to have callbacks to be invoked + when the widget's value gets updated (with the possibility of still + ignoring unimportant interim changes). +******************************************************************************/ { public: + InputWidget() : + m_changed(true) // Initializing counts as a change on its own right! + // (So, this ensures proper update followups even if + // an initial set() can't detect a change, e.g. because + // there's no "invalid" value in the type, or because + // set() ism't even called by the widget ctor (which is + // no longer assumed/required -- exactly because of this + // init safeguard...))! + {} + + // Tracking the value of the widget + // Derived real widgets will need to define getter/setters: + // - set(V value); // Will call changed() as applicable + // - V get(); + bool changed() const { return m_changed; } + Widget* setChanged(bool newstate = true) { m_changed = newstate; return this; } + //!!Rename this to a) not be so ambiguous with the above (const) one, and + //!!b) be in line with other similar ones (like enable/disable etc.)!... + //!!But change() here would be awkward -- even resorting to setChanged() + //!!feels better, but then it's out-of-style (for the new API) again...) + + template W* update(V value) { ((W*)this)->set(value); // set() is not virtual, must downcast to find it! (Also, `W::set(...)` could be too rigid, I think.) //!!NOTE: C++ won't find W::set() just via `set(value);` (even if no Widget::set())! :-/ @@ -43,7 +84,7 @@ template class InputWidget : public Widget //!! a) it can be configured "orthogonally", and b) we'd have //!! one less decision to make here. - changed(false); + setChanged(false); //!!?? We either clear the `changed` flag here, so that it won't get stuck (note: //!!?? set() isn't supposed to clear it, to allow batch updates!), and then it can't //!!?? be used to track an overarching "changed" state across these update callbacks, @@ -52,8 +93,8 @@ template class InputWidget : public Widget return (W*)this; } - //!! KINDA DEPRECATED...: - // Set callback for Update events + //!!SHOULD BE DEPRECATED, in favor of on(...): + // Set callback for the Updated events // 1. Applies to interactive (input) widgets only. // 2. The callback is not called for volatile interim state changes // (like typing/deleting chars to/from a text editor), but only when @@ -80,6 +121,39 @@ template class InputWidget : public Widget // { // return (SomeWidget*) Widget::setCallback(callback); // } + + //!!This actually belongs to InputWidget (`protected` exactly for it to be visible + //!!there), but not sure how a virtual would mix with that being a template!... + //!!Also, it's not even just a dummy callback like the others, but a dispatcher! :-/ + //!!Needs cleanup... + //!! -> Actually, as a generic "callback invoker", even up in EventHandler, like + //!! EventHandler::invoke_callback(which), could hit all the birds with one stone. +protected: + +// Virtuals/Callbacks -------------------------------------------------------- + virtual void onUpdated() //!! -> "EventHandler::invoke_callback(Updated)", and "updated" -> "notify(Updated)" or sg... + { + using namespace Event::internal; + + auto callback = m_callbackMap[Event::Update]; + assert( std::holds_alternative(callback) || std::holds_alternative(callback) ); + //!!?? This may no longer be true after getting something from the map that may not have + //!!?? been there! Seems to still work, so probably true, but make it explicit whether the + //!!?? map would indeed return a false std::function, properly wrapped in optional<>!... + + if (std::holds_alternative(callback) && std::get(callback)) + { + return (std::get(callback).value()) (this); + } + else if (std::holds_alternative(callback) && std::get(callback)) + { + return (std::get(callback).value()) (); + } + } + + +private: + bool m_changed = false; }; diff --git a/include/sfw/Widget.hpp b/include/sfw/Widget.hpp index 8164ea9..f30b3f3 100644 --- a/include/sfw/Widget.hpp +++ b/include/sfw/Widget.hpp @@ -32,24 +32,11 @@ class Tooltip; class GUI; /***************************************************************************** - Abstract base class for widgets - - - Every (interactive) widget has a dedicated value of a corresponding - type that the widget represents (models, stores, displays etc.). - - This value can be set via user input actions, or programmatically by the - constructor, and a dedicated set(...) method (and its overloads, if any). - - The current value can be queried with the widget's get() method. - (Unfortunately C++ doesn't support overloading on return types, so for - outward type conversions, separate getXxx() methods may be defined. - However, it's better to define the widget's data type explicitly, with - conversions in both directions (converting ctors and "cast operators"), - and then just use a single set/get method.) - - - Interactive widgets are typically configured to have callbacks to be - invoked when the widget's value gets updated (and finalized/committed, - so interim changes can be ignored). + + Abstract base for widgets + + See InputWidget as the base for "actually useful" widgets, though. ;) + *****************************************************************************/ class Widget : public gfx::Drawable, public Event::Handler { @@ -64,17 +51,6 @@ class Widget : public gfx::Drawable, public Event::Handler // Check if a point belongs to the widget bool contains(const sf::Vector2f& point) const; - // Tracking the value of the widget - // Derived real widgets will need to define getter/setters: - // - set(V value); // Will call changed() as applicable - // - V get(); - bool changed() const { return m_changed; } - Widget* changed(bool newstate = true) { m_changed = newstate; return this; } - //!!Rename this to a) not be so ambiguous with the above (const) one, and - //!!b) be in line with other similar ones (like enable/disable etc.)!... - //!!But change() here would be awkward -- even resorting to setChanged() - //!!feels better, but then it's out-of-style (for the new API) again...) - // Enable/disable processing (user) events // (Not just inputs, but also outputs like triggering user callbacks.) Widget* enable(bool state = true); @@ -185,15 +161,6 @@ friend class Tooltip; // just to access getMain() via Tooltip::m_owner->! virtual void onResized() {} virtual void onThemeChanged() {} - //!!This actually belongs to InputWidget (`protected` exactly for it to be visible - //!!there), but not sure how a virtual would mix with that being a template!... - //!!Also, it's not even just a dummy callback like the others, but a dispatcher! :-/ - //!!Needs cleanup... - //!! -> Actually, as a generic "callback invoker", even up in EventHandler, like - //!! EventHandler::invoke_callback(which), could hit all the birds with one stone. - protected: - virtual void onUpdated(); //!! -> "EventHandler::invoke_callback(Updated)", and "updated" -> "notify(Updated)" or sg... - private: WidgetContainer* m_parent = nullptr; Widget* m_previous = nullptr; @@ -201,7 +168,6 @@ friend class Tooltip; // just to access getMain() via Tooltip::m_owner->! bool m_focusable; WidgetState m_state; - bool m_changed; sf::Vector2f m_position; sf::Vector2f m_size; diff --git a/include/sfw/Widgets/OptionsBox.inl b/include/sfw/Widgets/OptionsBox.inl index f12ea91..68a2d27 100644 --- a/include/sfw/Widgets/OptionsBox.inl +++ b/include/sfw/Widgets/OptionsBox.inl @@ -62,6 +62,7 @@ template auto OptionsBox::set(size_t index) { if (index != m_currentIndex) { + this->setChanged(); //! this-> required here due to C++ cringe... update_selection(index); } return this; diff --git a/include/sfw/Widgets/Slider.hpp b/include/sfw/Widgets/Slider.hpp index aca4cfa..7fdc0de 100644 --- a/include/sfw/Widgets/Slider.hpp +++ b/include/sfw/Widgets/Slider.hpp @@ -68,7 +68,7 @@ class Slider: public InputWidget // Current value Slider* set(float value); float get() const; - // Unfortunately, these are also needed to seamlessly support integers, too, without warnings: + // Unfortunately, these are also needed to seamlessly support integers, too (without warnings): Slider* set(int value) { return set((float)value); } Slider* set(unsigned value) { return set((float)value); } diff --git a/src/lib/Widget.cpp b/src/lib/Widget.cpp index 4a1e617..46187d1 100644 --- a/src/lib/Widget.cpp +++ b/src/lib/Widget.cpp @@ -45,8 +45,7 @@ Widget* Widget::getWidget(const std::string& name) const //---------------------------------------------------------------------------- Widget::Widget() : m_focusable(true), - m_state(WidgetState::Default), - m_changed(false) + m_state(WidgetState::Default) { } @@ -57,7 +56,6 @@ Widget::Widget(Widget&& tmp) : m_next(tmp.m_next), m_focusable(tmp.m_focusable), m_state(tmp.m_state), - m_changed(tmp.m_changed), m_position(tmp.m_position), m_size(tmp.m_size), m_transform(tmp.m_transform) @@ -78,7 +76,6 @@ Widget::Widget(const Widget& other) : m_next(other.m_next), m_focusable(other.m_focusable), m_state(other.m_state), - m_changed(other.m_changed), m_position(other.m_position), m_size(other.m_size), m_transform(other.m_transform) @@ -341,29 +338,6 @@ const sf::Transform& Widget::getTransform() const } -// Virtuals/Callbacks -------------------------------------------------------- - -void Widget::onUpdated() -{ - using namespace Event::internal; - - auto callback = m_callbackMap[Event::Update]; - assert( std::holds_alternative(callback) || std::holds_alternative(callback) ); - //!!?? This may no longer be true after getting something from the map that may not have - //!!?? been there! Seems to still work, so probably true, but make it explicit whether the - //!!?? map would indeed return a false std::function, properly wrapped in optional<>!... - - if (std::holds_alternative(callback) && std::get(callback)) - { - return (std::get(callback).value()) (this); - } - else if (std::holds_alternative(callback) && std::get(callback)) - { - return (std::get(callback).value()) (); - } -} - - // Diagnostics --------------------------------------------------------------- #ifdef DEBUG diff --git a/src/lib/Widgets/Button.cpp b/src/lib/Widgets/Button.cpp index 43c68eb..68dc77b 100644 --- a/src/lib/Widgets/Button.cpp +++ b/src/lib/Widgets/Button.cpp @@ -44,7 +44,7 @@ Button* Button::click() { //! This weird kludge won't do antything to the visuals. (That would require //! way more hocus pocus, for just a little gimmick, so... perhaps later.) - changed(true); + setChanged(); updated(); return this; } diff --git a/src/lib/Widgets/CheckBox.cpp b/src/lib/Widgets/CheckBox.cpp index 2b41da0..dc4574d 100644 --- a/src/lib/Widgets/CheckBox.cpp +++ b/src/lib/Widgets/CheckBox.cpp @@ -28,7 +28,7 @@ CheckBox::CheckBox(std::function callback, bool checked): CheckBox* CheckBox::set(bool checked) { m_checked = checked; - changed(true); + setChanged(); return this; } diff --git a/src/lib/Widgets/Slider.cpp b/src/lib/Widgets/Slider.cpp index 97e1e70..f95149e 100644 --- a/src/lib/Widgets/Slider.cpp +++ b/src/lib/Widgets/Slider.cpp @@ -120,7 +120,11 @@ Slider* Slider::setRange(float min, float max, float step) Slider* Slider::set(float value) // Snaps to the closest multiple of step. (Remember: step may be non-integer!) { - //!! Disabling this until clarifying the "enigma" in #317! :) + //!!---------------------------------------------------------------------------- + //!! THIS IS NOW DONE, BUT LEAVING IT STILL HERE AS A MEMENTO... + //!!---------------------------------------------------------------------------- + //!! + //!! Disabling this if(...) until clarifying the "enigma" in #317! :) //!! ...And until finding a clean way to get this back, while also ensuring //!! the an initial set() would count as a real change() (no matter what //!! a C++ default-inited value would be! -> Universally, for each widget! @@ -128,7 +132,12 @@ Slider* Slider::set(float value) //!! should be taken care of during init (construction) independently from //!! set()? But that'd be yet another explicit widget init chore then... :-/ //!! - //!!if (value != m_value) // Really changing? + //!!---------------------------------------------------------------------------- + //!! (The -- trivial, in hindsight... also, kinda ingenious :) -- solution was + //!! just to init m_changed with true, instead of false in InputWidget!) + //!!---------------------------------------------------------------------------- + if (value != m_value) // Really changing? + //! NOTE: No early return, as we may have things to do even if this is false! //!!FIX: But if never actually changed (like ctor-init to 0...), then updateView, setTooltip etc. will never be called?! { //cerr << "set("<< value <<")... [" << range().min << ".." << range().max << "], step: "<< step() < Undo/Redo) + setChanged(); //!! Will become more sophisticated with Undo/Redo etc. return this; } @@ -506,6 +506,8 @@ void TextBox::onKeyPressed(const sf::Event::KeyEvent& key) // "Apply" case sf::Keyboard::Enter: + setChanged(); //!! None of the inidividual editing actions update this yet, + //!! so we just force it here for the time being... updated(); //!!TBD: Should it be forced even if !changed()? break;