End of buttons refactoring:

- cleanup of slider API
- documentation fixes

client/CPreGame.cpp

@@ -1286,7 +1286,7 @@ SelectionTab::SelectionTab(CMenuScreen::EState Type, const std::function<void(CM
 		new CButton(Point(55, 86), "CamCusL.DEF", CButton::tooltip(), boost::bind(&SelectionTab::sortBy, this, _name)); //by name
 	}
 
-	slider = new CSlider(372, 86, tabType != CMenuScreen::saveGame ? 480 : 430, boost::bind(&SelectionTab::sliderMove, this, _1), positions, curItems.size(), 0, false, 1);
+	slider = new CSlider(Point(372, 86), tabType != CMenuScreen::saveGame ? 480 : 430, boost::bind(&SelectionTab::sliderMove, this, _1), positions, curItems.size(), 0, false, CSlider::BLUE);
 	slider->addUsedEvents(WHEEL);
 	format =  CDefHandler::giveDef("SCSELC.DEF");
 
@@ -1353,16 +1353,16 @@ void SelectionTab::select( int position )
 	if(!curItems.size()) return;
 
 	// New selection. py is the index in curItems.
-	int py = position + slider->value;
+	int py = position + slider->getValue();
 	vstd::amax(py, 0);
 	vstd::amin(py, curItems.size()-1);
 
 	selectionPos = py;
 
 	if(position < 0)
-		slider->moveTo(slider->value + position);
+		slider->moveBy(position);
 	else if(position >= positions)
-		slider->moveTo(slider->value + position - positions + 1);
+		slider->moveBy(position - positions + 1);
 
 	if(txt)
 	{
@@ -1376,7 +1376,7 @@ void SelectionTab::select( int position )
 
 void SelectionTab::selectAbs( int position )
 {
-	select(position - slider->value);
+	select(position - slider->getValue());
 }
 
 int SelectionTab::getPosition( int x, int y )
@@ -1399,7 +1399,7 @@ void SelectionTab::sliderMove( int slidPos )
 void SelectionTab::printMaps(SDL_Surface *to)
 {
 
-	int elemIdx = slider->value;
+	int elemIdx = slider->getValue();
 
 	// Display all elements if there's enough space
 	//if(slider->amount < slider->capacity)
@@ -1557,15 +1557,15 @@ void SelectionTab::keyPressed( const SDL_KeyboardEvent & key )
 		moveBy = +positions-1;
 		break;
 	case SDLK_HOME:
-		select(-slider->value);
+		select(-slider->getValue());
 		return;
 	case SDLK_END:
-		select(curItems.size() - slider->value);
+		select(curItems.size() - slider->getValue());
 		return;
 	default:
 		return;
 	}
-	select(selectionPos - slider->value + moveBy);
+	select(selectionPos - slider->getValue() + moveBy);
 }
 
 void SelectionTab::onDoubleClick()
@@ -2243,7 +2243,7 @@ OptionsTab::OptionsTab():
 	pos = bg->pos;
 
 	if(SEL->screenType == CMenuScreen::newGame)
-		turnDuration = new CSlider(55, 551, 194, boost::bind(&OptionsTab::setTurnLength, this, _1), 1, 11, 11, true, 1);
+		turnDuration = new CSlider(Point(55, 551), 194, boost::bind(&OptionsTab::setTurnLength, this, _1), 1, 11, 11, true, CSlider::BLUE);
 }
 
 OptionsTab::~OptionsTab()
@@ -2262,7 +2262,7 @@ void OptionsTab::showAll(SDL_Surface * to)
 	printAtMiddleWBLoc(CGI->generaltexth->allTexts[520], 349, 110, FONT_SMALL, 70, Colors::YELLOW, to); //Starting Bonus
 	printAtMiddleLoc(CGI->generaltexth->allTexts[521], 222, 538, FONT_SMALL, Colors::YELLOW, to); // Player Turn Duration
 	if (turnDuration)
-		printAtMiddleLoc(CGI->generaltexth->turnDurations[turnDuration->value], 319,559, FONT_SMALL, Colors::WHITE, to);//Turn duration value
+		printAtMiddleLoc(CGI->generaltexth->turnDurations[turnDuration->getValue()], 319,559, FONT_SMALL, Colors::WHITE, to);//Turn duration value
 }
 
 void OptionsTab::nextCastle( PlayerColor player, int dir )

client/widgets/Buttons.cpp

@@ -393,7 +393,10 @@ void CToggleButton::clickLeft(tribool down, bool previousState)
 	if(previousState)//mouse up
 	{
 		if(down == false && getState() == PRESSED && canActivate())
+		{
+			onButtonClicked();
 			setSelected(!selected);
+		}
 		else
 			doSelect(selected); // restore
 	}
@@ -499,13 +502,22 @@ void CSlider::mouseMoved (const SDL_MouseMotionEvent & sEvent)
 	if(v!=value)
 	{
 		moveTo(v);
-		redrawSlider();
 	}
 }
 
-void CSlider::redrawSlider()
+void CSlider::setScrollStep(int to)
+{
+	scrollStep = to;
+}
+
+int CSlider::getAmount()
 {
-	//slider->show(screenBuf);
+	return amount;
+}
+
+int CSlider::getValue()
+{
+	return value;
 }
 
 void CSlider::moveLeft()
@@ -518,21 +530,18 @@ void CSlider::moveRight()
 	moveTo(value+1);
 }
 
-void CSlider::moveTo(int to)
+void CSlider::moveBy(int amount)
 {
-	vstd::amax(to, 0);
-	vstd::amin(to, positions);
-
-	//same, old position?
-	if(value == to)
-		return;
+	moveTo(value + amount);
+}
 
-	value = to;
+void CSlider::updateSliderPos()
+{
 	if(horizontal)
 	{
 		if(positions)
 		{
-			double part = static_cast<double>(to) / positions;
+			double part = static_cast<double>(value) / positions;
 			part*=(pos.w-48);
 			int newPos = part + pos.x + 16 - slider->pos.x;
 			slider->moveBy(Point(newPos, 0));
@@ -544,7 +553,7 @@ void CSlider::moveTo(int to)
 	{
 		if(positions)
 		{
-			double part = static_cast<double>(to) / positions;
+			double part = static_cast<double>(value) / positions;
 			part*=(pos.h-48);
 			int newPos = part + pos.y + 16 - slider->pos.y;
 			slider->moveBy(Point(0, newPos));
@@ -552,9 +561,21 @@ void CSlider::moveTo(int to)
 		else
 			slider->moveTo(Point(pos.x, pos.y+16));
 	}
+}
 
-	if(moved)
-		moved(to);
+void CSlider::moveTo(int to)
+{
+	vstd::amax(to, 0);
+	vstd::amin(to, positions);
+
+	//same, old position?
+	if(value == to)
+		return;
+	value = to;
+
+	updateSliderPos();
+
+	moved(to);
 }
 
 void CSlider::clickLeft(tribool down, bool previousState)
@@ -590,11 +611,12 @@ CSlider::~CSlider()
 
 }
 
-CSlider::CSlider(int x, int y, int totalw, std::function<void(int)> Moved, int Capacity, int Amount, int Value, bool Horizontal, int style):
+CSlider::CSlider(Point position, int totalw, std::function<void(int)> Moved, int Capacity, int Amount, int Value, bool Horizontal, CSlider::EStyle style):
     capacity(Capacity),
+    horizontal(Horizontal),
     amount(Amount),
+    value(Value),
     scrollStep(1),
-    horizontal(Horizontal),
     moved(Moved)
 {
 	OBJ_CONSTRUCTION_CAPTURING_ALL;
@@ -603,10 +625,10 @@ CSlider::CSlider(int x, int y, int totalw, std::function<void(int)> Moved, int C
 	addUsedEvents(LCLICK | KEYBOARD | WHEEL);
 	strongInterest = true;
 
-	pos.x += x;
-	pos.y += y;
+	pos.x += position.x;
+	pos.y += position.y;
 
-	if(style == 0)
+	if(style == BROWN)
 	{
 		std::string name = horizontal?"IGPCRDIV.DEF":"OVBUTN2.DEF";
 		//NOTE: this images do not have "blocked" frames. They should be implemented somehow (e.g. palette transform or something...)
@@ -650,8 +672,7 @@ CSlider::CSlider(int x, int y, int totalw, std::function<void(int)> Moved, int C
 		pos.h = totalw;
 	}
 
-	value = -1;
-	moveTo(Value);
+	updateSliderPos();
 }
 
 void CSlider::block( bool on )

client/widgets/Buttons.h

@@ -62,29 +62,36 @@ public:
 private:
 	std::vector<std::string> imageNames;//store list of images that can be used by this button
 	size_t currentImage;
+
 	ButtonState state;//current state of button from enum
 
 	std::array<int, 4> stateToIndex; // mapping of button state to index of frame in animation
-	std::array<std::string, 4> hoverTexts; //text for statusbar
+	std::array<std::string, 4> hoverTexts; //texts for statusbar, if empty - first entry will be used
 	std::string helpBox; //for right-click help
 
 	CAnimImage * image; //image for this button
-	CIntObject * overlay;//object-overlay
+	CIntObject * overlay;//object-overlay, can be null
+
 protected:
 	void onButtonClicked(); // calls callback
 	void update();//to refresh button after image or text change
 
+	// internal method to change state. Public change can be done only via block()
 	void setState(ButtonState newState);
 	ButtonState getState();
 
 public:
 	bool actOnDown,//runs when mouse is pressed down over it, not when up
-		hoverable,//if true, button will be highlighted when hovered
+		hoverable,//if true, button will be highlighted when hovered (e.g. main menu)
 		soundDisabled;
 
+	// if set, button will have 1-px border around it with this color
 	boost::optional<SDL_Color> borderColor;
 
+	/// adds one more callback to on-click actions
 	void addCallback(std::function<void()> callback);
+
+	/// adds overlay on top of button image. Only one overlay can be active at once
 	void addOverlay(CIntObject * newOverlay);
 	void addTextOverlay(const std::string &Text, EFonts font, SDL_Color color = Colors::WHITE);
 
@@ -113,6 +120,7 @@ public:
 	void hover (bool on) override;
 	void showAll(SDL_Surface * to) override;
 
+	/// generates tooltip that can be passed into constructor
 	static std::pair<std::string, std::string> tooltip();
 	static std::pair<std::string, std::string> tooltip(const JsonNode & localizedTexts);
 	static std::pair<std::string, std::string> tooltip(const std::string & hover, const std::string & help = "");
@@ -125,17 +133,20 @@ protected:
 
 	bool selected;
 
+	// internal method for overrides
 	virtual void doSelect(bool on);
 
 	// returns true if toggle can change its state
 	bool canActivate();
 
 public:
+	/// if set to false - button can not be deselected normally
 	bool allowDeselection;
 
 	CToggleBase(CFunctionList<void(bool)> callback);
 	virtual ~CToggleBase();
 
+	/// Changes selection to "on", and calls callback
 	void setSelected(bool on);
 
 	void addCallback(std::function<void(bool)> callback);
@@ -175,7 +186,10 @@ public:
 	CToggleGroup(const CFunctionList<void(int)> & OnChange, bool musicLikeButtons = false);
 
 	void addCallback(std::function<void(int)> callback);
+
+	/// add one toggle/button into group
 	void addToggle(int index, CToggleBase * button);
+	/// Changes selection to specific value. Will select toggle with this ID, if present
 	void setSelected(int id);
 
 	void show(SDL_Surface * to);
@@ -185,35 +199,63 @@ public:
 /// A typical slider which can be orientated horizontally/vertically.
 class CSlider : public CIntObject
 {
-public:
 	CButton *left, *right, *slider; //if vertical then left=up
 	int capacity;//how many elements can be active at same time (e.g. hero list = 5)
-	int amount; //total amount of elements (e.g. hero list = 0-8)
 	int positions; //number of highest position (0 if there is only one)
-	int value; //first active element
-	int scrollStep; // how many elements will be scrolled via one click, default = 1
 	bool horizontal;
 	bool wheelScrolling;
 	bool keyScrolling;
 
-	std::function<void(int)> moved;
+	int amount; //total amount of elements (e.g. hero list = 0-8)
+	int value; //first active element
+	int scrollStep; // how many elements will be scrolled via one click, default = 1
+	CFunctionList<void(int)> moved;
 
-	void redrawSlider(); 
+	void updateSliderPos();
 	void sliderClicked();
+
+public:
+	enum EStyle {
+		BROWN,
+		BLUE
+	};
+
+	void block(bool on);
+
+	/// Controls how many items wil be scrolled via one click
+	void setScrollStep(int to);
+
+	/// Value modifiers
 	void moveLeft();
 	void moveRight();
-	void moveTo(int to);
-	void block(bool on);
+	void moveTo(int value);
+	void moveBy(int amount);
+	void moveToMax();
+
+	/// Amount modifier
 	void setAmount(int to);
 
+	/// Accessors
+	int getAmount();
+	int getValue();
+
+	void addCallback(std::function<void(int)> callback);
+
 	void keyPressed(const SDL_KeyboardEvent & key);
 	void wheelScrolled(bool down, bool in);
 	void clickLeft(tribool down, bool previousState);
 	void mouseMoved (const SDL_MouseMotionEvent & sEvent);
 	void showAll(SDL_Surface * to);	
 
-	CSlider(int x, int y, int totalw, std::function<void(int)> Moved, int Capacity, int Amount, 
-		int Value=0, bool Horizontal=true, int style = 0); //style 0 - brown, 1 - blue
+	/**
+	 * @param position, coordinates of slider
+	 * @param length, length of slider ribbon, including left/right buttons
+	 * @param Moved, function that will be called whenever slider moves
+	 * @param Capacity, maximal number of visible at once elements
+	 * @param Amount, total amount of elements, including not visible
+	 * @param Value, starting position
+	 */
+	CSlider(Point position, int length, std::function<void(int)> Moved, int Capacity, int Amount,
+		int Value=0, bool Horizontal=true, EStyle style = BROWN);
 	~CSlider();
-	void moveToMax();
 };

client/widgets/ObjectLists.cpp

@@ -96,8 +96,8 @@ CListBox::CListBox(CreateFunc create, DestroyFunc destroy, Point Pos, Point Item
 	if (Slider & 1)
 	{
 		OBJ_CONSTRUCTION_CAPTURING_ALL;
-		slider = new CSlider(SliderPos.x, SliderPos.y, SliderPos.w, boost::bind(&CListBox::moveToPos, this, _1),
-			VisibleSize, TotalSize, InitialPos, Slider & 2, Slider & 4);
+		slider = new CSlider(SliderPos.topLeft(), SliderPos.w, boost::bind(&CListBox::moveToPos, this, _1),
+			VisibleSize, TotalSize, InitialPos, Slider & 2, Slider & 4 ? CSlider::BLUE : CSlider::BROWN);
 	}
 	reset();
 }

client/widgets/TextControls.cpp

@@ -295,9 +295,9 @@ void CTextBox::setText(const std::string &text)
 		label->setText(text);
 
 		OBJ_CONSTRUCTION_CAPTURING_ALL;
-		slider = new CSlider(pos.w - 32, 0, pos.h, boost::bind(&CTextBox::sliderMoved, this, _1),
-		                     label->pos.h, label->textSize.y, 0, false, sliderStyle);
-		slider->scrollStep = graphics->fonts[label->font]->getLineHeight();
+		slider = new CSlider(Point(pos.w - 32, 0), pos.h, boost::bind(&CTextBox::sliderMoved, this, _1),
+		                     label->pos.h, label->textSize.y, 0, false, CSlider::EStyle(sliderStyle));
+		slider->setScrollStep(graphics->fonts[label->font]->getLineHeight());
 	}
 }
 

client/windows/CQuestLog.cpp

@@ -132,7 +132,7 @@ void CQuestLog::init()
 	ok = new CButton(Point(547, 401), "IOKAY.DEF", CGI->generaltexth->zelp[445], boost::bind(&CQuestLog::close,this), SDLK_RETURN);
 
 	if (quests.size() > QUEST_COUNT)
-		slider = new CSlider(203, 199, 230, boost::bind (&CQuestLog::sliderMoved, this, _1), QUEST_COUNT, quests.size(), false, 0);
+		slider = new CSlider(Point(203, 199), 230, boost::bind (&CQuestLog::sliderMoved, this, _1), QUEST_COUNT, quests.size(), false);
 
 	for (int i = 0; i < quests.size(); ++i)
 	{

client/windows/CTradeWindow.cpp

@@ -702,8 +702,7 @@ CMarketplaceWindow::CMarketplaceWindow(const IMarket *Market, const CGHeroInstan
 
 	if(sliderNeeded)
 	{
-		slider = new CSlider(231,490,137,nullptr,0,0);
-		slider->moved = boost::bind(&CMarketplaceWindow::sliderMoved,this,_1);
+		slider = new CSlider(Point(231, 490),137, boost::bind(&CMarketplaceWindow::sliderMoved,this,_1),0,0);
 		max = new CButton(Point(229, 520), "IRCBTNS.DEF", CGI->generaltexth->zelp[596], [&] { setMax(); });
 		max->block(true);
 	}
@@ -793,7 +792,7 @@ void CMarketplaceWindow::makeDeal()
 {
 	int sliderValue = 0;
 	if(slider)
-		sliderValue = slider->value;
+		sliderValue = slider->getValue();
 	else
 		sliderValue = !deal->isBlocked(); //should always be 1
 
@@ -816,7 +815,7 @@ void CMarketplaceWindow::makeDeal()
 
 	if(slider)
 	{
-		LOCPLINT->cb->trade(market->o, mode, leftIdToSend, hRight->id, slider->value*r1, hero);
+		LOCPLINT->cb->trade(market->o, mode, leftIdToSend, hRight->id, slider->getValue()*r1, hero);
 		slider->moveTo(0);
 	}
 	else
@@ -933,7 +932,7 @@ std::string CMarketplaceWindow::selectionSubtitle(bool Left) const
 		case CREATURE:
 			{
 				int val = slider
-					? slider->value * r1
+					? slider->getValue() * r1
 					: (((deal->isBlocked())) ? 0 : r1);
 
 				return boost::lexical_cast<std::string>(val);
@@ -948,7 +947,7 @@ std::string CMarketplaceWindow::selectionSubtitle(bool Left) const
 		{
 		case RESOURCE:
 			if(slider)
-				return boost::lexical_cast<std::string>( slider->value * r2 );
+				return boost::lexical_cast<std::string>( slider->getValue() * r2 );
 			else
 				return boost::lexical_cast<std::string>(r2);
 		case ARTIFACT_TYPE:
@@ -1109,8 +1108,7 @@ CAltarWindow::CAltarWindow(const IMarket *Market, const CGHeroInstance *Hero /*=
 		 //To sacrifice creatures, move them from your army on to the Altar and click Sacrifice
 		new CTextBox(CGI->generaltexth->allTexts[480], Rect(320, 56, 256, 40), 0, FONT_SMALL, CENTER, Colors::YELLOW);
 
-		slider = new CSlider(231,481,137,nullptr,0,0);
-		slider->moved = boost::bind(&CAltarWindow::sliderMoved,this,_1);
+		slider = new CSlider(Point(231,481),137,boost::bind(&CAltarWindow::sliderMoved,this,_1),0,0);
 		max = new CButton(Point(147, 520), "IRCBTNS.DEF", CGI->generaltexth->zelp[578], boost::bind(&CSlider::moveToMax, slider));
 
 		sacrificedUnits.resize(GameConstants::ARMY_SIZE, 0);
@@ -1200,7 +1198,7 @@ void CAltarWindow::makeDeal()
 	if(mode == EMarketMode::CREATURE_EXP)
 	{
 		blockTrade();
-		slider->value = 0;
+		slider->moveTo(0);
 
 		std::vector<int> toSacrifice = sacrificedUnits;
 		for (int i = 0; i < toSacrifice.size(); i++)
@@ -1292,9 +1290,9 @@ void CAltarWindow::selectionChanged(bool side)
 			stackCount++;
 
 	slider->setAmount(hero->getStackCount(SlotID(hLeft->serial)) - (stackCount == 1));
-	slider->block(!slider->amount);
-	slider->value = sacrificedUnits[hLeft->serial];
-	max->block(!slider->amount);
+	slider->block(!slider->getAmount());
+	slider->moveTo(sacrificedUnits[hLeft->serial]);
+	max->block(!slider->getAmount());
 	readyToTrade = true;
 	redraw();
 }
@@ -1323,7 +1321,7 @@ Point CAltarWindow::selectionOffset(bool Left) const
 std::string CAltarWindow::selectionSubtitle(bool Left) const
 {
 	if(Left && slider && hLeft)
-		return boost::lexical_cast<std::string>(slider->value);
+		return boost::lexical_cast<std::string>(slider->getValue());
 	else if(!Left && hRight)
 		return hRight->subtitle;
 	else

client/windows/GUIClasses.cpp

@@ -184,7 +184,7 @@ void CRecruitmentWindow::select(CCreatureCard *card)
 
 		slider->setAmount(maxAmount);
 
-		if(slider->value != maxAmount)
+		if(slider->getValue() != maxAmount)
 			slider->moveTo(maxAmount);
 		else // if slider already at 0 - emulate call to sliderMoved()
 			sliderMoved(maxAmount);
@@ -214,7 +214,7 @@ void CRecruitmentWindow::buy()
 		if(dst->ID == Obj::HERO)
 		{
 			txt = CGI->generaltexth->allTexts[425]; //The %s would join your hero, but there aren't enough provisions to support them.
-			boost::algorithm::replace_first(txt, "%s", slider->value > 1 ? CGI->creh->creatures[crid]->namePl : CGI->creh->creatures[crid]->nameSing);
+			boost::algorithm::replace_first(txt, "%s", slider->getValue() > 1 ? CGI->creh->creatures[crid]->namePl : CGI->creh->creatures[crid]->nameSing);
 		}
 		else
 		{
@@ -225,7 +225,7 @@ void CRecruitmentWindow::buy()
 		return;
 	}
 
-	onRecruit(crid, slider->value);
+	onRecruit(crid, slider->getValue());
 	if(level >= 0)
 		close();
 }
@@ -261,8 +261,7 @@ CRecruitmentWindow::CRecruitmentWindow(const CGDwelling *Dwelling, int Level, co
 	OBJ_CONSTRUCTION_CAPTURING_ALL;
 	new CGStatusBar(new CPicture(*background, Rect(8, pos.h - 26, pos.w - 16, 19), 8, pos.h - 26));
 
-	slider = new CSlider(176,279,135,nullptr,0,0,0,true);
-	slider->moved = boost::bind(&CRecruitmentWindow::sliderMoved,this, _1);
+	slider = new CSlider(Point(176,279),135,boost::bind(&CRecruitmentWindow::sliderMoved,this, _1),0,0,0,true);
 
 	maxButton = new CButton(Point(134, 313), "IRCBTNS.DEF", CGI->generaltexth->zelp[553], boost::bind(&CSlider::moveToMax,slider), SDLK_m);
 	buyButton = new CButton(Point(212, 313), "IBY6432.DEF", CGI->generaltexth->zelp[554], boost::bind(&CRecruitmentWindow::buy,this), SDLK_RETURN);
@@ -341,10 +340,10 @@ void CRecruitmentWindow::availableCreaturesChanged()
 	//restore selection
 	select(cards[selectedIndex]);
 
-	if(slider->value == slider->amount)
-		slider->moveTo(slider->amount);
+	if(slider->getValue() == slider->getAmount())
+		slider->moveToMax();
 	else // if slider already at 0 - emulate call to sliderMoved()
-		sliderMoved(slider->amount);
+		sliderMoved(slider->getAmount());
 }
 
 void CRecruitmentWindow::sliderMoved(int to)
@@ -393,7 +392,7 @@ CSplitWindow::CSplitWindow(const CCreature * creature, std::function<void(int, i
 	animLeft = new CCreaturePic(20, 54, creature, true, false);
 	animRight = new CCreaturePic(177, 54,creature, true, false);
 
-	slider = new CSlider(21, 194, 257, boost::bind(&CSplitWindow::sliderMoved, this, _1), 0, sliderPositions, rightAmount - rightMin, true);
+	slider = new CSlider(Point(21, 194), 257, boost::bind(&CSplitWindow::sliderMoved, this, _1), 0, sliderPositions, rightAmount - rightMin, true);
 
 	std::string title = CGI->generaltexth->allTexts[256];
 	boost::algorithm::replace_first(title,"%s", creature->namePl);