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 errors in Weapon::Get_Percent_Ready_To_Fire #928

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

bobtista
Copy link
Contributor

@bobtista bobtista commented May 24, 2023

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2023

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 2.53%. Comparing base (a8f8d8b) to head (9e13820).

Files Patch % Lines
src/game/logic/object/weapon.cpp 0.00% 10 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #928   +/-   ##
========================================
  Coverage     2.53%    2.53%           
========================================
  Files          949      949           
  Lines       110171   110171           
  Branches     18880    18880           
========================================
  Hits          2797     2797           
  Misses      106972   106972           
  Partials       402      402           

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

@xezon xezon requested a review from jonwil May 25, 2023 05:24
@jonwil
Copy link
Contributor

jonwil commented May 25, 2023

Having reviewed the original code, the correct fix is to rename time_left to total_time and total_time to time_left.
So
unsigned int time_left = next_shot - m_whenLastReloadStarted;
becomes
unsigned int total_time = next_shot - m_whenLastReloadStarted;
and
unsigned int total_time = next_shot - m_whenLastReloadStarted;
becomes
unsigned int time_left = next_shot - m_whenLastReloadStarted;

and the other uses of both variables get swapped as well.

And no we should not use the getter in this case.

@bobtista bobtista force-pushed the GetPercentReadyToFire branch 4 times, most recently from 7ebe7c5 to 2f7ade7 Compare May 25, 2023 21:00
@xezon
Copy link
Contributor

xezon commented May 26, 2023

Does change fix #916 confirmed?

Does it perhaps also fix #888?

@xezon xezon force-pushed the GetPercentReadyToFire branch from 2f7ade7 to 5584900 Compare February 15, 2024 18:12
Copy link
Contributor

@jonwil jonwil left a comment

Choose a reason for hiding this comment

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

Verified that the change in this PR is correct. Approved.

@xezon xezon changed the title fixes math for weapon reload time_left and total_time Fixes math for weapon reload time_left and total_time Mar 6, 2024
@xezon xezon force-pushed the GetPercentReadyToFire branch from 5584900 to 9e13820 Compare March 6, 2024 09:13
@xezon
Copy link
Contributor

xezon commented Mar 6, 2024

I will check later if this fixes the bugs.

@xezon
Copy link
Contributor

xezon commented Mar 6, 2024

@xezon xezon changed the title Fixes math for weapon reload time_left and total_time Fix errors in Weapon::Get_Percent_Ready_To_Fire Mar 6, 2024
@xezon xezon merged commit d9e3e41 into TheAssemblyArmada:develop Mar 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assert in Weapon::Get_Percent_Ready_To_Fire() is hit after using clear mines ability
5 participants