Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Save last used IP for client #75

Open
pehala opened this issue Nov 16, 2020 · 21 comments
Open

Save last used IP for client #75

pehala opened this issue Nov 16, 2020 · 21 comments

Comments

@pehala
Copy link
Collaborator

pehala commented Nov 16, 2020

We should save somewhere the last IP that the client connected to to show instead of placeholder 127.0.0.1.

@PhilippvK
Copy link
Owner

Shall we use java.util.prefs.Preferences as it seems to manage the OS-dependent stuff?

@PhilippvK
Copy link
Owner

Is there anything else except the launcher options which should be saved in a persistent way as well? (f.e. last username,...)

@pehala
Copy link
Collaborator Author

pehala commented Nov 16, 2020

Username, port, ip, maybe even settings for the new lobby. But that all can be done iteratively, for now I would do only IP, port and username.

@PhilippvK
Copy link
Owner

At a later point we could also add a dropdown to choose the language in the launcher. But as a prerequisite we might have to scan which language actually do exist or hardcode them anywhere.

@pehala
Copy link
Collaborator Author

pehala commented Nov 16, 2020

Agree, we have them hardcoded in one Enum and we have ability to choose it with CLI options so actually only thing missing is adding it a a dropdown to the menu.

@pehala
Copy link
Collaborator Author

pehala commented Nov 16, 2020

I am not sure if the languages are actually used in the UI but I guess thats entirely different issue

@pehala
Copy link
Collaborator Author

pehala commented Nov 16, 2020

Shall we use java.util.prefs.Preferences as it seems to manage the OS-dependent stuff?

We could use it but preferences are backend neutral (at least thats what I have learn from a quick google) so we will probably have to write the file saving/loading code at least partially ourselves

@PhilippvK
Copy link
Owner

Shall we use java.util.prefs.Preferences as it seems to manage the OS-dependent stuff?

We could use it but preferences are backend neutral (at least thats what I have learn from a quick google) so we will probably have to write the file saving/loading code at least partially ourselves

I found this post about the Preferences API and it looks that it would be very easy to implement like in the example:

https://www.vogella.com/tutorials/JavaPreferences/article.html

@pehala
Copy link
Collaborator Author

pehala commented Nov 16, 2020

We could definitely try doing PoC

@PhilippvK
Copy link
Owner

Should only the last used ip+port in the launcher window be remembered or the --server or --port input of the command line as well?

@pehala
Copy link
Collaborator Author

pehala commented Nov 22, 2020

Ideally the command line as well

@PhilippvK
Copy link
Owner

Saving the currently chosen preferences can happen just after submitting the dialog? Alternatively we could only save the setting if the following server connection was successful but this would be a bit harder.

@pehala
Copy link
Collaborator Author

pehala commented Nov 22, 2020

After submitting dialog is fine

@PhilippvK
Copy link
Owner

@pehala I have a quick question regarding your ManifestVersionProvider:

If I run new ManifestVersionProvider().getVersion() it returns: ${COMMAND-FULL-NAME} 2.1.0.0-BETA

Is that intended? If yes, how can I get just the 2.1.0.0-BETA part?

@pehala
Copy link
Collaborator Author

pehala commented Nov 22, 2020

Yes, it is intended, it is used by the picocli library in CLI. I fetch the version (2.1.0.0-BETA) in that class with this <yourclass>.class.getPackage().getImplementationVersion(); (for this to works it requires building through Maven so it might not work in development environment but should work with generated jars).

@PhilippvK
Copy link
Owner

Thank you very much for clarification, I will try it out later.

I want to save the client version together with the preferences to have the ability to perform migrations when the preferences-concept somehow changes.

Do you have an idea on how to integrate the preferences management into the tests?

@pehala
Copy link
Collaborator Author

pehala commented Nov 22, 2020

Thank you very much for clarification, I will try it out later.

I want to save the client version together with the preferences to have the ability to perform migrations when the preferences-concept somehow changes.

Good idea!

Do you have an idea on how to integrate the preferences management into the tests?

You should design the implementation in a way that would allow you to use mocked preferences in test, so you could check the methods and stuff.

@PhilippvK
Copy link
Owner

Okay. I never used Mockito before and am currently not able to complete the tests. (Mostly the handling of dismissing/accepting the ServerSettings-Dialog is missing at the moment)

Would you mind if I just push my current state so that you can help me getting the tests to work?

@pehala
Copy link
Collaborator Author

pehala commented Nov 23, 2020

Of course, go for it

PhilippvK added a commit that referenced this issue Nov 23, 2020
Partially resolves Issue #75

TODO: Also save language setting after it was added to the Launcher
Dialog
@PhilippvK
Copy link
Owner

@pehala I am out of ideas. It seems like my replacement for the showSettingDialog never gets called...

@pehala
Copy link
Collaborator Author

pehala commented Nov 24, 2020

Just create PR and I will look at it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants