Copy server invite code to the clipboard automatically #9739

pelya · 2021-12-10T00:09:21Z

Motivation / Problem

When you create a server, you need to type the server invite code into email/chat letter by letter to send it to friends.

Description

With this patch, the invite code is copied to the clipboard automatically when the server registers with the masterserver.

Limitations

I am too lazy to add 'Copy to clipboard' button to the server info dialog, this is good enough for me.
This code works only with SDL2, other video backends will ignore the clipboard for now.

Checklist for review

2TallTyler · 2021-12-10T17:13:30Z

I'm +1 to this feature, but not how it's hidden from anyone who hasn't seen this PR. 😃

Personally I would prefer a Copy to clipboard button, although it would need to work with all video drivers.

Eilon · 2021-12-10T21:31:46Z

Setting clipboard data is "destructive" in the sense that it clears out whatever the user may have already had in their clipboard, which the user may have deemed valuable. Also, as @2TallTyler mentions, how would anyone even know that this happened so that they could actually make use of it?

I think adding an explicit button would be a better design, even if it's more work.

nielsmh

You need to make a button for copying the code, that makes it clear that clicking this button will replace the user's clipboard contents.
Don't silently overwrite the user's clipboard.

pelya · 2021-12-12T00:21:21Z

I've added a 'copy to clipboard' button to GUI

Button implemented

LC-Zorg · 2021-12-12T23:55:27Z

Please, don't care too much about this. Perhaps my opinion will be parochial and overly conservative for you, but...
I don't understand what this invitation code play is for at all. For me, but I also think that for many other average player, it makes no sense at all. Just like showing MD5sum or GRF ID in the NewGRF information - this information is completely useless for the player and only creates questions that the player doesn't need to know the answers to. You want to simplify the interface, but adding a button to such an enigmatic function doesn't simplify it at all.
I see this as the same function as the server password now. The difference is that it is more difficult to remember, so much that you need to add another function, which is in this PR. In addition, this code is shown even where a password is not needed at all. I'm sorry, but I really don't understand this.

2TallTyler · 2021-12-13T05:40:42Z

The invite code is for players to join their friends' games without having to list the game publicly. See #9434. But the value of this approach has nothing to do with this PR, which simply aims to make it easier to share the invite code.

pelya · 2021-12-13T07:57:43Z

I thought invite code was a way to join servers behind NAT? It looks pretty important to me.

…

On Mon, 13 Dec 2021, 07:40 Tyler Trahan, ***@***.***> wrote: The invite code is for players to join their friends' games without having to list the game publicly. See #9434 <#9434>. But the value of this approach has nothing to do with this PR, which simply aims to make it easier to share the invite code. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#9739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF5QC7V5SPPQWFVIUECP3UQWBOJANCNFSM5JXZVA6A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

nielsmh · 2021-12-13T08:12:31Z

@LC-Zorg
The invite code is the "address" of the game. When you want a friend to join your game, instead of having to figure out your IP address and set up port forwarding, you send the invite code to your friend. Your friend then enters the invite code, and gets connected to your game. (There is a blog post with more technical details if you're interested.)

That's why the invite code is important: It's the easy way to let others join your private game.

LC-Zorg · 2021-12-13T15:31:59Z

Ok, I don't use it and I don't think it will be any different in the future for me, but I can understand there is a need for some to hide the server. However, I think there are at least one maybe two significant issues with this feature that make its functionality significantly limited, and in its current form, at least from my point of view, it is more problematic than useful. It is independent of this PR, so I will write about it elsewhere. (#9746) (#9747)

If I understand the code correctly, this copy button is in the client list window in the server settings field. If so, I think there is no problem and it is a good solution. :) Previously, I thought this might be for the server list window, and that wouldn't be good.

LordAro

Like others I am generally +1 to this feature, but I don't understand the current implementation

Why is set_clipboard_text stored at all? Why not just call SDL_SetClipboardText directly in VideoDriver::SetClipboardContents ?

pelya · 2022-01-05T12:44:12Z

Because it must be called from the SDL main thread, not from the game thread.

…

On Wed, 5 Jan 2022, 14:42 Charles Pigott, ***@***.***> wrote: ***@***.**** commented on this pull request. Like others I am generally +1 to this feature, but I don't understand the current implementation Why is set_clipboard_text stored at all? Why not just call SDL_SetClipboardText directly in VideoDriver::SetClipboardContents ? — Reply to this email directly, view it on GitHub <#9739 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF5QCBBUU5LUBGP5H7ROLUUQ4D5ANCNFSM5JXZVA6A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

rubidium42

When actually squashing/merging, maybe note that it is SDL2 for now?

One tricky thing from the user's point of view is that is a button that does nothing (unless they use SDL2). That might be considered buggy behaviour, so does that make this patch bugged?
And what would be the best way to remedy this? Implementing it for the other platforms as well in this PR? Or ship it buggy and hope the maintainers fix those bugs? Or introduce some manner where the UI can ask the video driver whether it supports writing to the clipboard and then disable the button when the video driver doesn't support it?

pelya · 2022-02-04T20:39:30Z

Isn't SDL2 used for all release binaries that are published to openttd.org?
I guess we can surround this code with
#if SDL_VERSION_ATLEAST(2,0,0)

glx22 · 2022-02-04T20:44:46Z

SDL2 is used only on linux builds.

pelya · 2022-02-04T21:02:08Z

Eh okay. SDL2 is designed to abstract away Linux/Windows/MacOS differences, it seems counter-intuitive that OpenTTD is not using this feature. Well, we can copypaste SDL2 implementation at least. https://github.com/libsdl-org/SDL/blob/main/src/video/windows/SDL_windowsclipboard.c#L51 https://github.com/libsdl-org/SDL/blob/main/src/video/cocoa/SDL_cocoaclipboard.m#L29

…

On Fri, 4 Feb 2022, 22:44 Loïc Guilloux, ***@***.***> wrote: SDL2 is used only on linux builds. — Reply to this email directly, view it on GitHub <#9739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF5QF34FJXL2FY6W6CXL3UZQ3ERANCNFSM5JXZVA6A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

2TallTyler · 2022-02-04T22:04:03Z

I don't see a point shipping a build with a button that doesn't work for the majority of users. (I imagine Linux will be more prevalent among OpenTTD users than the average, but W3 Schools says it's used by about 4.3% of users.)

pelya · 2022-02-04T22:25:08Z

Yes, releasing the feature only for Linux makes little sense. I'm not using Windows or MacOS, I can only point to SDL2 clipboard implementation. Someone else will need to fill in the missing code in this patch.

…

On Sat, 5 Feb 2022, 00:04 Tyler Trahan, ***@***.***> wrote: I don't see a point shipping a build with a button that doesn't work for the majority of users. (I imagine Linux will be more prevalent among OpenTTD users than the average, but W3 Schools says it's used by <https://www.w3schools.com/browsers/browsers_os.asp> about 4.3% of users.) — Reply to this email directly, view it on GitHub <#9739 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF5QASQZBHDAC5F4CKLP3UZREN3ANCNFSM5JXZVA6A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you authored the thread.Message ID: ***@***.***>

nielsmh · 2022-02-05T07:54:01Z

I was about to say that the Win32 and Cocoa video drivers also support setting the clipboard, but I was mixing that up with the getting clipboard function, which is implemented in the OS code and not video driver.

OpenTTD/src/os/windows/win32.cpp

Lines 511 to 531 in 7f0efbe

    
           bool GetClipboardContents(char *buffer, const char *last) 
        
           { 
        
           	HGLOBAL cbuf; 
        
           	const char *ptr; 
        
           	if (IsClipboardFormatAvailable(CF_UNICODETEXT)) { 
        
           		OpenClipboard(nullptr); 
        
           		cbuf = GetClipboardData(CF_UNICODETEXT); 
        
           		ptr = (const char*)GlobalLock(cbuf); 
        
           		int out_len = WideCharToMultiByte(CP_UTF8, 0, (LPCWSTR)ptr, -1, buffer, (last - buffer) + 1, nullptr, nullptr); 
        
           		GlobalUnlock(cbuf); 
        
           		CloseClipboard(); 
        
           		if (out_len == 0) return false; 
        
           	} else { 
        
           		return false; 
        
           	} 
        
           	return true; 
        
           }

OpenTTD/src/os/macosx/macos.mm

Lines 191 to 206 in 7f0efbe

    
           bool GetClipboardContents(char *buffer, const char *last) 
        
           { 
        
           	NSPasteboard *pb = [ NSPasteboard generalPasteboard ]; 
        
           	NSArray *types = [ NSArray arrayWithObject:NSPasteboardTypeString ]; 
        
           	NSString *bestType = [ pb availableTypeFromArray:types ]; 
        
           	/* Clipboard has no text data available. */ 
        
           	if (bestType == nil) return false; 
        
           	NSString *string = [ pb stringForType:NSPasteboardTypeString ]; 
        
           	if (string == nil || [ string length ] == 0) return false; 
        
           	strecpy(buffer, [ string UTF8String ], last); 
        
           	return true; 
        
           }

OpenTTD/src/os/unix/unix.cpp

Lines 271 to 289 in 7f0efbe

    
           #ifndef WITH_COCOA 
        
           bool GetClipboardContents(char *buffer, const char *last) 
        
           { 
        
           #ifdef WITH_SDL2 
        
           	if (SDL_HasClipboardText() == SDL_FALSE) { 
        
           		return false; 
        
           	} 
        
           	char *clip = SDL_GetClipboardText(); 
        
           	if (clip != nullptr) { 
        
           		strecpy(buffer, clip, last); 
        
           		SDL_free(clip); 
        
           		return true; 
        
           	} 
        
           #endif 
        
           	return false; 
        
           } 
        
           #endif

I think it would make the most sense to move this code to the video driver (because the concept of a clipboard is usually quite tightly bound to the "exterior" windowing system used), and then of course implement setting the clipboard for all platforms as well.

LordAro

No issues with this at all, but it needs to support more than just SDL2 before being merged

2TallTyler · 2022-04-30T22:49:49Z

As @pelya posted on TT-Forums a month ago, he will likely not be working on this any time in the near future.

2TallTyler · 2022-11-07T19:34:07Z

This is a great feature but the PR can't be merged without supporting all OSs. Since @pelya has indicated that he's not willing/able to add that support, I'm closing the PR. Feel free to reopen if you disagree or have code that makes it work for other OSs. 🙂

Copy server invite code to the clipboard automatically

2234e9d

nielsmh previously requested changes Dec 10, 2021

View reviewed changes

Button to copy invite code to clipboard in the server options

52778e9

LordAro reviewed Jan 5, 2022

View reviewed changes

rubidium42 approved these changes Feb 4, 2022

View reviewed changes

LordAro requested changes Apr 30, 2022

View reviewed changes

2TallTyler closed this Nov 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy server invite code to the clipboard automatically #9739

Copy server invite code to the clipboard automatically #9739

pelya commented Dec 10, 2021

2TallTyler commented Dec 10, 2021

Eilon commented Dec 10, 2021 •

edited

nielsmh left a comment

pelya commented Dec 12, 2021

LC-Zorg commented Dec 12, 2021

2TallTyler commented Dec 13, 2021

pelya commented Dec 13, 2021 via email

nielsmh commented Dec 13, 2021

LC-Zorg commented Dec 13, 2021 •

edited

LordAro left a comment

pelya commented Jan 5, 2022 via email

rubidium42 left a comment

pelya commented Feb 4, 2022

glx22 commented Feb 4, 2022

pelya commented Feb 4, 2022 via email

2TallTyler commented Feb 4, 2022

pelya commented Feb 4, 2022 via email

nielsmh commented Feb 5, 2022

LordAro left a comment

2TallTyler commented Apr 30, 2022

2TallTyler commented Nov 7, 2022

Copy server invite code to the clipboard automatically #9739

Copy server invite code to the clipboard automatically #9739

Conversation

pelya commented Dec 10, 2021

Motivation / Problem

Description

Limitations

Checklist for review

2TallTyler commented Dec 10, 2021

Eilon commented Dec 10, 2021 • edited

nielsmh left a comment

Choose a reason for hiding this comment

pelya commented Dec 12, 2021

LC-Zorg commented Dec 12, 2021

2TallTyler commented Dec 13, 2021

pelya commented Dec 13, 2021 via email

nielsmh commented Dec 13, 2021

LC-Zorg commented Dec 13, 2021 • edited

LordAro left a comment

Choose a reason for hiding this comment

pelya commented Jan 5, 2022 via email

rubidium42 left a comment

Choose a reason for hiding this comment

pelya commented Feb 4, 2022

glx22 commented Feb 4, 2022

pelya commented Feb 4, 2022 via email

2TallTyler commented Feb 4, 2022

pelya commented Feb 4, 2022 via email

nielsmh commented Feb 5, 2022

LordAro left a comment

Choose a reason for hiding this comment

2TallTyler commented Apr 30, 2022

2TallTyler commented Nov 7, 2022

Eilon commented Dec 10, 2021 •

edited

LC-Zorg commented Dec 13, 2021 •

edited