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

New UI next steps (?) #2753

Closed
DanVanAtta opened this issue Dec 28, 2017 · 4 comments
Closed

New UI next steps (?) #2753

DanVanAtta opened this issue Dec 28, 2017 · 4 comments
Labels
Discussion team communication thread, meant for coordination and decision making

Comments

@DanVanAtta
Copy link
Member

New UI is now available behind a beta flag, not fully functional yet - a question here for dev team here on how to proceed. We have a few options:

  1. Try to make new UI work with existing network model code.
  2. Remove existing UI so we can refactor model code and then integrate it with the new UI
  3. Build a new network code infrastructure (JSON over Java sockets or something similar)

Option number one is a bit painful, but with enough stamina and teeth gritting and hacking we could get it to work. A nice bonus is that we can have both UIs, new and old side by side with both working.

Option two would make integration of network model code with the new UI much easier. We would also be able to drop a very large amount of 'legacy' UI code.

Option three requires potentially the most work as we would be rebuilding the network model, and this could have implications for actual gameplay as well beyond the staging screens. If we want to start a new network model/codebase, this could be a good option. A downside to this approach is that it would probably take longest as we rewrite TripleA's network code.

So question for dev's, which option seems most reasonable/best? I think I am leaning towards number one or two, but if there is some help I could be talked into number three. Number two is a bit attractive, but it requires us to commit with going with the new UI and not turning back. Happily the new UI does have all the screens mostly completed, largely we just need to wire together the network models so that the game launches (so it is actually pretty close). IF we drop the old UI code and can then refactor the network model code, that would likely be the most efficient and interestingly least risky for getting the new UI launched. On the other hand there is some good value in having both UI's running in parallel while we work to complete the UI update.

@DanVanAtta DanVanAtta added the Discussion team communication thread, meant for coordination and decision making label Dec 28, 2017
@DanVanAtta DanVanAtta changed the title New UI next steps New UI next steps (?) Dec 28, 2017
@RoiEXLab
Copy link
Member

@DanVanAtta Option 2 would probably a good compromise.
The network code really needs some work in general. Currently there's no way to implement a new feature without it causing an exception until the lobby (or the other client) is being updated.
Also the current state of the network code doesn't look like it can easily be changed to support TLS encryption.

TL;DR: I'd prefer option 2, especially because this would make JavaFX easier as well. The JavaFX UI will probably take longer than the "new swing UI". The biggest difference being that every UI component is going to be initialized exactly once and being displayed when neccessary, all in one window to allow for fullscreen etc. (exception: FileChooser/DirectoryChooser dialogs)

@DanVanAtta
Copy link
Member Author

Thanks for the feedback - a side note @RoiEXLab - My hope is that the 'new UI', using the custom Swing builder lib's will make JavaFX migration easier. The builders should help us split the logic from the UI code (and UI configuration), so eventually, when we go to JavaFX the builder API stuff could maybe be cleanly replaced.

@RoiEXLab
Copy link
Member

@DanVanAtta I hope so as well.

One thing I'm probably going to submit a PR for soon is the way the current UI handles the ClientSettings:
Currently some ClientSettings have a "not set" state, where they default to the hardcoded value. Instead they should have a "default" value that gets restored when klicking on "reset to default".
Little things like this already make implementing a new UI easier.

@RoiEXLab
Copy link
Member

@DanVanAtta But at some point it already did: The ClientSetting "framework" helped a lot when implementing the settings in JavaFX

DanVanAtta added a commit that referenced this issue Jul 21, 2018
* Drop "new ui" code, take a different approach

New approach: update existing UI code inline, move it incrementally towards something better using 'client.launch.screens' as an example.

Last we decided to drop the 'old' code so we could refactor/graft the simplified UI code to the 'old' model and networking support code. #2753

Why the change?
- it's still a really heavy lift to get multiplayer working
- it's still a challenge to remove the 'legacy' code as it is intermingled with logic. We will not be able to delete that as cleanly.
- seems more feasible/sensible to work this the other way round, to morph our existing code into the prototype that is being removed
- the approach in the prototype was pretty quick and when revisiting I started to break it up and get it test-able. This was not too bad an effort, but at same time it was almost easier using the new code as a guide; which was how that code was written based on the existing code. I think the net effect is I have a bit more of a worked out map of where to go.
- the new UI was going really well for the single player case and could even launch a game. Adding multiplayer chat was not cleanly done, then integrating with a multiplayer game was getting even worse. At some point an assumption was then baked in that a person had a default map loaded (98% of the time that's true). Coming back to fix the NPE, the cost-benefit of doing an in-place refactor vs cleanup then rewrite model code to fit new UI; seemed like an in-place cleanup is a winner. Having gone that route for a little bit, it seems to be effective and the benefits being merged to master are immediate (compared to merging updates to code that is behind a feature flag).

For reference, the UI updates were previously discussed on forum: https://forums.triplea-game.org/topic/504/updating-initial-screens-and-game-hosting-screen/

* Whitespace change, retrigger the build
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion team communication thread, meant for coordination and decision making
Projects
None yet
Development

No branches or pull requests

2 participants