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

feat: mobile main hud #169

Merged
merged 12 commits into from
Jan 19, 2024
Merged

feat: mobile main hud #169

merged 12 commits into from
Jan 19, 2024

Conversation

kuruk-mm
Copy link
Member

@kuruk-mm kuruk-mm commented Jan 16, 2024

  • Change Icons in the HUD
  • Screen scaling. The base resolution for the UI is 1280x720, and if you change the size, the UI will resize to match that size.
  • Menu navigation UI improved, header increased the size
  • Joystick now works in an left-bottom area of the screen
  • Chat only is shown when it's used (on the future, we are going to implement last 5 messages notifications above it)
  • Implement SafeAreaHUD, an area that the "interactive" UI must be drawn. This is for avoiding physicals parts in the Mobile phone (like a camera, or some parts of it) that you cannot interact

@kuruk-mm kuruk-mm marked this pull request as ready for review January 17, 2024 02:25
Copy link
Collaborator

@leanmendoza leanmendoza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor reviews
Regarding the tests:

  • Download the snapshots.zip from the CI summary and get the right snapshots to replace in repo/tests/snapshots
  • Before replacing text-shape's snapshots, see what and why they're changing

.github/workflows/android-build.yml Outdated Show resolved Hide resolved
godot/.godot/global_script_class_cache.cfg Outdated Show resolved Hide resolved
godot/src/logic/player/player.gd Show resolved Hide resolved
)
resolution_manager.change_resolution(get_window(), get_viewport(), Global.config.resolution)
resolution_manager.change_ui_scale(get_window(), Global.config.ui_scale)
#resolution_manager.change_window_size(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead code
this is still in the settings

@@ -73,8 +73,12 @@ func _ready() -> void:
hide()


func custom_input(event: InputEvent) -> void:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you detail how this works?

fix: remove incorrect code from virtual joystick, that was already there
fix: safe area margin now works correctly
Copy link
Collaborator

@leanmendoza leanmendoza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, looks good to me 😄
Just fix tests before merging

@codecov-commenter
Copy link

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (a027e05) 47.37% compared to head (4ec6b75) 47.40%.

Files Patch % Lines
...dot-lib/src/scene_runner/global_get_node_helper.rs 0.00% 1 Missing ⚠️
...raland-godot-lib/src/scene_runner/scene_manager.rs 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #169      +/-   ##
==========================================
+ Coverage   47.37%   47.40%   +0.02%     
==========================================
  Files         140      140              
  Lines       16850    16868      +18     
==========================================
+ Hits         7983     7996      +13     
- Misses       8867     8872       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kuruk-mm kuruk-mm merged commit 2626dfc into main Jan 19, 2024
3 checks passed
@kuruk-mm kuruk-mm deleted the feat/mobile-main-hud branch January 19, 2024 18:49
This was referenced Jan 19, 2024
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.

3 participants