vpn/proxy issues when creating new mp game #6715

ofetios · 2022-05-06T17:09:13Z

resolves #6709, resolves #6649 by opening a connection to dropbox when starting a new mp game

Tested on windows and linux (Zorin OS). Seems to be working now.
I removed the redundant ping and replaced it with opening a connection to dropbox

I've also fixed the unnecessary commits and will close the other PR

replaced redundant ping with opening a connection to dropbox to fix the proxy issue

touhidurrr · 2022-05-06T17:33:55Z

firstly, I think I suggested that you use https://content.dropboxapi.com for connection checking instead of http://www.dropbox.com. The api and their main website is entirely two different thing altogether.
Also i suggested that you use com.unciv.UncivGame.Current.settings.multiplayerServer when available.

touhidurrr · 2022-05-06T17:47:19Z

I suggest this changes!!!
https://github.com/alexban011/Unciv/pull/2/files

…com`

ofetios · 2022-05-06T17:53:57Z

I suggested that you use https://content.dropboxapi.com

I'll change it now but

com.unciv.UncivGame.Current.settings.multiplayerServer

not sure how to check for that

touhidurrr · 2022-05-06T17:55:39Z

not sure how to check for that

Did you check my pr in your fork?

check multiplayerServer url if the user is playing on it

ofetios · 2022-05-06T17:58:13Z

Did you check my pr in your fork?

I did and merged it, thanks !

Mape6 · 2022-05-06T19:33:25Z

Looks good from my I-don't-know-anything-about-Kotlin point of view.

touhidurrr · 2022-05-06T19:42:08Z

Looks good from my I-don't-know-anything-about-Kotlin point of view.

Lol. I used https://play.kotlinlang.org & https://www.google.com to test some assumed lines of kotlin code and checked if those worked before doing these commits.

I know I used to say that. I will change 'anything' to 'much' from now on maybe. 🤣

I saw a YouTube video on Kotlin basics months ago. Maybe I should add that too.

ofetios · 2022-05-06T19:50:13Z

Looks good from my I-don't-know-anything-about-Kotlin point of view.

Lol. I used https://play.kotlinlang.org & https://www.google.com to test some assumed lines of kotlin code and checked if those worked before doing these commits.

I know I used to say that. I will change 'anything' to 'much' from now on maybe. 🤣

I saw a YouTube video on Kotlin basics months ago. Maybe I should add that too.

yeah I never wrote Kotlin before committing to this repo, I just assumed its somewhat similar to java and so far it seems to be. (more like python variables combined with java but yeah) :))

SomeTroglodyte · 2022-05-06T19:57:01Z

somewhat similar

Kotlin is an Island and a language. Java is an Island and a language. Intentional. The Kotlin compiler emits Java VM bytecode - unless a different backend is used.

if connected to proxy but no internet access it will freeze for a couple seconds (until it finishes the for loop) but it works

touhidurrr · 2022-05-06T23:52:15Z

This new code looks a lot of sus to me. How does it work?
Also if it freezes for a few seconds then why not use promise and loop through promise.all output?

touhidurrr · 2022-05-07T00:01:54Z

Also if internet is not working shouldn’t the previous changes be enough to check it?
Do we really need to check internet connectivity further like this?
Feels like a waste of time to me.

Or is the previous version is not enough to detect proxy?

ofetios · 2022-05-07T00:06:15Z

How does it work?

well its the code that runs on desktop, but I'm not sure why it freezes.
Removing the android SDK stuff also works the same way (execution probably doesnt even get there unless it has internet access), so the freezing seems to be coming from connect(), but changing the timeouts doesn't fix the 3 sec freeze. (Btw it only freezes when connected to proxy)

Or is the previous version is not enough to detect proxy?

No, the previous version was not working when connected to proxy

ofetios · 2022-05-07T00:16:41Z

So I could remove the SDK code. That would make the function identical to the desktop one so It could be moved back to NewGameScreen to avoid duplicate code? Or leave it in PlatformSpecificHelpers?

I kept it there in case that fails, so that it continues to the SDK code, but I probably should have added another try-catch inside that one. Might be redundant though, so what do you think? Keep the android SDK code or get rid of it? And if the latter should I move isInternetConnected() out of the platform specific stuff and move it in NewGameScreen where its being used?

touhidurrr · 2022-05-07T01:13:12Z

Keep the android SDK code or get rid of it

Probably something that runs on both is better. Android Specific codes are a problem.

Also if the previous changes then Unciv shouldn’t work on proxy in the first place. URL comes directly from Java so i don't get why it wouldn’t work.

touhidurrr · 2022-05-07T01:16:13Z

Maybe check Dropbox codes and use the network utilities used there? They should work regardless of platform a they are already working properly currently right?

yairm210 · 2022-05-07T20:03:54Z

android/src/com/unciv/app/PlatformSpecificHelpersAndroid.kt

@@ -34,13 +36,23 @@ Sources for Info about current orientation in case need:
    }

    override fun isInternetConnected(): Boolean {


If you're changing this to not only check internet connectivity, there are actually 2 fail states and so A. the function name should be changed (checkConnectionToMultiplayerServer or something) and B. we should tell the user if it's a problem with his internet or the server itself, so either return a fail string or an enum or something.

This also means we can keep the already-translated line of "No internet connection!" and just add another translation line of "The server is unreachable" or something

yeah I'm gonna split the 2 in 2 separate functions

yairm210 · 2022-05-07T20:05:18Z

android/src/com/unciv/app/PlatformSpecificHelpersAndroid.kt

+            val connectivityManager = activity.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
+            for (network in connectivityManager.allNetworks) {
+                val networkCapabilities = connectivityManager.getNetworkCapabilities(network) ?: continue
+                val isInternet = networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+                val info = connectivityManager.getNetworkInfo(network) ?: continue
+                if (isInternet && info.isConnected) return true
+            }
+
+            true


And obviously this should be first.
First check if we even have internet, if no - failure A.
Then check if we can reach server - if no, failure B.

I tried with that first but the code fails (the freeze is back)

And obviously this should be first.
First check if we even have internet, if no - failure A.
Then check if we can reach server - if no, failure B.

I think alex said that even after doing a connection check to the server, if the client is connected to the a proxy / vpn URL.connect doesn't work (?)

So, if B always fails ob Proxy, doing it before A is pointless.

ofetios · 2022-05-07T20:16:52Z

ignore my last commit :)

SomeTroglodyte · 2022-05-07T20:28:00Z

Sorry, in its current state I'd call this a no-go, wrong design.

Java URL.openConnection is not exactly platform-dependent. Not as cross-platform as Gdx APIs maybe, but still. So - doesn't really belong in platform-specific code if there are no other compelling reasons.
Querying the Connectivity manager do decide whether or not to attempt an actual internet connection is good practice. It's meant to determine stuff like whether config allows, and interfaces are up, and should typically do its job without any packet on the line (and if it sends stuff that's the OS guy's decision and none of our business). So before, not after, and it is the correct thing to do. The equivalent on Linux would be to query the Network manager, maybe even by calling nmcli and parsing the result; on Windows one could query whether a default route exits and its associated interface it up; or the API offers something equivalent (which probably would do similar things under the hood.)
If you're sure the usecase justifies putting a packet on the line without prior test whether it even has a chance to reach its destination, go ahead, you don't need the whole PlatformSpecificHelpers stuff. Unless existing Java or Gdx apis are lacking a feature.
Uh, and a tiny thing: ?: "Dropbox"; if (multiplayerServer != "Dropbox") is silly. val url = URL(UncivGame.Current.settings.multiplayerServer ?: "https://content.dropboxapi.com") would do. And the static dripbox address there could be clearer as a static const val somewhere appropriate.

Ah, parallel comments & commits, OK pick what's still relevant.

removed frunctions from PlatformSpecificHelpers__

ofetios · 2022-05-08T22:24:22Z

In platform-specific code: NO - UI doesn't belong there

so I guess its good where it is now (next to the invalid ID code)

touhidurrr

simplify and redo it!!

touhidurrr · 2022-05-06T18:11:28Z

core/src/com/unciv/ui/newgamescreen/NewGameScreen.kt

@@ -73,7 +73,7 @@ class NewGameScreen(
            if (gameSetupInfo.gameParameters.isOnlineMultiplayer) {
                if (UncivGame.Current.platformSpecificHelper?.isInternetConnected() != true) {
                    val noInternetConnectionPopup = Popup(this)
-                    noInternetConnectionPopup.addGoodSizedLabel("No internet connection!".tr()).row()
+                    noInternetConnectionPopup.addGoodSizedLabel("No connection to internet or dropbox!".tr()).row()


Maybe Just Couldn't connect to the Server would do?

Suggested change

noInternetConnectionPopup.addGoodSizedLabel("No connection to internet or dropbox!".tr()).row()

noInternetConnectionPopup.addGoodSizedLabel("Couldn't connect to the Server!".tr()).row()

@SomeTroglodyte what do you think?

touhidurrr · 2022-05-07T20:24:02Z

android/src/com/unciv/app/PlatformSpecificHelpersAndroid.kt

+            val connectivityManager = activity.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
+            for (network in connectivityManager.allNetworks) {
+                val networkCapabilities = connectivityManager.getNetworkCapabilities(network) ?: continue
+                val isInternet = networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+                val info = connectivityManager.getNetworkInfo(network) ?: continue
+                if (isInternet && info.isConnected) return true
+            }
+
+            true


And obviously this should be first.
First check if we even have internet, if no - failure A.
Then check if we can reach server - if no, failure B.

I think alex said that even after doing a connection check to the server, if the client is connected to the a proxy / vpn URL.connect doesn't work (?)

So, if B always fails ob Proxy, doing it before A is pointless.

touhidurrr · 2022-05-08T12:16:46Z

core/src/com/unciv/ui/newgamescreen/NewGameScreen.kt

+        return try {
+            val u = URL(UncivGame.Current.settings.multiplayerServer)
+            val conn = u.openConnection()
+            conn.connect()


According to java docs, openConnection() and connect() is used like this:

The connection object is created by invoking the openConnection method on a URL.

The setup parameters and general request properties are manipulated.

The actual connection to the remote object is made, using the connect method.

The remote object becomes available. The header fields and the contents of the remote object can be accessed.

So basically openConnection() makes a connection object and connect() connects to the server and makes the remote resouces available for further requests?

So basically java allows you to make a URLConnection object, modify it and then do actual network operations with connect(). There also seems to a property called connected that stores boolean values of a connection.

So, I guess proper way to do this would be something like

URL().openConnection().setTimeout(5000).connect()

Also it's unnecessary to ping any specific url since we are only checking the connection. But isalive would be better I guess since it sends less data.

touhidurrr · 2022-05-10T22:08:04Z

but it doesn't really matter if it can't check for dropbox)

@alexban011 I think I have found the correct way to do this. See this:

Unciv/core/src/com/unciv/ui/worldscreen/mainmenu/OptionsPopup.kt

Line 273 in 7b48337

    
           if (settings.multiplayerServer != Constants.dropboxMultiplayerServer) settings.multiplayerServer

So, I think something like this would do:

import java.net.URL
import com.unciv.Constants
import com.unciv.UncivGame

private fun isConnectionToServerPossible(): Boolean {
    return try {
        val multiplayerServer = UncivGame.Current.settings.multiplayerServer
        val isDropbox = multiplayerServer == Constants.dropboxMultiplayerServer
        val u =  URL(if (isDropbox) "https://content.dropboxapi.com" else multiplayerServer)
        val con = u.openConnection()
        con.setConnectTimeout(5000)
        con.connect()

        true
    } catch(ex: Throwable) {
        val noInternetConnectionPopup = Popup(this)
        val label = if (isDropbox) "Couldn't connect to Dropbox!" else "Couldn't connect to Multiplayer Server!"
        noInternetConnectionPopup.addGoodSizedLabel(label.tr()).row()
        noInternetConnectionPopup.addCloseButton()
        noInternetConnectionPopup.open()
        return@onClick

        false
    }
}

ofetios · 2022-05-10T22:54:22Z

Just tried it on android and it seems to work
I did had to make a slight modification: I moved the popup in the main function in an if(!isConnectionToServerPossible()) cause it was putting an error at @OnClick

did you tested it on the Pi?

touhidurrr · 2022-05-10T22:58:59Z

did you tested it on the Pi?

I can't compile the source for reasons. Give me a jar to test.

ofetios · 2022-05-10T23:16:04Z

I can't compile the source for reasons. Give me a jar to test.

I'm trying. I'm getting this:
Kotlin could not find the required JDK tools in the Java installation. Make sure Kotlin compilation is running on a JDK, not JRE.

I'll try reinstalling the JDK as people on forums said this should solve this

Sending it now

ofetios · 2022-05-10T23:32:56Z

Sorry for mediafire link, I have no clue how else to send it:
Download link

@touhidurrr

SomeTroglodyte · 2022-05-10T23:35:21Z

putting an error at @OnClick

You're lucky (or maybe unlucky) there seems to be no actual user with that name around, or you would have spammed them. And the jar location is in the wiki not far under the gradlew syntax.

SomeTroglodyte · 2022-05-10T23:38:27Z

mediafire

otnolatrnup.com, doubleclick.net and aaxads.com are all quite malicious. do not visit without a good blocker.

ofetios · 2022-05-10T23:40:23Z

You're lucky (or maybe unlucky) there seems to be no actual user with that name around, or you would have spammed them.

I made sure I'm not pinging anyone when I sent that :))

ofetios · 2022-05-11T00:18:10Z

So now it works on Windows and Android even with proxy (both Mobile Hotspot and 1.1.1.1 vpn).

Now the "Couldn't connect to Dropbox!" makes more sense than the old "No internet connection!", but should I change it back to the old one to keep the translation or keep the new one as its more informative of what the issue is?

ofetios · 2022-05-11T00:34:41Z

Last commit is just cleanup and I replaced the setConnectionTimeout because it was giving a warning.

Also I set the timeout to 3000 instead of 5000 since if the server doesn't respond in 3 seconds I really doubt it will in 5.
The freeze still happens on my phone but only if it was connected to the vpn before losing connection, so I don't think we should worry about it. Weirdly enough it seems to only freeze once and then the next time I press "start game" it displays the message instantly.

touhidurrr · 2022-05-11T09:13:15Z

Never pull updates in a branch!!! But only in master / main.

touhidurrr · 2022-05-11T09:22:26Z

desktop/src/com/unciv/app/desktop/PlatformSpecificHelpersDesktop.kt

-
-    override fun isInternetConnected(): Boolean {
-        return try {
-            InetAddress.getByName("8.8.8.8").isReachable(500)  // Parameter timeout in milliseconds


Sigh could it be that this is the reason why my checks always failed on Pi?
Proxy increases latency by a good margin and 500 is really not enough.
It is not enough even without proxy. And will fail 5-10% of times.
Who even sets timeout to only 0.5 seconds, I am astonished that nobody pointed this out.

ofetios · 2022-05-11T09:23:24Z

Never pull updates in a branch!!! But only in master / main.

Noted.

touhidurrr · 2022-05-11T09:31:22Z

Sorry for mediafire link, I have no clue how else to send it:
Download link

Sorry or not I can't even download it.

touhidurrr · 2022-05-11T09:32:42Z

@alexban011 use this https://www.file.io/

ofetios · 2022-05-11T09:46:32Z

@alexban011 use this https://www.file.io/

https://file.io/uQ3O9ABTwnyx

touhidurrr · 2022-05-11T10:01:59Z

@alexban011 I tested your jar and it works fine on raspberry pi

This pr got too big, maybe open a new one with clear & concise changes?

ofetios · 2022-05-11T10:09:02Z

maybe open a new one with clear & concise changes?

yeah I was thinking about that. I'll do it now

SomeTroglodyte · 2022-05-11T10:38:59Z

Never pull updates in a branch

Disagree. There are some very valid reasons to do so. Own work in two+ studio installations, or testing someone's branch and they pushed, or mixing features to see if they work together....... .. or maybe you meant "from yairm210/master"? That I don't do but even that could work if you know what you're doing - it'll start a merge.

touhidurrr · 2022-05-11T10:51:22Z

Merge branch 'yairm210:master' into master-proxyIssues

I said that cause I don't like this kind of commits.

even that could work if you know what you're doing - it'll start a merge.

Yeah that also. For starters you would like them to start clean. After a while they would understand these things anyways.
At first i deleted and forked repos again after each of my prs were accepted and closed prs if I saw any such commits in the history. Lol.
Now I just use the pen button on a file to automatically create a new branch for me & then commit at that branch.
Since I use mobile data cloning things locally feels troublesome.

SomeTroglodyte · 2022-05-11T11:10:58Z

I said that cause I don't like

Then a wording like "Never" gives a misleading impression, the sentence was an imperative. With three Char(0x21) to boot. Simply "I never..." would have hit the mark, right? Online, culture barriers and so on, be careful..

vpn/proxy issues when creating new mp game

5ec2a78

replaced redundant ping with opening a connection to dropbox to fix the proxy issue

ofetios mentioned this pull request May 6, 2022

vpn/proxy issues when creating new mp game #6711

Closed

updated error message to include dropbox

607470c

check multiplayerServer url if the user is playing on it

023226a

use https://content.dropboxapi.com instead of https://www.dropbox.…

ed09541

…com`

Merge pull request #2 from touhidurrr/patch-2

6bf0608

check multiplayerServer url if the user is playing on it

ofetios added 2 commits May 7, 2022 02:25

fixed proxy issues on android

f5000f2

if connected to proxy but no internet access it will freeze for a couple seconds (until it finishes the for loop) but it works

forgot to add the imports

aa78e1f

yairm210 reviewed May 7, 2022

View reviewed changes

removed android sdk network check to fix proxy issues

ccfff19

ofetios mentioned this pull request May 7, 2022

Can't create new Multiplayer Games from Desktop #6709

Closed

check for internet now separate from check for multiplayerServer

5338e5c

removed frunctions from PlatformSpecificHelpers__

yairm210 approved these changes May 10, 2022

View reviewed changes

touhidurrr suggested changes May 10, 2022

View reviewed changes

ofetios added 2 commits May 11, 2022 03:03

Merge branch 'yairm210:master' into master-proxyIssues

ef4e4a5

added improvements made by touhidurrr

db75d30

removed unused imports and replaced setter with property access syntax

916ecb5

touhidurrr reviewed May 11, 2022

View reviewed changes

ofetios closed this May 11, 2022

ofetios mentioned this pull request May 11, 2022

Fixed proxy issues when starting a new multiplayer game #6757

Merged

ofetios deleted the master-proxyIssues branch May 25, 2022 11:31

		@@ -34,13 +36,23 @@ Sources for Info about current orientation in case need:
		}

		override fun isInternetConnected(): Boolean {

	noInternetConnectionPopup.addGoodSizedLabel("No connection to internet or dropbox!".tr()).row()
	noInternetConnectionPopup.addGoodSizedLabel("Couldn't connect to the Server!".tr()).row()

vpn/proxy issues when creating new mp game #6715

vpn/proxy issues when creating new mp game #6715

Conversation

ofetios commented May 6, 2022 • edited

touhidurrr commented May 6, 2022

touhidurrr commented May 6, 2022

ofetios commented May 6, 2022 • edited

touhidurrr commented May 6, 2022

ofetios commented May 6, 2022

Mape6 commented May 6, 2022

touhidurrr commented May 6, 2022 • edited

ofetios commented May 6, 2022

SomeTroglodyte commented May 6, 2022

touhidurrr commented May 6, 2022 • edited

touhidurrr commented May 7, 2022

ofetios commented May 7, 2022 • edited

ofetios commented May 7, 2022 • edited

touhidurrr commented May 7, 2022

touhidurrr commented May 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofetios commented May 7, 2022

SomeTroglodyte commented May 7, 2022

ofetios commented May 8, 2022

touhidurrr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

touhidurrr commented May 10, 2022 • edited

ofetios commented May 10, 2022

touhidurrr commented May 10, 2022 • edited

ofetios commented May 10, 2022 • edited

ofetios commented May 10, 2022 • edited

SomeTroglodyte commented May 10, 2022

SomeTroglodyte commented May 10, 2022

ofetios commented May 10, 2022

ofetios commented May 11, 2022

ofetios commented May 11, 2022

touhidurrr commented May 11, 2022 • edited

Choose a reason for hiding this comment

ofetios commented May 11, 2022

touhidurrr commented May 11, 2022

touhidurrr commented May 11, 2022

ofetios commented May 11, 2022

touhidurrr commented May 11, 2022 • edited

ofetios commented May 11, 2022

SomeTroglodyte commented May 11, 2022 • edited

touhidurrr commented May 11, 2022 • edited

SomeTroglodyte commented May 11, 2022

ofetios commented May 6, 2022 •

edited

ofetios commented May 6, 2022 •

edited

touhidurrr commented May 6, 2022 •

edited

touhidurrr commented May 6, 2022 •

edited

ofetios commented May 7, 2022 •

edited

ofetios commented May 7, 2022 •

edited

touhidurrr commented May 10, 2022 •

edited

touhidurrr commented May 10, 2022 •

edited

ofetios commented May 10, 2022 •

edited

ofetios commented May 10, 2022 •

edited

touhidurrr commented May 11, 2022 •

edited

touhidurrr commented May 11, 2022 •

edited

SomeTroglodyte commented May 11, 2022 •

edited

touhidurrr commented May 11, 2022 •

edited