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

Ammo and Clip Quantity Bug #767

Open
Quickmind01 opened this issue Sep 16, 2019 · 3 comments
Open

Ammo and Clip Quantity Bug #767

Quickmind01 opened this issue Sep 16, 2019 · 3 comments
Labels
Agent Equipment Something that affects equipping or maintaining Agents/Scientists/Engineers Battlescape An issue relating to something in the battle overview part of Apocalypse !BUG! low priority This is a bug in OpenApoc. It will need resolving when time allows. It does not always cause a CTD Cityscape An issue relating to something in the City overview part of Apocalypse Code Query Something doesn't seem right in the code. It may not be a bug, but it probably needs fixing! Enhancement This is a request for something that makes OpenApoc work better or more intuitively Marketplace / Economy Something that affects transactions or behaviour or logic in the Marketplace or Economy screens Vehicle Equipment Something that affects equipping or maintaining Vehicles Verified / Replicated This issue has been verified or replicated by a developer

Comments

@Quickmind01
Copy link

If I recall correctly, if the ammo is sold in this save, you will end up with the millions of clips bug again. The Heavy Plasma Clip is a good example. I had sold it as a partial clip, and when I rebought it, it was still a partial clip. Had I had a second full clip, it would have likely refilled. Also, I believe this clip bug affected my storage space.

This is using OpenApoc-v0.2.0.2-16-g2f96387e and V7.1 EWM.

save_Ammo and Clip Bug.zip

@JonnyH
Copy link
Collaborator

JonnyH commented Sep 16, 2019

It seems like the save already has a nonsense number of ammo clips, no need to buy/sell anything.

But this gives a good clue - it looks like the buy/sell code isn't checking if an ammo clip is partially full - it just takes the (MAX_AMMO) of one clip out of the stores.

But that naturally fails if you try to sell a non-full clip, as it takes MAX_AMMO off a value less than MAX_AMMO, causing an underflow (as it's an unsigned value, instead of going negative it ends up with UINT32_MAX - however many it would be below zero, so over 4 billion). Which neatly explains the issue.

So this raises the question of how we should handle partially filled clips - us trying to be "clever" and add features onto the OG by tracking the number of bullets not clips outside battles has a number of knock on effects that I'm not sure it's "worth" the extra complexity for a feature most people won't even notice:

  • Should buying/selling a partially filled clip still have the same cost as a full one?
  • The buy/sell UI really has no way of informing the user that a clip is partially filled, either buying or selling
  • There's already no way in-battle to merge partially filled clips, so if you're not careful you might end up in the situation where you fight multiple battles in a row and have multiple near-empty clips on each agent, which just seems frustrating

I might go through and slash and burn this feature and just auto-merge partially filled clips at the end of each battle. I'm not sure if this management really /adds/ anything to the game.

@JonnyH
Copy link
Collaborator

JonnyH commented Sep 16, 2019

So looking further into the code, it seems like the market doesn't actually support partial clips, after removing the number of rounds from the stores, the economy just tracks the number of clips.

That means you always buy full clips, even if you sold it partially filled - so a simple clamp here will magically refill a clip if you sell it and buy it back.

But as the selling will underflow to a CRAZY large number, so the buying it back will then overflow that back down to a sane number - back to where you started.

@makus82 makus82 added the !BUG! low priority This is a bug in OpenApoc. It will need resolving when time allows. It does not always cause a CTD label Sep 16, 2019
@Paddywhacker
Copy link

If you want a vote then I would vote for not having partial clip management in bases at all, and let all clips be refilled on return to base, along with personal teleports and shields being recharged.

@FilmBoy84 FilmBoy84 added Agent Equipment Something that affects equipping or maintaining Agents/Scientists/Engineers Battlescape An issue relating to something in the battle overview part of Apocalypse Cityscape An issue relating to something in the City overview part of Apocalypse Code Query Something doesn't seem right in the code. It may not be a bug, but it probably needs fixing! Enhancement This is a request for something that makes OpenApoc work better or more intuitively Marketplace / Economy Something that affects transactions or behaviour or logic in the Marketplace or Economy screens Vehicle Equipment Something that affects equipping or maintaining Vehicles Verified / Replicated This issue has been verified or replicated by a developer labels Oct 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Agent Equipment Something that affects equipping or maintaining Agents/Scientists/Engineers Battlescape An issue relating to something in the battle overview part of Apocalypse !BUG! low priority This is a bug in OpenApoc. It will need resolving when time allows. It does not always cause a CTD Cityscape An issue relating to something in the City overview part of Apocalypse Code Query Something doesn't seem right in the code. It may not be a bug, but it probably needs fixing! Enhancement This is a request for something that makes OpenApoc work better or more intuitively Marketplace / Economy Something that affects transactions or behaviour or logic in the Marketplace or Economy screens Vehicle Equipment Something that affects equipping or maintaining Vehicles Verified / Replicated This issue has been verified or replicated by a developer
Projects
None yet
Development

No branches or pull requests

5 participants