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 functionality for DebugLines #1766

Merged
merged 10 commits into from Jun 29, 2019

Conversation

@Max-Rensen
Copy link
Contributor

commented Jun 27, 2019

Description

This is is a possible solution for #1651. It adds some simple methods inside DebugLines and DebugLinesComponent that add new shapes consisting of .

Additions

  • Add add_square, add_rotated_square, add_box, add_rotated_box, add_circle, add_rotated_circle, add_sphere functions to DebugLinesComponent and the corresponding draw functions to DebugLines to draw simple shapes with debug lines.

PR Checklist

By placing an x in the boxes I certify that I have:

  • Ran cargo test --all locally if this modified any rs files.
  • Ran cargo +stable fmt --all locally if this modified any rs files.
  • Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment.
  • Added unit tests for new APIs if any were added in this PR.
  • Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme.

Max-Rensen added some commits Jun 27, 2019

Max-Rensen added some commits Jun 27, 2019

@codecov

This comment has been minimized.

Copy link

commented Jun 28, 2019

Codecov Report

Merging #1766 into master will increase coverage by 21.79%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1766       +/-   ##
===========================================
+ Coverage   65.65%   87.44%   +21.79%     
===========================================
  Files          99       70       -29     
  Lines        6580     4803     -1777     
===========================================
- Hits         4320     4200      -120     
+ Misses       2260      603     -1657
Impacted Files Coverage Δ
amethyst_assets/src/loader.rs 93.54% <ø> (+54.35%) ⬆️
amethyst_ui/src/lib.rs 0% <0%> (-100%) ⬇️
src/lib.rs 87.87% <0%> (-12.13%) ⬇️
src/state.rs 71.69% <0%> (-7.75%) ⬇️
amethyst_core/src/timing.rs 98.58% <0%> (-1.42%) ⬇️
amethyst_core/src/float.rs 96.1% <0%> (-0.15%) ⬇️
...methyst_core/src/transform/components/transform.rs 98.36% <0%> (-0.14%) ⬇️
amethyst_input/src/input_handler.rs 86.99% <0%> (-0.08%) ⬇️
amethyst_assets/src/reload.rs 0% <0%> (ø) ⬆️
amethyst_ui/src/format.rs 0% <0%> (ø) ⬆️
... and 53 more

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 5a24cc5...173dee3. Read the comment docs.

amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
amethyst_rendy/src/debug_drawing.rs Outdated Show resolved Hide resolved
@Max-Rensen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

I was wondering if I should just use the add_rotated_... in the add_..., it's a bit slower that way, because it still has to calculate the rotations, but I don't know if that matters as it's just for debugging purposes.
Also, I added a cylinder, which might be useful for debugging.

@Frizi

Frizi approved these changes Jun 29, 2019

@torkleyy
Copy link
Member

left a comment

Looks good otherwise!

docs/CHANGELOG.md Outdated Show resolved Hide resolved
@torkleyy
Copy link
Member

left a comment

Thanks!
bors r+

bors bot added a commit that referenced this pull request Jun 29, 2019

Merge #1766
1766: New functionality for DebugLines r=torkleyy a=Max-Rensen

## Description

This is is a possible solution for #1651. It adds some simple methods inside `DebugLines` and `DebugLinesComponent` that add new shapes consisting of .

## Additions

- Add `add_square`, `add_rotated_square`, `add_box`, `add_rotated_box`, `add_circle`, `add_rotated_circle`, `add_sphere` functions to `DebugLinesComponent` and the corresponding draw functions to `DebugLines` to draw simple shapes with debug lines.

## PR Checklist

By placing an x in the boxes I certify that I have:

- [x] Ran `cargo test --all` locally if this modified any rs files.
- [x] Ran `cargo +stable fmt --all` locally if this modified any rs files.
- [x] Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment.
- [ ] Added unit tests for new APIs if any were added in this PR.
- [x] Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme.


Co-authored-by: Max Rensen <40267916+max-rensen@users.noreply.github.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

@torkleyy

This comment has been minimized.

Copy link
Member

commented Jun 29, 2019

bors r+

cc @azriel91 Not sure about this segfault, is this a kcov bug?

bors bot added a commit that referenced this pull request Jun 29, 2019

Merge #1766
1766: New functionality for DebugLines r=torkleyy a=Max-Rensen

## Description

This is is a possible solution for #1651. It adds some simple methods inside `DebugLines` and `DebugLinesComponent` that add new shapes consisting of .

## Additions

- Add `add_square`, `add_rotated_square`, `add_box`, `add_rotated_box`, `add_circle`, `add_rotated_circle`, `add_sphere` functions to `DebugLinesComponent` and the corresponding draw functions to `DebugLines` to draw simple shapes with debug lines.

## PR Checklist

By placing an x in the boxes I certify that I have:

- [x] Ran `cargo test --all` locally if this modified any rs files.
- [x] Ran `cargo +stable fmt --all` locally if this modified any rs files.
- [x] Added a changelog entry if this will impact users, or modified more than 5 lines of Rust that wasn't a doc comment.
- [ ] Added unit tests for new APIs if any were added in this PR.
- [x] Acknowledged that by making this pull request I release this code under an MIT/Apache 2.0 dual licensing scheme.


Co-authored-by: Max Rensen <40267916+max-rensen@users.noreply.github.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

@bors bors bot merged commit 173dee3 into amethyst:master Jun 29, 2019

2 checks passed

bors Build succeeded
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
@azriel91

This comment has been minimized.

Copy link
Member

commented Jun 29, 2019

I don't think it's kcov (nothing related on their issue tracker).

  • Exit code 139 means bash exited by a signal 21 (bash fatal error 128 + signal).

  • 21 is SIGTTIN Terminal Input

  • The linux kernel sends SIGTTIN to a process when:

    • The process is in the background.
    • The process wants to read input from stdin.
    • OS wants to tell the process it won't get any input because it's in the background.

    (or something similar)

I suspect that kcov is returning the exit code of the inner application, but need to get the script to output which one it's running to tell which it is.

Opened #1775 to get more info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.