Page 17 of 17

Re: Gspöt - retained GUI lib

Posted: Fri Feb 12, 2021 5:04 am
by Anase Skyrider
Visual behavior for buttons is a bit weird and unintuitive and lacks any detection of "being mouseddown" in collaboration with the new mouserelease version of click. I've modified the Gspot.button.draw function as so (feel free to collapse the indentation):

Code: Select all

draw = function(this, pos)
	if this.parent and this.value ~= nil and this.value == this.parent.value then
		if this == this.Gspot.mousein then
			if love.mouse.isDown(1) or love.mouse.isDown(2) then
				setColor(this.style.default)
			else
				setColor(this.style.focus)
			end
		else
			setColor(this.style.hilite)
		end
	else
		if this == this.Gspot.mousein then
			if love.mouse.isDown(1) or love.mouse.isDown(2) then
				setColor(this.style.focus)
			else
				setColor(this.style.hilite)
			end
		else
			setColor(this.style.default)
		end
	end
	-- insert the rest of the code here
end
For regular buttons:
- default = style.default
- mouseover = style.hilite
- mousedown = style.focus

For a toggled button (like Option):
- default = style.hilite
- mouseover = style.focus
- mousedown = style.default

I've added a check for "this.value ~= nil" to fix (what I believe to be) the bug that causes regular buttons to change their color behavior when they are parented to another object. Only buttons which have an explicit this.value property will change their color behavior.

Re: Gspöt - retained GUI lib

Posted: Fri Feb 12, 2021 5:56 am
by Anase Skyrider
I've created a new Gspot element type called textbutton which utilizes the above attempt-at-programming and applies it to text elements -- for all your text-only-button goodness. The number of work-arounds needed to get this to work with a regular button basically meant rewriting the draw code, which at that rate, it was easier to just reuse the prior if-thens and a text element and just make a whole new subtype.

Feed any non-false (easiest is just "true") for option and then your value for the value parameter, and it will work just like an option button.

Code: Select all

Gspot.textbutton = {
	load = function(this, Gspot, label, pos, parent, autosize, option, value)
		local element = Gspot:text(label, pos, parent, autosize)
		if option then
			element.value = value
			element.click = function(this) this.parent.value = this.value end
		end
		element.draw = function(this, pos)
			if this.parent and this.value ~= nil and this.value == this.parent.value then
				if this == this.Gspot.mousein then
					if love.mouse.isDown(1) or love.mouse.isDown(2) then
						setColor(this.style.labelfg or this.style.fg)
					else
						setColor(this.style.focus)
					end
				else
					setColor(this.style.hilite)
				end
			else
				if this == this.Gspot.mousein then
					if love.mouse.isDown(1) or love.mouse.isDown(2) then
						setColor(this.style.focus)
					else
						setColor(this.style.hilite)
					end
				else
					setColor(this.style.labelfg or this.style.fg)
				end
			end
			if this.autosize then lgprint(this.label, pos.x + (this.style.unit / 4), pos.y + (this.style.unit / 8))
			else lgprintf(this.label, pos.x + (this.style.unit / 4), pos.y + (this.style.unit / 8), (this.autosize and pos.w) or pos.w - (this.style.unit / 2), 'left') end
		end
		return element
	end,
}
setmetatable(Gspot.textbutton, {__index = Gspot.util, __call = Gspot.textbutton.load})

Re: Gspöt - retained GUI lib

Posted: Fri Feb 12, 2021 6:53 pm
by pgimeno
Anase Skyrider wrote: Fri Feb 12, 2021 5:04 am I've added a check for "this.value ~= nil" to fix (what I believe to be) the bug that causes regular buttons to change their color behavior when they are parented to another object. Only buttons which have an explicit this.value property will change their color behavior.
Can you provide a test case that shows this problem? I don't see anything wrong in the default example.

Please note also point 7 here: https://love2d.org/forums/viewtopic.php?f=3&t=64151 - personally I don't mind multi-posting but it seems to piss off some forum users, and it's a rule, so...

Re: Gspöt - retained GUI lib

Posted: Sat Feb 13, 2021 4:12 am
by Anase Skyrider
pgimeno wrote: Fri Feb 12, 2021 6:53 pm Can you provide a test case that shows this problem? I don't see anything wrong in the default example.
You can see it in the demo, though. Parented buttons default to hilite and hover to focus, whereas regular buttons default to default and hover to hilite.
pgimeno wrote: Fri Feb 12, 2021 6:53 pm Please note also point 7 here:
Edit-button wasn't available an hour later when I decided to add, test, and write-up about the textbutton element I thought would be a useful addition. Wasn't gonna wait until you responded just to post that. /shrug.

[EDIT on the next day: It's because I wasn't logged in. LOVE's forums periodically log you out or something, it makes writing posts somewhat annoying. Apologies, I didn't know I could still edit it.]

Re: Gspöt - retained GUI lib

Posted: Sat Feb 13, 2021 11:27 am
by pgimeno
I still don't understand. The "Speak" button has the "Chat" input element as parent, while the "A Button" button has no parent, and I don't see a difference between them.

Re: Gspöt - retained GUI lib

Posted: Sat Feb 13, 2021 8:52 pm
by Anase Skyrider
pgimeno wrote: Sat Feb 13, 2021 11:27 am I still don't understand. The "Speak" button has the "Chat" input element as parent, while the "A Button" button has no parent, and I don't see a difference between them.
[EDIT: You only looked at those two buttons. There're other buttons, like the X buttons in the boxes, that are clearly brighter than "A Button" and "Speak" despite not being recolored in the code.]

Recall that the full, original if-condition is

Code: Select all

if this.parent and this.value == this.parent.value then
Now look at this from the gspot.input.load:

Code: Select all

element.value = (value and tostring(value)) or ''
The Speak button is not working because it checks if this.value == this.parent.value and its parent is an input with a .value of "", and this.value defaults to nil, so they don't equal. And `nil == nil` returns true. So when you have a frame with a nil .value that's parented to a frame with a nil .value, it will cause it to recolor.

You can test this very easily, and I've attached the quick test I mocked up below. You could also do what I did before I reported the problem and the solution, which was to create some buttons and mess with their parenting after changing the .default to {1,0,0,1}, the .hilite to {0,1,0,1}, and the .focus to {0,0,1,1} just to see how the colors are interacted with.
test.zip
(16.47 KiB) Downloaded 307 times

Re: Gspöt - retained GUI lib

Posted: Sun Feb 14, 2021 12:07 pm
by pgimeno
Thanks for the test case. I see the effect you mention. It's hard to know whether this is intentional, because the original author is no longer around to ask, but in any case, it's clear that changing the behaviour is going to affect existing applications, including the demo, so I'm disinclined to apply the patch in the form that you've suggested. Do you have some other suggestion that keeps backwards compatibility?

Re: Gspöt - retained GUI lib

Posted: Wed Feb 17, 2021 2:41 am
by Anase Skyrider
pgimeno wrote: Sun Feb 14, 2021 12:07 pm Do you have some other suggestion that keeps backwards compatibility?
In my opinion, it's a feature not worth keeping nor making backwards compatible, but... If you're gonna do it, you'd need to add a bool that uses the default functionality on false/nil, and not increment the colors for parented buttons on true. EDIT2: If you want *me* to write that code, I have no problem with that.

You could still add the depress coloring (seriously, buttons that don't change color when you mousedown? Pfft, that's crazy :P), just use the bool for whether or not parented buttons will shift the color-states used.

You *could* forgo the bool and just make it a thing that happens on frame creation -- if it has a parent, then shift the color states -- and then the developer can "avoid" it by setting the parent after creating it, or perhaps by caching the default colors for an unparented button and then setting the colors afterwards, but that would remove the color-incrementing behavior occurring dynamically when the button is parented or unparented.

Regardless of how or whether you keep it backwards compatible, having three colors to use for default+hover+click is a good idea.

EDIT1: Since double-posting is frowned upon, here's a massive edit.

Gspot:image elements cannot be scaled. If you supply a width/height on element-creation, it's overwritten: pos.w = img:getWidth(), and pos.h = img:getHeight(). drawimg() function still uses pos.<dimension> and img:get<Dimension> for the offsets. For Gspot:image elements, they equal each other, so the net offset is 0 (it adds and then subtracts itself). The only reason it does this: the `button` element can have its .img field supplied with a love.image object to add a centered image to the center of the button. That's it, it's the only object with this property.

So to add image scaling to Gspot, I changed imgdraw(), Gspot.image.load(), and Gpot.button.draw(). This is backwards compatible with old code. If you give a Gspot:image element a width and/or height, it will rescale to match. It also can take a pos.r parameter to rotate itself about its top-left anchor-point (this is exclusive to Gspot:image elements because I don't know what it would do otherwise). Here's the changes, with comments explaining the changes.

Code: Select all

Gspot.image = {
	-- other code
	load = function(this, Gspot, label, pos, parent, img)
		local width, height = pos.w, pos.h -- cache width/height if supplied
		local element = Gspot:element('image', label, pos, parent)
		element:setimage(img) -- sets width and height to image width and height
		if width then element.pos.w = width end -- overwrite object width
		if height then element.pos.h = height end -- overwrite object height
		return Gspot:add(element)
	end,
	-- other code
}

Gspot.util = {
	-- other code
	drawimg = function(this, pos, isNotScalable)
		local r, g, b, a = love.graphics.getColor()
		setColor(255, 255, 255, 255)
		if isNotScalable == true then -- if this bool is true, it will center the image relative to its pos.w and pos.h
			love.graphics.draw(this.img, (pos.x + (pos.w / 2)) - (this.img:getWidth() / 2), (pos.y + (pos.h / 2)) - (this.img:getHeight() / 2))
		else -- if this bool is false, pos.w and pos.h are used to rescale the image to match the supplied dimensions
			love.graphics.draw(this.img, pos.x, pos.y, pos.r, pos.w/this.img:getWidth(), pos.h/this.img:getHeight())
		end
		love.graphics.setColor(r, g, b, a)
	end,
	-- other code
}

Gspot.button = {
	-- other code
	draw = function(this, pos)
		-- other code
		if this.shape == 'circle' then
			if this.img then this:drawimg(pos, true) end -- button images are not scaled to match the button's dimensions
			if this.label then lgprint(this.label, (pos.x + pos.r) - (this.style.font:getWidth(this.label) / 2), (this.img and (pos.y + (pos.r * 2)) + ((this.style.unit - this.style.font:getHeight()) / 2)) or (pos.y + pos.r) - (this.style.font:getHeight() / 2)) end
		else
			if this.img then this:drawimg(pos, true) end -- button images are not scaled to match the button's dimensions
			if this.label then lgprint(this.label, (pos.x + (pos.w / 2)) - (this.style.font:getWidth(this.label) / 2), (this.img and pos.y + ((this.style.unit - this.style.font:getHeight()) / 2)) or (pos.y + (pos.h / 2)) - (this.style.font:getHeight() / 2)) end
		end
	end,
	-- other code
}
And if you want an image on a button that has these properties, add it as a separate Gspot:image element that is a child of the button.

Re: Gspöt - retained GUI lib

Posted: Wed Feb 17, 2021 9:48 pm
by pgimeno
Anase Skyrider wrote: Wed Feb 17, 2021 2:41 am
pgimeno wrote: Sun Feb 14, 2021 12:07 pm Do you have some other suggestion that keeps backwards compatibility?
In my opinion, it's a feature not worth keeping nor making backwards compatible, but... If you're gonna do it, you'd need to add a bool that uses the default functionality on false/nil, and not increment the colors for parented buttons on true. EDIT2: If you want *me* to write that code, I have no problem with that.
Looks like it's not too difficult to work around if it bothers you, by setting the 'value' property of either the button or the parent (but not both) to false. I'm inclined to leave it as is.

Anase Skyrider wrote: Wed Feb 17, 2021 2:41 am You could still add the depress coloring (seriously, buttons that don't change color when you mousedown? Pfft, that's crazy :P), just use the bool for whether or not parented buttons will shift the color-states used.
There are more things in how this lib works that I don't personally like; it's unconventional in some areas, and I'm very conventional :) But that also makes it a GUI with its own personality, which may be good for some use cases. Also I believe it's used for things like RPG inventory or in-game buttons, where activating an action on mouse up might have an impact on the feeling of responsiveness, for example.

One of the key features of this library is its compactness: it's a single file. But with 44K, it's already somewhat large. That's the main reason why I don't want to add features. It's easy to hack if you want to add your own feature set, as you seem to be doing. Trying to add features to satisfy all normal uses would make it too large. A multi-line edit box, for example, is a must for many use cases, and is something I really miss in it, but it'd be too much code.

I like your idea of making images resizeable, but for the above reasons I'm unsure whether to implement it.

Re: Gspöt - retained GUI lib

Posted: Thu Feb 18, 2021 5:44 pm
by Anase Skyrider
I don't wanna be rude by arguing, but I have polite disagreements and want to voice them for the last time (and stop clogging the thread thereafter).
pgimeno wrote: Wed Feb 17, 2021 9:48 pm Looks like it's not too difficult to work around if it bothers you, by setting the 'value' property of either the button or the parent (but not both) to false. I'm inclined to leave it as is.
I just added the check for nil that I suggested, personally, but that could work too. It should be worth a mention to users on the wiki IMHO.
pgimeno wrote: Wed Feb 17, 2021 9:48 pm Also I believe it's used for things like RPG inventory or in-game buttons, where activating an action on mouse up might have an impact on the feeling of responsiveness, for example.
Almost every RPG I've played, and really just UI in general, has UI interactions on mouse-up, not mouse-down. Accidental mouse-downs are a thing (I do them a lot), and I always appreciate when a click is activated on mouse-up because I can mouse-off the button before mousing-up and not accidentally click the button.

Only exception I can think of is World of Warcraft. Many addons let you activate action bars on mouse-down, but those are a special case because those are spells in a fast-paced game *full* of spell icons to click on, so it increases speed (especially if you turn off spell-dragging so you can't accidentally drag them off your action bar), and has use if you play with keybinds because it turns spell-clicking into pressing buttons to do an action, like playing a third person action game. But keypressed is a separate function, so you'd have to code it manually to call click, meaning activating UI on mouse-up doesn't affect this. The user was already deciding, during coding, if they wanted to call click on keypressed or keyreleased.
pgimeno wrote: Wed Feb 17, 2021 9:48 pm A multi-line edit box, for example, is a must for many use cases, and is something I really miss in it, but it'd be too much code.
This would actually take, in my estimation, less than 15 new lines of code to add because most features would be replacing lines of code. Text elements already handle multi-line features with LOVE's font:getWrap() and love.graphics.printf(), so input could just use that. this.cursor is a substring position of this.value -- a 1D value, which is perfect.
  • Make Gspot.input use a scrollgroup as the base if you pass true to isMultiline on creation.
  • Add a check so that the enter button inputs a return character when isMultiline == true. This is only for user-added new-lines, returns are not added when wrapping this.value to the box.
  • When drawing the text, use Gspot's lgprintf() so that the raw this.value string will wrap to the width of the box, and allow the height to clip using the scrollgroup. Users can make the box automatically adjust the height with some very simple code.
  • To do the cursor position, just pass the substring of (1, this.cursor) to :getWrap(), which returns a table lines containing the sequence of line-by-line substrings. cursorx is calculated with the substring of the lines[#lines] instead of using the whole this.value, and cursory is the font-height multiplied by #lines (remember: lines is the substring from 1 to this.cursor, not the whole string).
  • I don't quite know how scrollgroups work, but you *might* have to add variables to pre-existing lines to get the cursor to offset.
Most of the work is already done for you. Very few lines need to be added, and it's backward-compatible with old versions of Gspot since it's based on a single bool (isMultiline).
pgimeno wrote: Wed Feb 17, 2021 9:48 pm I like your idea of making images resizeable, but for the above reasons I'm unsure whether to implement it.
This feature is very necessary if you want to do things like add click logic to an image that is highly animated without reimplementing it manually. Stuff like sprites whose animation isn't *just* the frames but also stuff like shearing and scaling. And my change doesn't impact old code, this is actually backward-compatible with very few added lines of code.

I'm still not sure why you wouldn't want to update the library to make it a little better to use, to be honest. Of the libraries I've looked at or played with, Gspot is one of the easier ones to use without being too unwieldy like some of the other ones. Do people automatically update their LOVE libraries without looking at the changelog and fixing their code accordingly?