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

Integrate screenPickerPlugin with gizmosPlugin #938

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

tomas7770
Copy link
Contributor

@tomas7770 tomas7770 commented Feb 1, 2024

Description

Changes gizmosPlugin to store gizmo ids in screenPickerPlugin instead of a separate buffer.

Checklist

  • Self-review changes.
  • Evaluate impact on the documentation.
  • Ensure test coverage.
  • Write new samples.
  • Add entry to the changelog's unreleased section.

@tomas7770 tomas7770 linked an issue Feb 1, 2024 that may be closed by this pull request
@github-actions github-actions bot added A-Engine B-Gizmos S-Blocked Blocked on another issue or PR labels Feb 1, 2024
Copy link
Contributor

github-actions bot commented Feb 1, 2024

PR Preview Action v1.4.6
🚀 Deployed preview to https://GameDevTecnico.github.io/cubos/docs-preview/pr-938/
on branch gh-pages at 2024-02-01 23:31 UTC

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

engine/src/cubos/engine/gizmos/gizmos.cpp Outdated Show resolved Hide resolved
engine/include/cubos/engine/gizmos/gizmos.hpp Outdated Show resolved Hide resolved
engine/src/cubos/engine/gizmos/gizmos.cpp Outdated Show resolved Hide resolved
engine/include/cubos/engine/gizmos/gizmos.hpp Outdated Show resolved Hide resolved
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

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

Comparison is base (a347008) 34.43% compared to head (48ef38f) 34.46%.

Files Patch % Lines
engine/src/cubos/engine/gizmos/plugin.cpp 0.00% 23 Missing ⚠️
engine/src/cubos/engine/gizmos/gizmos.cpp 0.00% 4 Missing ⚠️
...e/src/cubos/engine/screen_picker/screen_picker.cpp 0.00% 2 Missing ⚠️
engine/src/cubos/engine/gizmos/renderer.cpp 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #938      +/-   ##
==========================================
+ Coverage   34.43%   34.46%   +0.03%     
==========================================
  Files         283      283              
  Lines       23710    23687      -23     
==========================================
  Hits         8164     8164              
+ Misses      15546    15523      -23     

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

@tomas7770 tomas7770 force-pushed the 870-integrate-screenpickerplugin-with-gizmosplugin branch from 66778c0 to 98b8b70 Compare February 1, 2024 23:37
@github-actions github-actions bot dismissed their stale review February 1, 2024 23:38

No Clang-Tidy warnings found so I assume my comments were addressed

@tomas7770 tomas7770 force-pushed the 870-integrate-screenpickerplugin-with-gizmosplugin branch from 98b8b70 to 2df0609 Compare February 1, 2024 23:44
@tomas7770 tomas7770 marked this pull request as ready for review February 1, 2024 23:52
Copy link
Member

@RiscadoA RiscadoA left a comment

Choose a reason for hiding this comment

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

Damn that was fast 👀 Thanks for working on this! The code LGTM, also ran it locally, it did fix that bug where clicking the transform gizmo deselected the entity. Feel free to merge

@tomas7770 tomas7770 force-pushed the 870-integrate-screenpickerplugin-with-gizmosplugin branch from 2df0609 to 48ef38f Compare February 2, 2024 12:54
@tomas7770 tomas7770 merged commit d4cbd61 into main Feb 2, 2024
11 checks passed
@tomas7770 tomas7770 deleted the 870-integrate-screenpickerplugin-with-gizmosplugin branch February 2, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Engine B-Gizmos S-Blocked Blocked on another issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate screenPickerPlugin with gizmosPlugin
2 participants