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 crash in armor coverage test #55037

Merged
merged 1 commit into from
Feb 3, 2022
Merged

Conversation

BrettDong
Copy link
Member

Summary

Bugfixes "Fix crash in armor coverage test"

Purpose of change

Tests have been consistently crashing on MinGW recently, and also crashes randomly in GCC 9 Linux configuration.

Describe the solution

The test case spawns two test NPCs, one of them overlaps with avatar character, causing very weird behaviors in projectile attack and damage handling code. This pull request shifts the position of test creatures by 4 tiles and seemingly solves the crash.

Describe alternatives you've considered

Testing

Wait for general build matrix to pass on all build configurations.

Additional context

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Feb 2, 2022
@BrettDong BrettDong added <Bugfix> This is a fix for a bug (or closes open issue) Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` labels Feb 2, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Feb 3, 2022
@BrettDong
Copy link
Member Author

This pull request also vastly shortens the time spent on armor coverage tests:

(~[slow] ~[.])=> 2.477 s: Full melee and ranged coverage vs. ranged attack
(~[slow] ~[.])=> 2.477 s: Ranged coverage vs. bullet
(~[slow] ~[.])=> 2.301 s: No ranged coverage vs. ranged attack
(~[slow] ~[.])=> 2.301 s: Ranged coverage vs. bullet

master branch:

(~[slow] ~[.])=> 104.525 s: Full melee and ranged coverage vs. ranged attack
(~[slow] ~[.])=> 104.525 s: Ranged coverage vs. bullet
(~[slow] ~[.])=> 105.030 s: No ranged coverage vs. ranged attack
(~[slow] ~[.])=> 105.031 s: Ranged coverage vs. bullet

@akrieger
Copy link
Member

akrieger commented Feb 3, 2022

I'm kinda shocked by the test runtime reduction. Was it hitting some edge case in some pathing/visibility/aim algorithm or something?

@kevingranade kevingranade merged commit edc3055 into CleverRaven:master Feb 3, 2022
@BrettDong
Copy link
Member Author

I'm kinda shocked by the test runtime reduction. Was it hitting some edge case in some pathing/visibility/aim algorithm or something?

When I was stepping in the debugger to troubleshoot the use-after-free error, I incidentally found that the majority of time is spent on handling projectile attack damage on our avatar character rather than the test dummy NPCs.

@BrettDong BrettDong deleted the pos branch February 3, 2022 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash in MinGW cross-compile CI test
3 participants