Add a `url` property to `LinkButton`

[zaksnet]

This is useful when you want to add link buttons through code (simplifies things) and anyway without this linkButton is just a button with an underline. I tried connecting the pressed signal instead of launching the URL from the draw event but i couldn't figure it out, any guidance is appreciated. Also documented linkButton.xml

If this is going to get merged it would be nice to add:

• [ ] StyleBoxes for the rest of the states (currently only for focused exists).
• [ ] Text alignment.

I wouldn't bother otherwise since you can do all this with a button already, but since this exists, we could improve it.

EDIT: Changed keyword from Link to Url since its more proper in this case (link most of the times implies a website URL, while URL can be anything from websites, file paths to system paths or even UNC network paths). Also added launch_url to manually launch the Url. @akien-mga please review.

[aaronfranke]

@zaksnet Is this still desired? If so, it needs to be rebased on the latest master branch.

[zaksnet]

Alright this finaly seems to be working: @Calinou !

Just as a reminder, all this PR does is adding the Url parameter for the LinkButton. This is convenient because all the user has to do is enter the url in the editor. Otherwise to get the same result you'd have to create a script and export a variable to the editor.

[zaksnet]

~~The errors in the editor after closing the project seem to be related to OS::get_singleton()->shell_open(url);.~~

ERROR: Condition "!process_map->has(p_pid)" is true. Returning: FAILED
   at: OS_Windows::kill (platform\windows\os_windows.cpp:489)

EDIT: NVM, i get them with any project when i open/close it a couple of times.

[KoBeWi]

This is useful when you want to add link buttons through code (simplifies things)

You mean like

var link = LinkButton.new()
link.url = "https://godotengine.org/"

instead of

var link = LinkButton.new()
link.connect("pressed", OS, "shell_open", ["https://godotengine.org/"])

?

btw by connecting pressed to open_url() you make it impossible to use the button for a different purpose.

[zaksnet]

This is useful when you want to add link buttons through code (simplifies things)

You mean like

var link = LinkButton.new()
link.url = "https://godotengine.org/"

instead of

var link = LinkButton.new()
link.connect("pressed", OS, "shell_open", ["https://godotengine.org/"])

?

Yup, pretty much. Plus you can set the Url in the editor.

btw by connecting pressed to open_url() you make it impossible to use the button for a different purpose.

Well, it seems to me that a link button has very targeted behaviour, which is to launch a Url. Again, you could achieve the same result with a base button but since this exists i though we could make it as easy as possible to set it up. You think i should remove the connection with pressed signal?

[zaksnet]

var link = LinkButton.new() link.connect("pressed", OS, "shell_open", ["https://godotengine.org/"])

Btw that's cool! If its that simple to do it (never though of this!) then we could just document it and close this PR. But is this still possible in Godot 4.0 with callables?

[KoBeWi]

You think i should remove the connection with pressed signal?

The problem I see is that open_url throws an error if url is empty, so this button will throw an error by default when pressed. This also means the signal becomes effectively useless for the user, because it will always want to open url, so you can't connect it to anything else.

But is this still possible in Godot 4.0 with callables?

Yes, but the syntax is a bit different: link.pressed.connect(OS.shell_open.bind("https://godotengine.org/"))

[zaksnet]

You think i should remove the connection with pressed signal?

The problem I see is that open_url throws an error if url is empty, so this button will throw an error by default when pressed. This also means the signal becomes effectively useless for the user, because it will always want to open url, so you can't connect it to anything else.

But is this still possible in Godot 4.0 with callables?

Yes, but the syntax is a bit different: link.pressed.connect(OS.shell_open.bind("https://godotengine.org/"))

Cool! I say we close this PR and document link.pressed.connect(OS.shell_open.bind("https://godotengine.org/")). Otherwise we could just make it so that the pressed signal is only being connected when the Url is not empty (so that we avoid the error).

[zaksnet]

The latest commit only connects the signal if Url parameter is empty.

[KoBeWi]

This won't work (you can test it yourself). The error is mostly useful when using open_url() directly, so you can just bind another method under this name that would do the error check.

So you have _open_url() which opens the link if not empty. pressed signal would be connected here. open_url(), which does the error check and calls _open_url(). This would be the method exposed in GDScript.

[zaksnet]

This won't work (you can test it yourself). The error is mostly useful when using open_url() directly, so you can just bind another method under this name that would do the error check.

So you have _open_url() which opens the link if not empty. pressed signal would be connected here. open_url(), which does the error check and calls _open_url(). This would be the method exposed in GDScript.

So now when you try to invoke open_url() from code while the url is empty you get the error, but when you click the LinkButton you dont get an error even if the url is empty (but it will also not launch anything). I guess the benefit now is that the users can leave the url empty and use the pressed signal for a different purpose 👍

[zaksnet]

Looks ok functionality-wise. Docs for url member might need some changes though. I'm still not 100% convinced that this PR is useful (especially seeing 0 upvotes). It makes sense, but maybe there should be a proposal opened.

Yup, I agree with your concerns. But if i personally would have to open a proposal about the LinkButton then I would say that this node should not even exist. Since it does thought, this PR probably makes it a bit more "usable". Also, I think that just documenting on how to use this along with shell_open() would be enough for newcomers (as you demonstrated in your example). But again as I said in the original post without this PR the LinkButton is just a button with an underline.

[mhilbrunner]

I don't think we should have such a specific button.

Having a button that has a specific style because it suggests it links to stuff (an in-game wiki, a reference, the in-game help) or a resource like a website/URL is fine.

But changing it to have a URL connected that is opened on the OS seems too specific, especially if it is just a single connect to the shell open function to do it yourself.

IMO this isn't needed, like an exit button - every game may have one, but we can probably expect users to connect the pressed signal to get_tree().quit() themselves.

I'm for closing this PR and leaving the button as a simple general style option and nothing else.

[aaronfranke]

With the current LinkButton in master without this PR, the only difference that I can see between LinkButton and a regular Button with StyleBoxEmpty is the underline. Also note that RichTextLabel can have links, such as this in the RTL demo:

as well as [color=aqua][url=https://godotengine.org]custom URLs[/url][/color]

So I'd say that removing LinkButton isn't a huge loss and I would also be in favor of removing it.

[aaronfranke]

@zaksnet We reviewed this in a PR meeting. We discussed removing LinkButton, it was decided against because @reduz used it recently in Godot's codebase. However, this PR needs to be rebased before it can be considered.

[akien-mga]

We discussed removing LinkButton, it was decided against because @reduz used it recently in Godot's codebase. However, this PR needs to be rebased before it can be considered.

To clarify, @reduz independently implemented this in #59810 - and that PR is still on hold for the same reason that we were still unsure whether we want to keep LinkButton at all. But now we agree to get either implementation finalized and merged (should likely be cross-reviewed) and we can always reassess the usefulness of that node later.

[zaksnet]

@aaronfranke @Calinou This is ready for review

[akien-mga]

I'd suggest overriding the virtual pressed() method instead of using the signal, as done in #59810.

I'm also not convinced about the usefulness of the open_url() method. A button's function is to be pressed and that's when the URL should be opened. If users want to open the URL without pressing the button they don't really need the button in the first place, but they could just do OS.shell_open($LinkButton.url) if need be.

[akien-mga]

I force pushed an update to solve merge conflicts, and do some further improvements to the docs, and clean up of unused methods added to the header.

I also changed the url property to uri, as I think it's the more correct term for this kind of identifier which may or may not have a protocol specified. E.g. AFAIK godotengine.org would be a URI while https://godotengine.org is a URL, and both should work here. Maybe @Faless can confirm.

I don't mind changing back to url if we feel like this is still the better name.

[akien-mga]

Thanks!

[timothyqiu]

Cherry-picked for 3.6.