Ability to Broadcast games on the Local Area Network #3828

sladyn98 · 2020-01-25T10:35:17Z

Ability to Broadcast games on the Local Area Network

Contains

JoinGameScreen.java - Added the broadcast button to be enabled only when the server is not broadcasting, along with a custom message of all the modules, ports and IpAddresse's

Broadcast.java - Added the ability to broadcast the game on all local listening servers.
The message is broadcasted every 5 minutes.
"Fixes #3760"

How to test

The broadcast button is added to the User Interface and the respective broadcasted message can be read on the Join Game Server on a machine present on the LAN.

Outstanding before merging

✔️ UI button for broadcasting
✔️ Make sure Broadcasting is enabled for headless server.
Need to add documentation for this.

…o the local area network. JoinGameScreen.java - Added the broadcast button to be enabled only when the server is not broadcasting.

GooeyHub · 2020-01-25T10:35:18Z

Can one of the admins please verify this patch?

sladyn98 · 2020-01-25T10:37:06Z

CC @DarkWeird

DarkWeird · 2020-01-25T11:54:41Z

It's wip now?
I don't sure what using all upped non loopback interface is good idea. I haven't idea how make this part better :3
Please, add todo list to Outstanding before merging.

sladyn98 · 2020-01-25T12:06:26Z

@DarkWeird I added the Button for the broadcast but when I run the game it does not show up , am I missing something ?

DarkWeird · 2020-01-25T12:47:23Z

I am not familiar with NUI.
Use it:
https://github.com/Terasology/TutorialNui

engine/src/main/java/org/terasology/rendering/nui/layers/mainMenu/JoinGameScreen.java

DarkWeird · 2020-01-26T18:50:29Z

Don't forget cleanup resources (shutdown executor on exit)
Maybe better integrate server to network subsystem, or separate.
Be careful, we have some works with nui extraction in separate lib.
I wanna see your code, working on headless server too :3

sladyn98 · 2020-01-27T16:43:48Z

Don't forget cleanup resources (shutdown executor on exit)

Yes adding it :)

Maybe better integrate server to network subsystem, or separate.

With respect to integration with integration with network subsytem, this is already separate module isn't it ?

Be careful, we have some works with nui extraction in separate lib.

Yeah it does not break anything for now, Do I need to take care of something specific ?

I wanna see your code, working on headless server too :3

Working on the headless server part,

DarkWeird · 2020-01-27T17:03:28Z

I think better implement as part of network system, or near.

Your broadcast placed at nui's package now :3

lgtm-com · 2020-01-28T18:17:15Z

This pull request introduces 1 alert when merging 835c573 into 6671bf5 - view on LGTM.com

new alerts:

1 for Dereferenced variable may be null

Added a translation for the broadcast and terminate broadcast features also closed the executor thread after termination.

DarkWeird · 2020-02-01T12:53:17Z

Fix imports, plz (broadcastserver have import on nui)
Why broadcastserver in headless setup use DI stuffs(its work?), in nui not
What with multiple network interfaces?
If you bind server on all interfaces - it is 0.0.0.0 - unreachable target port. You need broadcast every interface ip.
If specific(one)- you not need broadcast to others network. (It's my imho. Based on network knowledge. Not deep terasology's network knowledge)

SoniEx2 · 2020-02-01T14:31:09Z

maybe use UPnP or something?

sladyn98 · 2020-02-01T15:19:05Z

@DarkWeird

Why broadcastserver in headless setup use DI stuffs(its work?), in nui not

I am not sure what DI stuff, could you explain what do you mean ?

If you bind server on all interfaces - it is 0.0.0.0 - unreachable target port. You need broadcast every interface ip.

   Vector<String> container=new Vector<String>();
        container.add(message);
        String m=classname.getMYIP();
               //here u have your own IP address..now suppose your IP is 192.168.1.1.basically m sending hello packet to every other machine.if that machine is active ,it will reply n we get to know that machine is active......
               for(int i=1;i<255;i++)
           {
                        try
                               {
                     
                    String n="192.168.1."+i;//192.168.1.1,192.168.1.2 n so on.........to 192.168.1.255
                     
                        new Thread(new client(n,container)).start();//create client threads n send hello packet         to every other machine......
                        System.out.println(i);
                     
                }catch(Exception e){e.printStackTrace();}
             
        }

We could do this , it is a much brute force approach and only checks for 255 machines.
What we are doing here is basically

Determine your own IP address
Determine your own netmask
Scan all the addresses (except the lowest, which is your network address and the highest, which is your broadcast address)

sladyn98 · 2020-02-01T15:19:49Z

maybe use UPnP or something?

I guess this is very susceptible to attacks, it is a bit of a dangerous approach ?

SoniEx2 · 2020-02-01T15:48:24Z

properly configured UPnP isn't any more risky than UDP broadcast.

improperly configured UPnP doesn't suddenly become more dangerous because apps are using it. improperly configured UPnP is inherently dangerous, but doesn't affect the safety of terasology.

whether UPnP is properly or improperly configured depends on the router, not on terasology.

DarkWeird · 2020-02-01T15:48:54Z

@sladyn98

Dependency injection. In our case - @ In annotation.
I say about several network cards (wifi/lan, several lan)
You can start server on all network interface(use ip 0.0.0.0, as ip for bind server) or specific(192.168.1.1 (lan for example) or 10.0.1.1(wifi for example))
If you start server on wifi, your code will broadcast invalid ip for lan(for wifi it bradcast too)
If you start on any network card(interface), your code will broadcast invalid ip for one of network interface(lan or wifi)

Maybe, some problem will be with docker, vpn, etc. With all what generate virtual network interfaces, if it installed/used on system

DarkWeird · 2020-02-01T15:58:28Z

@SoniEx2 i think implemention of UPnP can be as separate PR.
IIRC, UPnP configure router's routing from WAN to LAN, isn't it?

Hmm, broadcast to WAN ?

SoniEx2 · 2020-02-01T16:42:08Z

Please read up on UPnP https://en.wikipedia.org/wiki/Universal_Plug_and_Play

It is a more suitable solution to this problem.

sladyn98 · 2020-02-01T16:45:20Z

Please read up on UPnP https://en.wikipedia.org/wiki/Universal_Plug_and_Play

It is a more suitable solution to this problem.

Thank you so much for the feedback, really appreciate it. Will Read up on this.

sladyn98 · 2020-02-01T16:49:30Z

@DarkWeird Looking at the dependency injection, rather I left it to the class to be responsible for the creation for the object. It would work fine, if i hard code the object creation, If you suggest that is a better approach, I am definitely on board :)

sladyn98 · 2020-02-02T07:53:20Z

engine/src/main/java/org/terasology/network/BroadCastServer.java

+        // ----------------------------------------- //
+        StringBuffer discoveryMessage = new StringBuffer();
+        discoveryMessage.append("M-SEARCH * HTTP/1.1\r\n");
+        discoveryMessage.append("HOST: " + ssdpIP + ":" + ssdpPort + "\r\n");


This looks to be sufficient in order to broadcast both the port and address.

DarkWeird · 2020-02-11T16:35:57Z

This methods using system default encoding (windows-125* on windows, utf-8 on linux(or any other encoding, if user has very interesting ideas :) )).
One string with different encodings can represent as different bytes.

Further, it has not zero chance to use different, non equals encoding.

Oh, java encoding called as charset.

My(and many libraries authors) recommendation: explicitly use UTF-8 ( new String(bytes, charset), getBytes(charset))

sladyn98 · 2020-02-12T18:46:28Z

@DarkWeird I added the encoding. Thanks

engine/src/main/java/org/terasology/network/BroadCastServer.java

engine/src/main/java/org/terasology/rendering/nui/layers/mainMenu/JoinGameScreen.java

engine/src/main/java/org/terasology/network/BroadCastServer.java

DarkWeird · 2020-02-12T19:58:28Z

engine/src/main/java/org/terasology/network/BroadCastServer.java

+    private DatagramSocket getReceiveSocket()  {
+        try {
+            if (receiveSocket == null) {
+                receiveSocket = new DatagramSocket(8002,
+                    InetAddress.getByName("0.0.0.0")); // 0.0.0.0 for listen to all ips
+                receiveSocket.setBroadcast(true);
+            }
+        } catch (Exception e) {
+            logger.error("Broadcast Exception Encountered" + e.getMessage());
+        }
+        return receiveSocket;
+    }
+
+    private DatagramSocket getSendSocket() {
+        try {
+            if (sendSocket == null) {
+                sendSocket = new DatagramSocket(8001, InetAddress.getLocalHost());
+                sendSocket.setBroadcast(true);
+            }
+        } catch (Exception e) {
+            logger.error("Broadcast Exception Encountered" + e.getMessage());
+        }
+        return sendSocket;
+    }


In these methods:
If sockets cannot to create(e.g. ports already binded), you will receive NullPointerException, because methods will return null at the end.

We won't be needing a new socket right? Because we would be using the same one to send the request. Do you think there is a better way to do this?

I'm would do separate socket intialization and getters, because it can throw exception.
If it will throw exception, then broadcast feature cannot will work properly at least.
If you will use one class instance - you use one socket.
Additional, You can split client and server functionality

Thanks for the feedback but in my opinion this code should catch broadcast errors if the sockets are null or the ports are unavailable so that the broadcast can be terminated immediately. I do not think we need more modularity in terms of client and server functionality because this offers us the best error handling.

engine/src/main/java/org/terasology/network/BroadCastServer.java

engine/src/main/java/org/terasology/rendering/nui/layers/mainMenu/JoinGameScreen.java

sladyn98 · 2020-02-25T15:27:30Z

@DarkWierd got most of the changes committed :)

DarkWeird · 2020-02-26T04:57:29Z

@sladyn98
Your Runnable runnable with listenToBroadCast not using now.

I don't think what PR done.
You print response in log. How is regular player can use your feature? Regular player don't use console.

sladyn98 · 2020-02-26T18:29:35Z

@DarkWeird

You print response in log. How is regular player can use your feature? Regular player don't use console.

Yeah this was something I was waiting for feedback on. Do we want to build a separate area for displaying the servers or just show up on the JoinGameScreen.?

engine/src/main/java/org/terasology/network/BroadCastServer.java

DarkWeird · 2020-02-26T20:14:30Z

@DarkWeird

You print response in log. How is regular player can use your feature? Regular player don't use console.

Yeah this was something I was waiting for feedback on. Do we want to build a separate area for displaying the servers or just show up on the JoinGameScreen.?

Better separate area with something like 'LAN' title. Or something like separate Tab (if it is possible).
When I saw your PR I thought about Counter-Strike 1.6's 'LAN Games' :)

engine/src/main/java/org/terasology/network/BroadCastServer.java

sladyn98 · 2020-03-09T17:38:56Z

Should this PR involve the development of a new UI component, since the IP addresses for the broadcast would anyway show up in the Join game section. What do you guys think ?
cc @skaldarnar @DarkWeird

Cervator · 2020-03-11T04:18:42Z

IMHO I would try to just finish out this PR at its current level of functionality so we can merge it and start fresh with a second effort involving the UI. This has been an awesome PR but the longer one keeps going the more unwieldy the PR gets and the higher the risk of code going stale / conflicting with changes made in the meantime. Small relatively quick PRs ftw - although some topics to warrant some deeper review and some extra effort cycles :-)

sladyn98 · 2020-03-13T16:40:18Z

Cool @DarkWeird This looks done to me then ?

sladyn98 · 2020-04-15T11:07:40Z

Gentle Ping @DarkWeird @Cervator Can we get this over the line :)
CC @skaldarnar

jdrueckert · 2020-04-15T21:31:45Z

🤔

skaldarnar

For some reason the main menu translation was broken when testing this PR. Not sure where this comes from, though.

When I click the "Join Game Screen" my game just crashes with this error:

23:34:15.553 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#game-settings'
23:34:15.558 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#start-singleplayer-game'
23:34:15.558 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#host-multiplayer-game'
23:34:15.558 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#join-game-online'
23:34:15.558 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#game-settings'
23:34:15.558 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#extras-label'
23:34:15.558 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#exit-game'
23:34:15.563 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#non-native-jvm-warn'
23:34:16.053 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#storage-service'
23:34:16.053 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#storage-service-logged-out'
23:34:16.053 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#select-game-title'
23:34:16.058 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#select-singleplayer-game-sub-title'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#world-generator'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-list'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#create-game'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#load-game'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#delete-game'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#save-game-details'
23:34:16.068 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#back'
23:34:16.073 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#save-game-path'
23:34:16.078 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#new-game-title'
23:34:16.078 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#select-singleplayer-game-sub-title'
23:34:16.078 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#game-name'
23:34:16.078 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#gameplay'
23:34:16.083 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#return-main-menu'
23:34:16.083 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#start-playing'
23:34:16.083 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#back'
23:34:16.083 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#advanced'
23:34:16.093 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#advanced-game-setup'
23:34:16.093 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#warn-modules'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#world-seed'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-filter'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#disable-all-modules'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-name'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-version-installed'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-version-available'
23:34:16.098 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-status'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#activate-module'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#download-module'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#game-details-module-details'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#select-modules-filter-reset'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#return-basic-setup'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#continue-universe-setup'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#return-main-menu'
23:34:16.103 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#start-playing'
23:34:16.108 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#load-delay-warning'
23:34:16.153 [main] WARN  o.t.engine.internal.TimeBase - Delta too great (10644), capping to 1000
23:34:16.158 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#storage-service'
23:34:16.158 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#storage-service-token-not-present'
23:34:16.162 [main] INFO  o.t.logic.console.ConsoleImpl - [NOTIFICATION] ?storage-service?: ?storage-service-token-not-present?
23:34:23.582 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#settings-title'
23:34:23.584 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#player-settings'
23:34:23.584 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#video-settings'
23:34:23.584 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#audio-settings'
23:34:23.584 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#input-settings'
23:34:23.585 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#back'
23:34:25.466 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#audio-settings-title'
23:34:25.467 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#sound-volume'
23:34:25.469 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#music-volume'
23:34:25.469 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#back'
23:34:35.734 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#join-server-title'
23:34:35.735 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#listed-servers'
23:34:35.736 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#custom-servers'
23:34:35.736 [main] WARN  o.t.rendering.nui.asset.UIFormat - Field 'defaultCard' not recognized for interface org.terasology.rendering.nui.UIWidget in {"type":"CardLayout","id":"cards","defaultCard":"onlineServerListScrollArea","contents":[{"type":"RelativeLayout","id":"customServerListScrollArea","contents":[{"type":"ScrollableArea","content":{"type":"UIList","id":"customServerList","family":"module-list"},"layoutInfo":{"position-top":{"target":"TOP"},"position-bottom":{"target":"TOP","widget":"edit","offset":8}}},{"type":"UIButton","id":"edit","text":"${engine:menu#edit-server}","layoutInfo":{"use-content-height":true,"width":135,"position-bottom":{"target":"TOP","widget":"add","offset":8}}},{"type":"UIButton","id":"remove","text":"${engine:menu#remove-server}","layoutInfo":{"use-content-height":true,"position-left":{"target":"RIGHT","widget":"edit","offset":8},"position-bottom":{"target":"TOP","widget":"add","offset":8}}},{"type":"UIButton","id":"add","text":"${engine:menu#add-server}","layoutInfo":{"use-content-height":true,"position-bottom":{"target":"BOTTOM"}}}]},{"type":"RelativeLayout","id":"onlineServerListScrollArea","contents":[{"type":"ScrollableArea","content":{"type":"UIList","id":"onlineServerList","family":"module-list"},"layoutInfo":{"position-top":{"target":"TOP"},"position-bottom":{"target":"TOP","widget":"download","offset":4}}},{"type":"UILabel","id":"download","text":"<download info text>","layoutInfo":{"use-content-height":true,"position-bottom":{"target":"BOTTOM","offset":4}}}]}],"layoutInfo":{"position-top":{"target":"BOTTOM","widget":"serverTypeRow"},"position-bottom":{"target":"BOTTOM"}}}
23:34:35.737 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#edit-server'
23:34:35.737 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#remove-server'
23:34:35.737 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#add-server'
23:34:35.738 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#server-name'
23:34:35.738 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#server-owner'
23:34:35.738 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#server-address'
23:34:35.738 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#server-port'
23:34:35.738 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#ping'
23:34:35.738 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#online-players'
23:34:35.739 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#game-worlds'
23:34:35.739 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#module-list'
23:34:35.739 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#join-server-refresh'
23:34:35.739 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#join-server-action'
23:34:35.739 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#join-server-broadcast'
23:34:35.739 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#back'
23:34:35.869 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#enter-username-message'
23:34:35.870 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#dialog-ok'
23:34:35.870 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#dialog-cancel'
23:34:35.885 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#parsing-content'
23:34:35.885 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#parsing-content'
23:34:35.887 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#parsing-content'
23:34:35.887 [main] WARN  o.t.i18n.TranslationSystemImpl - No translation for 'engine:menu#parsing-content'
23:34:35.890 [main] ERROR o.terasology.engine.TerasologyEngine - Uncaught exception, attempting clean game shutdown
java.lang.NullPointerException: null
	at org.terasology.rendering.nui.layers.mainMenu.JoinGameScreen$8.get(JoinGameScreen.java:359)
	at org.terasology.rendering.nui.layers.mainMenu.JoinGameScreen$8.get(JoinGameScreen.java:356)
	at org.terasology.rendering.nui.widgets.UIButton.onDraw(UIButton.java:152)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:425)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:411)
	at org.terasology.rendering.nui.layouts.RowLayout.onDraw(RowLayout.java:136)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:425)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:411)
	at org.terasology.rendering.nui.layouts.relative.RelativeLayout.onDraw(RelativeLayout.java:85)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:425)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:411)
	at org.terasology.rendering.nui.layouts.ColumnLayout.onDraw(ColumnLayout.java:191)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:425)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:411)
	at org.terasology.rendering.nui.layouts.relative.RelativeLayout.onDraw(RelativeLayout.java:85)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:425)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:411)
	at org.terasology.rendering.nui.CoreScreenLayer.onDraw(CoreScreenLayer.java:211)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawStyledWidget(CanvasImpl.java:425)
	at org.terasology.rendering.nui.internal.CanvasImpl.drawWidget(CanvasImpl.java:411)
	at org.terasology.rendering.nui.internal.NUIManagerInternal.render(NUIManagerInternal.java:509)
	at org.terasology.engine.modes.StateMainMenu.render(StateMainMenu.java:210)
	at org.terasology.engine.subsystem.lwjgl.LwjglGraphics.postUpdate(LwjglGraphics.java:166)
	at org.terasology.engine.TerasologyEngine.tick(TerasologyEngine.java:471)
	at org.terasology.engine.TerasologyEngine.mainLoop(TerasologyEngine.java:425)
	at org.terasology.engine.TerasologyEngine.run(TerasologyEngine.java:401)
	at org.terasology.engine.Terasology.main(Terasology.java:162)
23:34:35.891 [main] INFO  o.terasology.engine.TerasologyEngine - Shutting down Terasology...

I didn't look into the details further as I could not test it properly. One thing I noticed is that you don't make use of the Closeable sockets, e.g., use try-with-resources. That may simplify the creation and closing of the sockets in your code.

skaldarnar · 2020-04-15T21:20:03Z

engine/src/main/java/org/terasology/network/BroadCastServer.java

+    private static boolean turnOnBroadcast;
+    private static final Logger logger = LoggerFactory.getLogger(BroadCastServer.class);
+    private  final String discoveryRequest = "DISCOVERY_REQUEST";
+    private  final String discoveryResponse = "DISCOVERY_RESPONSE";
+    private  DatagramSocket receiveSocket;
+    private  DatagramSocket sendSocket;
+
+


I think I would order these members differently. First the static final fields (the string constants can also be final I would think), second the static members, and finally the class members.

The string constants should then be written in all-caps.

Suggested change

private static boolean turnOnBroadcast;

private static final Logger logger = LoggerFactory.getLogger(BroadCastServer.class);

private final String discoveryRequest = "DISCOVERY_REQUEST";

private final String discoveryResponse = "DISCOVERY_RESPONSE";

private DatagramSocket receiveSocket;

private DatagramSocket sendSocket;

private static final Logger logger = LoggerFactory.getLogger(BroadCastServer.class);

private static final String DISCOVERY_REQUEST= "DISCOVERY_REQUEST";

private static final String DISCOVERY_RESPONSE = "DISCOVERY_RESPONSE";

private static boolean turnOnBroadcast;

private DatagramSocket receiveSocket;

private DatagramSocket sendSocket;

skaldarnar · 2020-04-15T21:21:25Z

engine/src/main/java/org/terasology/network/BroadCastServer.java

+
+
+    public boolean isBroadCastTurnedOn() {
+        return turnOnBroadcast;


maybe use active as word here? BroadCastServer.isActive() may be a bit more intuitive.

skaldarnar · 2020-04-15T21:27:23Z

engine/src/main/java/org/terasology/network/BroadCastServer.java

+        BroadCastServer.turnOnBroadcast = true;
+        try {
+            sendBroadCast();
+            Executors.newSingleThreadExecutor().execute(new Runnable() {


Can use a lambda expression for the runnable here.

skaldarnar · 2020-04-16T08:51:30Z

engine/src/main/java/org/terasology/rendering/nui/layers/mainMenu/JoinGameScreen.java

@@ -92,6 +85,9 @@
    @In
    private StorageServiceWorker storageServiceWorker;

+    @In
+    private BroadCastServer broadCastServer;


BroadCastServer is just a regular class without any annotations or such. Thus, I don't think it can just be injected like this.

skaldarnar · 2020-04-16T08:52:21Z

engine/src/main/java/org/terasology/rendering/nui/layers/mainMenu/JoinGameScreen.java

+            broadCastServerButton.bindText(new ReadOnlyBinding<String>() {
+                @Override
+                public String get() {
+                    if (!broadCastServer.isBroadCastTurnedOn()) {


In case broadCastServer cannot be injected this will be null, leading to a NullPointerException. That probably breaks the UI screen then.

skaldarnar

@sladyn98 please make sure that your changes are working locally for you. If you're adding new buttons or adjusting the UI it's always a good idea to include screenshots of the changes/how it should look like.

skaldarnar · 2020-05-23T21:18:11Z

Hey @sladyn98, are you still around?

Please do continue work on this and ask questions if you're still interested in this. Otherwise, I'm going to close this PR due to inactivity.

SoniEx2 · 2020-05-23T22:57:38Z

I could fork this tbh

skaldarnar · 2020-05-24T21:57:23Z

@SoniEx2 be our guest 🙃 broadcasting games in the local network would be a nice feature 🤓

sladyn98 · 2020-06-08T12:29:11Z

@SoniEx2 @skaldarnar I am so sorry guys, I do not think I can complete this feature, I am working with Jenkins for GSoC 2020 and I feel really bad for leaving this code incomplete. TBH it took a lot more than I initially expected, :) but I enjoyed working on it. Thanks for the reviews

skaldarnar · 2020-06-11T15:20:16Z

Thanks for the heads up @sladyn98 - closing this PR (for now).

BroadCastServer.java - Class to handle the broadcasting of messages t…

499bf49

…o the local area network. JoinGameScreen.java - Added the broadcast button to be enabled only when the server is not broadcasting.

sladyn98 requested a review from pollend January 25, 2020 10:36

sladyn98 changed the title ~~Ability to Broadcast games on the Local Area Network~~ (WIP ) Ability to Broadcast games on the Local Area Network Jan 25, 2020

txrp0x9 reviewed Jan 25, 2020

View reviewed changes

engine/src/main/java/org/terasology/rendering/nui/layers/mainMenu/JoinGameScreen.java Outdated Show resolved Hide resolved

committed suggestion

19383eb

Added UI for BroadCast Button and fixed broadcast message

835c573

Sladyn added 2 commits January 30, 2020 16:52

Translation Addition and Terminate Broadcast Feature Addition

a9e79ea

Added a translation for the broadcast and terminate broadcast features also closed the executor thread after termination.

Added broadcast facility to headless server

34f0f07

sladyn98 changed the title ~~(WIP ) Ability to Broadcast games on the Local Area Network~~ Ability to Broadcast games on the Local Area Network Feb 1, 2020

Added broadcast service using UnPnP

1f0cd29

sladyn98 commented Feb 2, 2020

View reviewed changes

Added UTF-8 encoding

dc53aa1

DarkWeird requested changes Feb 12, 2020

View reviewed changes

Committed suggestions

9643751

Made runnable use an Executor Service

9573cea

DarkWeird requested changes Feb 26, 2020

View reviewed changes

engine/src/main/java/org/terasology/network/BroadCastServer.java Outdated Show resolved Hide resolved

engine/src/main/java/org/terasology/network/BroadCastServer.java Outdated Show resolved Hide resolved

engine/src/main/java/org/terasology/network/BroadCastServer.java Show resolved Hide resolved

Added thread interruption and clse sockets feature

8a9c340

sladyn98 commented Feb 27, 2020

View reviewed changes

engine/src/main/java/org/terasology/network/BroadCastServer.java Show resolved Hide resolved

engine/src/main/java/org/terasology/network/BroadCastServer.java Show resolved Hide resolved

Removed static variables

9556818

skaldarnar self-requested a review March 9, 2020 13:14

skaldarnar requested changes Apr 15, 2020

View reviewed changes

skaldarnar reviewed Apr 16, 2020

View reviewed changes

skaldarnar closed this Jun 11, 2020



		public boolean isBroadCastTurnedOn() {
		return turnOnBroadcast;

Ability to Broadcast games on the Local Area Network #3828

Ability to Broadcast games on the Local Area Network #3828

Conversation

sladyn98 commented Jan 25, 2020 • edited

Contains

How to test

Outstanding before merging

GooeyHub commented Jan 25, 2020

sladyn98 commented Jan 25, 2020

DarkWeird commented Jan 25, 2020

sladyn98 commented Jan 25, 2020

DarkWeird commented Jan 25, 2020

DarkWeird commented Jan 26, 2020

sladyn98 commented Jan 27, 2020

DarkWeird commented Jan 27, 2020

lgtm-com bot commented Jan 28, 2020

DarkWeird commented Feb 1, 2020 • edited

SoniEx2 commented Feb 1, 2020

sladyn98 commented Feb 1, 2020

sladyn98 commented Feb 1, 2020

SoniEx2 commented Feb 1, 2020

DarkWeird commented Feb 1, 2020 • edited

DarkWeird commented Feb 1, 2020

SoniEx2 commented Feb 1, 2020

sladyn98 commented Feb 1, 2020

sladyn98 commented Feb 1, 2020

Choose a reason for hiding this comment

DarkWeird commented Feb 11, 2020

sladyn98 commented Feb 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sladyn98 Feb 25, 2020 • edited

Choose a reason for hiding this comment

sladyn98 commented Feb 25, 2020

DarkWeird commented Feb 26, 2020

sladyn98 commented Feb 26, 2020

DarkWeird commented Feb 26, 2020 • edited

sladyn98 commented Mar 9, 2020

Cervator commented Mar 11, 2020

sladyn98 commented Mar 13, 2020

sladyn98 commented Apr 15, 2020 • edited

jdrueckert commented Apr 15, 2020

skaldarnar 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

Choose a reason for hiding this comment

skaldarnar left a comment

Choose a reason for hiding this comment

skaldarnar commented May 23, 2020

SoniEx2 commented May 23, 2020

skaldarnar commented May 24, 2020

sladyn98 commented Jun 8, 2020

skaldarnar commented Jun 11, 2020

sladyn98 commented Jan 25, 2020 •

edited

DarkWeird commented Feb 1, 2020 •

edited

DarkWeird commented Feb 1, 2020 •

edited

sladyn98 Feb 25, 2020 •

edited

DarkWeird commented Feb 26, 2020 •

edited

sladyn98 commented Apr 15, 2020 •

edited