Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add origin options to FormSpec labels #8658

Closed
wants to merge 12 commits into from

Conversation

v-rob
Copy link
Member

@v-rob v-rob commented Jul 7, 2019

Note: This PR is closed due to bad implementation. A new one will be worked on after #9755 is merged (or rejected for some reason).

This PR allows the label and vertlabel elements in FormSpecs to have origin set and custom newline spacing. This closes #7613. It adds three new styling options to label and vertlabel:

From lua_api.txt:

  • label, vertlabel
    • horigin - Sets horizontal origin in relation to label text. Possible values:
      • left: Left side of the bounding box
      • center: Center of the bounding box
      • right: Right side of the bounding box
      • leftline (vertlabel only): Left end of first column
      • centerline (vertlabel only): Center of first column
      • rightline (vertlabel only): Right end of first column
        Default left for label and centerline for vertlabel.
    • vorigin - Sets vertical origin in relation to label text. Possible values:
      • top: Top of the bounding box
      • center: Center of the bounding box
      • bottom: Bottom of the bounding box
      • topline (label only): Top of first line
      • centerline (label only): Center of first line
      • bottomline (label only): Bottom of first line
        Default centerline for label and top for vertlabel.
    • newline_spacing - number, sets space between lines/columns of text. If
      the number is prefixed with *, the spacing is set to a multiple of the
      label's font height or width, otherwise it's in coordinates. Default 0.5.

In addition to these styling options, newlines have been added to vertlabel since it didn't previously support this. Each newline moves the text to the right, unless, of course, it's negative.

A nice side effect of how it is implemented is that the label and vertlabel rects are as tall as the text height as opposed to a full coordinate as it is currently. This also makes it so that it will work with really gigantic fonts as well as normal ones.

Screenshot

A picture's worth a thousand words, so here you go:
image
The gray dots show where the label position is.

To do

This PR is Ready to Review.

How to test

It's very long

This is the above screenshot:

local function boxes(num_x, num_y)
	local boxes = ""
	for x = 0, num_x do
		for y = 0, num_y do
			if x % 2 == y % 2 then
				boxes = boxes .. "box["..x..","..y..";1,1;red]"
			end
		end
	end
	return boxes
end

local function point(x, y)
	return "box[".. x-0.05 ..",".. y-0.05 ..";0.1,0.1;gray]"
end

minetest.show_formspec("singleplayer", "formspec:test",
	"formspec_version[3]"..
	"size[20,12]"..
	boxes(19, 11)..
	---
	point(1, 1)..
	"style_type[label;horigin=left]".. -- Remove value for default label horigin: left
	"label[1,1;H-left]"..

	point(1, 2)..
	"style_type[label;horigin=center]"..
	"label[1,2;H-center]"..

	point(1, 3)..
	"style_type[label;horigin=right]"..
	"label[1,3;H-right]"..

	"box[2.95,0;0.1,12;gray]"..
	"box[0,6.95;3,0.1;gray]"..
	---
	point(1, 8)..
	"style_type[label;horigin=;newline_spacing=*1]"..
	"label[1,8;Newline 1 *1\nNewline 2 *1\nNewline 3 *1]"..
	
	point(1, 11)..
	"style_type[label;horigin=;newline_spacing=*-2]"..
	"label[1,11;Newline 1 *-2\nNewline 2 *-2\nNewline 3 *-2]"..
	---
	"style_type[label;newline_spacing=0.33]"..

	point(4, 1)..
	"style_type[label;vorigin=top]".. -- Remove value for default label vorigin: centerline
	"label[4,1;V-top\nV-top2\nV-top3]"..

	point(4, 3)..
	"style_type[label;vorigin=center]"..
	"label[4,3;V-center\nV-center2\nV-center3]"..

	point(4, 5)..
	"style_type[label;vorigin=bottom]"..
	"label[4,5;V-bottom\nV-bottom2\nV-bottom3]"..

	point(4, 6)..
	"style_type[label;vorigin=topline]"..
	"label[4,6;V-topline\nV-topline2\nV-topline3]"..

	point(4, 8)..
	"style_type[label;vorigin=centerline]"..
	"label[4,8;V-centerline\nV-centerline2\nV-centerline3]"..

	point(4, 10)..
	"style_type[label;vorigin=bottomline]"..
	"label[4,10;V-bottomline\nV-bottomline2\nV-bottomline3]"..

	"box[5.95,0;0.1,12;gray]"..
	---
	"style_type[label;newline_spacing=-0.33]"..

	point(7, 1)..
	"style_type[label;vorigin=top]"..
	"label[7,1;V-top\nV-top2\nV-top3]"..

	point(7, 3)..
	"style_type[label;vorigin=center]"..
	"label[7,3;V-center\nV-center2\nV-center3]"..

	point(7, 5)..
	"style_type[label;vorigin=bottom]"..
	"label[7,5;V-bottom\nV-bottom2\nV-bottom3]"..

	point(7, 6)..
	"style_type[label;vorigin=topline]"..
	"label[7,6;V-topline\nV-topline2\nV-topline3]"..

	point(7, 8)..
	"style_type[label;vorigin=centerline]"..
	"label[7,8;V-centerline\nV-centerline2\nV-centerline3]"..

	point(7, 10)..
	"style_type[label;vorigin=bottomline]"..
	"label[7,10;V-bottomline\nV-bottomline2\nV-bottomline3]"..

	"box[8.95,0;0.1,12;gray]"..
	---
	"style_type[label;horigin=]"..

	point(10, 2)..
	"style_type[label;vorigin=top]".. -- Remove value for default vertlabel vorigin: top
	"vertlabel[10,2;V|TOP]"..

	point(11, 2)..
	"style_type[label;vorigin=center]"..
	"vertlabel[11,2;V|CEN]"..

	point(12, 2)..
	"style_type[label;vorigin=bottom]"..
	"vertlabel[12,2;V|BTM]"..

	"box[9,3.95;11,0.1;gray]"..
	"box[15.95,0;0.1,4;gray]"..
	---
	point(17, 1)..
	"style_type[label;vorigin=;newline_spacing=*1]"..
	"vertlabel[17,1;\\\\N 1 *1\n\\\\N 2 *1\n\\\\N 3 *1]"..
	
	point(19, 1)..
	"style_type[label;vorigin=;newline_spacing=*-2]"..
	"vertlabel[19,1;\\\\N 1 *-2\n\\\\N 2 *-2\n\\\\N 3 *-2]"..
	---
	"style_type[label;newline_spacing=0.25]"..

	point(10, 5)..
	"style_type[label;horigin=left]".. -- Comment for default horigin: centerline
	"vertlabel[10,5;H|LFT\nH|LFT2\nH|LFT3]"..

	point(12, 5)..
	"style_type[label;horigin=center]"..
	"vertlabel[12,5;H|CEN\nH|CEN2\nH|CEN3]"..

	point(14, 5)..
	"style_type[label;horigin=right]"..
	"vertlabel[14,5;H|RGT\nH|RGT2\nH|RGT3]"..

	point(15, 5)..
	"style_type[label;horigin=leftline]"..
	"vertlabel[15,5;H|LFL\nH|LFL2\nH|LFL3]"..

	point(17, 5)..
	"style_type[label;horigin=centerline]"..
	"vertlabel[17,5;H|CTL\nH|CTL2\nH|CTL3]"..

	point(19, 5)..
	"style_type[label;horigin=rightline]"..
	"vertlabel[19,5;H|RTL\nH|RTL2\nH|RTL3]"..

	"box[9,7.95;11,0.1;gray]"..
	---
	"style_type[label;newline_spacing=-0.25]"..

	point(10, 9)..
	"style_type[label;horigin=left]"..
	"vertlabel[10,9;H|LFT\nH|LFT2\nH|LFT3]"..

	point(12, 9)..
	"style_type[label;horigin=center]"..
	"vertlabel[12,9;H|CEN\nH|CEN2\nH|CEN3]"..

	point(14, 9)..
	"style_type[label;horigin=right]"..
	"vertlabel[14,9;H|RGT\nH|RGT2\nH|RGT3]"..

	point(15, 9)..
	"style_type[label;horigin=leftline]"..
	"vertlabel[15,9;H|LFL\nH|LFL2\nH|LFL3]"..

	point(17, 9)..
	"style_type[label;horigin=centerline]"..
	"vertlabel[17,9;H|CTL\nH|CTL2\nH|CTL3]"..

	point(19, 9)..
	"style_type[label;horigin=rightline]"..
	"vertlabel[19,9;H|RTL\nH|RTL2\nH|RTL3]"
)

@SmallJoker SmallJoker added Feature ✨ PRs that add or enhance a feature Formspec labels Jul 10, 2019
@v-rob
Copy link
Member Author

v-rob commented Jul 14, 2019

Can this, along with #8665, #8652, and #8646 be added to the formspec merging list (https://github.com/minetest/minetest/projects/6)?

I know there are a lot of formspec PRs, but I think it's best for all formspec changes to be made in a single release since they require a client update for them to work properly. Don't worry, I don't have too many left to do ;-)

@SmallJoker SmallJoker added this to Highest priority on top in Formspec Priority List Jul 15, 2019
@rubenwardy rubenwardy added Documentation needed WIP The PR is still being worked on by its author and not ready yet. Rebase needed The PR needs to be rebased by its author. labels Aug 8, 2019
@v-rob v-rob force-pushed the formspec-label-options branch 2 times, most recently from 8bc4629 to 8987f07 Compare November 28, 2019 00:57
@paramat
Copy link
Contributor

paramat commented Dec 12, 2019

Seems to be actively worked on so removing 'possible close' label. Or was there a reason for that?

@v-rob v-rob force-pushed the formspec-label-options branch 16 times, most recently from 0fabbaf to 9027b4c Compare January 16, 2020 04:45
@v-rob
Copy link
Member Author

v-rob commented Jan 16, 2020

This PR is now Ready for Review

I've added support for vertlabel. I've given them newline support as well just like label, and they can also be aligned the same way. Note that this doesn't break anything: if you use a newline in a vertlabel before now, it will cut off the text in the rect, plus the fact that this all only works with real coordinates enabled.

doc/lua_api.txt Outdated Show resolved Hide resolved
doc/lua_api.txt Outdated Show resolved Hide resolved
@rubenwardy rubenwardy requested a review from SmallJoker May 5, 2020 18:21
@rubenwardy rubenwardy removed One approval ✅ ◻️ Rebase needed The PR needs to be rebased by its author. labels May 5, 2020
@rubenwardy
Copy link
Member

There's a bit of repetition here, not necessary a bad problem

It would be good to have the test formspec in minimal. The points are also clearer when red:

image

@sorcerykid
Copy link
Contributor

sorcerykid commented May 6, 2020

Hmm, this seems way too complicated for label elements. Even being an avid developer with formspecs myself, I would honestly never use 90% of the options in this PR. As I recall, #7613 just asked for basic left, centere, and right alignment, I cannot imagine many use-cases where people require more sophisticated fine-control just to position labels above or beside a field.

I get the impression the overall feature set of formspecs is very off-balance and lacks an agreed-upon vision for the future. Is there not an official roadmap posted somewhere? For example, I just saw a feature request for embedding any kind of content inside buttons. It's still not even possible to dynamically update formspecs in the client, yet apparently we'll be able to have buttons that can have entire mini-formspecs in them. That doesn't make any sense O_o

I seem to recall Rubenwardy had an issue opened awhile back for refactoring the entire formspec backend. I guess that's not happening now that the goal is just randomly adding cool and fun features.

@rubenwardy
Copy link
Member

rubenwardy commented May 6, 2020

The extra features are required in order to retain compatibility and consistency. But you could remove the many *line alternatives and literally only have left/center/right/legacy. This is what I suggested, adding the rest could be considered going overboard

There's still no consensus on a refactor (#9358), I received no response to my suggestion

@ghost
Copy link

ghost commented May 6, 2020

It's still not even possible to dynamically update formspecs in the client, yet apparently we'll be able to have buttons that can have entire mini-formspecs in them. That doesn't make any sense O_o

Sure it does. It's an easy feature that's impossible for mods to properly hack in with the current API, and the necessary backend work lays the API groundwork for a larger and more important feature--allowing tabs to have 'pages' of content instead of forcing a re-send every tab change.

Dynamically updating formspecs is super important, but it requires significant design and implementation effort to get in and is more in the 'concept phase' than anything else. Expecting people to be working on something that isn't actually hashed out fully is a great way to make bad API decisions--something that Minetest suffers greatly from as-is.

@v-rob
Copy link
Member Author

v-rob commented May 6, 2020

I personally consider the *line options more useful; that's what the original PR did. I realize that some people might consider the non-line options useful though. The line for the first char isn't really that useful, but wasn't hard to implement. It can be used to line up a label and a vertlabel at the same position so that they have the same first character. I can still remove that if wanted.

@v-rob
Copy link
Member Author

v-rob commented May 12, 2020

I think I should probably clarify my rationale for making the line options: Labels are primarily for putting text next to, above, or somewhere in relation to an element in order to give it more meaning. They aren't intended as large blocks of text; that's textarea's job (and see #9755, which is a much smaller PR besides), but as small descriptors. Now, let's look at the real_coordinates screenshot:

See the label with newlines over the buttons? Try aligning that without the line options. And what if each label has to be above the button with topline? I suppose that separate labels can be used, but that takes more work, and it's impossible to combine when making the labels have a newline_spacing as a multiple of font size.

I realize that non-line options are useful for making a pseudo-textarea without the scrolling, word wrap, or highlighting, which can be used for, e.g., an overlay over a button, but that doesn't discount the usefulness of the lines.

I can remove the line options if others are insistent, but I find them useful, and they add barely any more code. The non-line options use more code, actually.

The character alignment variation of the line options isn't really useful -- that's something I think I will remove. I only added that because I could, and that is a bad reason. Edit: Removed

@v-rob
Copy link
Member Author

v-rob commented Jun 3, 2020

I was mulling over this PR a bit yesterday, and I think I've revised my position on whether or not this should be merged. It seems pretty certain that sized labels with halign and valign will be made at some point, and I suppose it would be redundant to have two similar things, even though they have subtle differences. It would probably end up as confusing for modders and not worth the extra code in the engine.

As a result, I think I will close this PR and create one for aligned sized labels after the very simple #9755 is merged to minimize conflicts. It would probably have newline_spacing as well, but it would not include vertlabels whatsoever since that can be emulated with label with a newline after every character and a newline_spacing of *1. I wouldn't include *line options because they wouldn't work in that case well anyway.

Edit: And it would be able to support the old coordinate system for, say, a textarea's units.

I won't act without feedback, though, so I would appreciate any anyone could give me.

@sorcerykid
Copy link
Contributor

Thanks for being receptive to my feedback v-rob. And sorry if I kind of lashed out with so many criticisms before. I value the work you do, and this PR (at least in its earlier state) had the exact feature-set that I'd dreamed about for years. So it's not for lack of appreciation of the concept. It's moreso that I just feel it's important that formspecs have at least remain balanced as possible when it comes to capabilities vs. complexity. I tend to feel that a generic API that solves at least the most common use-cases, while still accommodating edge-cases via documented workarounds, is usually the most elegant solution. Hope that makes sense :)

@v-rob v-rob closed this Jun 18, 2020
@v-rob v-rob deleted the formspec-label-options branch July 18, 2020 20:43
@v-rob v-rob moved this from PRs: Highest priority on top to Adoption needed or rejected in Formspec Priority List Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature ✨ PRs that add or enhance a feature Formspec
Projects
Formspec Priority List
  
Adoption needed or rejected
Development

Successfully merging this pull request may close these issues.

Formspec `label alignment
7 participants