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

Fix lobby menu & Use Menu Builder #5188

Merged
merged 2 commits into from
Sep 10, 2019
Merged

Fix lobby menu & Use Menu Builder #5188

merged 2 commits into from
Sep 10, 2019

Conversation

DanVanAtta
Copy link
Member

Overview

  • Refactoring mistake added the sound options to the main lobby menu rather than the settings submenu. Commit (1) fixes this: 7097918

  • Commit (2) refactors the lobby menu to use the builder API. This reduces a lot of the boiler plate code and makes it very difficult to add sub-menu's to the wrong menu.

Functional Changes

[ ] New map or map update
[ ] New Feature
[ ] Feature update or enhancement
[ ] Feature Removal
[x] Code Cleanup or refactor
[ ] Configuration Change
[x] Bug fix

Testing

[ ] Covered by existing automated tests
[ ] Covered by newly added automated tests
[x] Manually tested
[ ] No testing done

Launched local lobby and connected to it locally to verify the menu.

Screens Shots

Before

Screenshot from 2019-09-09 15-33-21

After

Screenshot from 2019-09-09 15-33-17

Add sound options to the 'Settings' menu and not the menubar.
@codecov-io
Copy link

Codecov Report

Merging #5188 into master will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5188      +/-   ##
============================================
+ Coverage     24.08%   24.09%   +0.01%     
- Complexity     6763     6765       +2     
============================================
  Files          1016     1016              
  Lines         77836    77802      -34     
  Branches      11598    11597       -1     
============================================
+ Hits          18746    18748       +2     
+ Misses        56956    56922      -34     
+ Partials       2134     2132       -2
Impacted Files Coverage Δ Complexity Δ
...a/games/strategy/triplea/ui/menubar/LobbyMenu.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../strategy/triplea/odds/calculator/DummyPlayer.java 51.64% <0%> (+2.19%) 15% <0%> (+2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28a7ef8...ebe38d0. Read the comment docs.

@ron-murhammer ron-murhammer merged commit 5686833 into triplea-game:master Sep 10, 2019
@DanVanAtta DanVanAtta deleted the lobby-menu-fix branch September 10, 2019 05:15
@RoiEXLab
Copy link
Member

Hmm looks like coverage is still somewhat flaky.
DummyPlayer shouldn't have been affected by this

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

Successfully merging this pull request may close these issues.

4 participants