PhilippvK / playforia-minigolf

Client & Server for Minigolf Game known from Playforia/Playray/Appeli. Written in Java.
84 stars 31 forks source link

Add command line argument parsing #40

Closed pehala closed 4 years ago

pehala commented 4 years ago

Fixes #38

PhilippvK commented 4 years ago

May I ask you if you are doing this just for fun or do you play the game with others in a "regular" manner?

pehala commented 4 years ago

Kind of both, we used to play a lot with friends and we are slowly getting back to playing it and also I am a sucker for open source and I like to contribute to stuff I am using, I always have some kind of hobby programming project that I am working on side so right now this is it :D

PhilippvK commented 4 years ago

Kind of both, we used to play a lot with friends and we are slowly getting back to playing it and also I am a sucker for open source and I like to contribute to stuff I am using.

That's great. But do you have bad experience with the performance of the client-server communication? While the client is almost the same as the playforia JAR the server was reimplemented in a rather unreliable way. Until now I was in the opinion that the effort of fixing the issues with the server is not worth it because it works okay most of the time. Do you have any opinion on this.

I assume that you host the server from your home network for your friends? Do you think this is the best approach for now or would you prefer something else?

pehala commented 4 years ago

Kind of both, we used to play a lot with friends and we are slowly getting back to playing it and also I am a sucker for open source and I like to contribute to stuff I am using.

That's great. But do you have bad experience with the performance of the client-server communication? While the client is almost the same as the playforia JAR the server was reimplemented in a rather unreliable way. Until now I was in the opinion that the effort of fixing the issues with the server is not worth it because it works okay most of the time. Do you have any opinion on this.

I assume that you host the server from your home network for your friends? Do you think this is the best approach for now or would you prefer something else?

If you are asking about improvements to the network communication code itself, I think that it is alright for now, although we did not play for long enough yet to have enough data to judge this. It is written in a very suboptimal way, as most of this project is, but it seems to be somehow working for now. Only thing I think might be worthwhile is upgrading to a newer version of Netty to keep this project up to date, but that is only nice to have and should be done with caution as we do not have any test coverage to make sure it is working.

If you are asking about improvements in general for this project, I think that they are no updates actively needed as it is working fine. With that said, I also think that we can improve on a lot of areas like other distribution packages, better usability, maintaining code and others, but none of this is really necessary just nice to have.

pehala commented 4 years ago

Anyway, if you have some issues you would like to help with, just tag me in and I can take a look.

pehala commented 4 years ago

Rebased to the current master

PhilippvK commented 4 years ago

@pehala Hey, I can not find /icons/playforia.png inside the repo. Is is possible that you forgot to push it?

pehala commented 4 years ago

100% is although I am surprised it passed the CI then

PhilippvK commented 4 years ago

appletloader_playforia.gif is the file, right?

It just crashes when launching the Jar.

pehala commented 4 years ago

Yep it seems to be missing, sorry about that, should I hotfix that or will you can of it?

PhilippvK commented 4 years ago

I can do it. Should not require a PR.

Should I convert the GIF to PNG before?

pehala commented 4 years ago

appletloader_playforia.gif is the file, right?

It just crashes when launching the Jar.

Nope the image is the same as the rest of the items, just in the PNG format. I took it from you distribution branch.

PhilippvK commented 4 years ago

Ahhh I remember! Thanks.

PhilippvK commented 4 years ago

Why do you think CI should have caught that missing file. Is there an option to test the compiled program for runtime exeptions?

pehala commented 4 years ago

I thought it was compile time :D I forgot I used icon on the runtime.

Anyway I think it would be possible to specify test this (we could write a test case - specifically for this icon) and cli so I created issue for it.

PhilippvK commented 4 years ago

I recognized that the possibility that the popup with the Host & Port selection is dismissed via the cancel button is not catched. In this case the program is supposed to quit.

Resulting Exception:

Exception in thread "Thread-5" java.lang.IllegalArgumentException: protocol = http host = null
    at sun.net.spi.DefaultProxySelector.select(DefaultProxySelector.java:177)
    at sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1156)
...
pehala commented 4 years ago

I recognized that the possibility that the popup with the Host & Port selection is dismissed via the cancel button is not catched. In this case the program is supposed to quit.

Resulting Exception:

Exception in thread "Thread-5" java.lang.IllegalArgumentException: protocol = http host = null
  at sun.net.spi.DefaultProxySelector.select(DefaultProxySelector.java:177)
  at sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1156)
...

I will take look at it, thanks!

PhilippvK commented 4 years ago

I already kinda fixed it on my master branch. If you want to handle in in another way feel free to propose something.