Skip to content

Round tool stats display to 1 decimal#2872

Closed
GasSpace630 wants to merge 1 commit intoPixelGuys:masterfrom
GasSpace630:round-tool-stats
Closed

Round tool stats display to 1 decimal#2872
GasSpace630 wants to merge 1 commit intoPixelGuys:masterfrom
GasSpace630:round-tool-stats

Conversation

@GasSpace630
Copy link
Copy Markdown
Contributor

This PR replaces #2871 which fixes #2696
The previous PR contained unrelated changes, so it was closed.
This version only includes the formatting changes.

@Wunka Wunka moved this to Easy to Review in PRs to review Apr 11, 2026
Copy link
Copy Markdown
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Also please rebase, there was a refactor that affects this.

Comment thread src/items.zig Outdated
Comment thread src/items.zig Outdated
proceduralItem.getProperty(property.destination orelse continue).* += sum;
}
if (proceduralItem.damage < 1) proceduralItem.damage = 1/(2 - proceduralItem.damage);
if (proceduralItem.swingSpeed < 1) proceduralItem.swingSpeed = 1/(2 - proceduralItem.swingSpeed);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove this? This is required to prevent negative values.

@IntegratedQuantum
Copy link
Copy Markdown
Member

also CI failed, please run the formatter

@IntegratedQuantum IntegratedQuantum moved this from Easy to Review to In review in PRs to review Apr 12, 2026
Copy link
Copy Markdown
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Now you are only applying rounding to small and negative values, and they remain negative.

Comment thread src/items.zig Outdated
proceduralItem.getProperty(property.destination orelse continue).* += sum;
}
if (proceduralItem.damage < 1) proceduralItem.damage = 1/(2 - proceduralItem.damage);
if (proceduralItem.swingSpeed < 1) proceduralItem.swingSpeed = 1/(2 - proceduralItem.swingSpeed);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm asking you again: Why did you remove the old checks?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

dont worry i did because it would reduce noise and maybe make the stats cleaner
and about the negative values
at line:509 it just clamps it to 0.1

i can change this if this isnt the desired behavoiur.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The reason for this code to exist was to allow negative values and let them still impact the stats, even for large negative values. A clamp does not allow this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ohh... i didnt know we wanted -ve values, my bad 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

wait so if we have -ve value for damage does that heal the enemy?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No. The previous code basically mapped all negative values to positives following this function:
Screenshot at 2026-04-23 16-29-14

Copy link
Copy Markdown
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Please apply fixes after #2891

@GasSpace630 GasSpace630 reopened this Apr 25, 2026
@IntegratedQuantum
Copy link
Copy Markdown
Member

What you've done now is remove the code that rounds the actual values, instead of fixing it up with #2891.

I am annoyed by how many times I've had to review this PR, so I decided to quickly do this myself in 8b4d843

For the future I suggest to learn how to use git rebase, and try to be more careful with how you address conflicts. Furthermore please try to at least look at the changes you have made on github at least once to catch these mistakes yourself, instead of me having to point them out to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Apply rounding to tool damage numbers (?)

3 participants