Skip to content

Conversation

@royfalk
Copy link
Contributor

@royfalk royfalk commented Dec 29, 2025

as volume was 0 and there was a div by 0 error.

Closes #1411

Code Changes:

  • Have the PR Validation Tests been run? No. Just the bug fix.

as volume was 0 and there was a div by 0 error.
Copy link

@kheckwrecker kheckwrecker left a comment

Choose a reason for hiding this comment

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

Looks good to me.

// Check the maximum you can fit in your hold, but only if volume != 0
if(item->GetVolume() > 0) {
int max_stackable_quantity = static_cast<int>(std::floor(hold->AvailableCapacity() / item->GetVolume()));
quantity = std::min(max_stackable_quantity, quantity);
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this also have a std::max with 0 so it doesn't go negative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this is a bit complex. We have a 'mult_shady_moreupgrade' upgrade, which could show up as negative volume but for now is implemented as volume=0.1.
In reality, if we have a negative volume for actual upgrades, this will have issues elsewhere as well.
Better to fix this properly in cargo.

Copy link
Member

Choose a reason for hiding this comment

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

while I agree we should fix it elsewhere too, we should also protect ourselves here as well.

- Prevent weapons from costing upgrade volume
- Refactor concluseTransaction
- Remove superfluous upgradeNotAddedToCargo function.
@royfalk
Copy link
Contributor Author

royfalk commented Jan 5, 2026

Last commit belongs in task_fix_1488. Reverting.

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

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

Unable to buy armor

4 participants