cataclysmbnteam / Cataclysm-BN

Cataclysm: Bright Nights, A fork/variant of Cataclysm:DDA by CleverRaven.
https://docs.cataclysmbn.org
Other
662 stars 260 forks source link

Barter price is calculated on load, not on finalize #556

Open Coolthulhu opened 3 years ago

Coolthulhu commented 3 years ago

Expected: black powder 9mm costs 30% of factory made smokeless 9mm Got: black powder 9mm has the useless "pre-apoc" price at 30%, but barter price is the same as for factory made

chaosvolt commented 3 years ago

That suggests that modifiers to price literally only modifiy price, since the post-apoc price is a separate JSON value. Test adding a modifier to the post-apoc price and see if it works? If so, that means a loooot of items are going to need a tedious JSON update, unless there's a baked-in "also update post-apoc price if you see price being modified" code that's supposed to work but doesn't?

olanti-p commented 3 years ago

Just tested @chaosvolt's suggestion, and he's correct: price and price_postapoc are independent, and must be defined separately in proportional. Black powder 9mm defines proportional only for the pre-apoc price, the post-apoc one is unaffected.

The only code I found that ties the two prices together is a fallback for price_postapoc:

If price_postapoc is defined as negative amount, it should be replaced with the final pre-apoc price: https://github.com/cataclysmbnteam/Cataclysm-BN/blob/1943ad1a02e8aff68c29b444bd7b21e7fb0efcf5/src/item_factory.cpp#L165-L168 Except it's broken since you can't define a negative price value:

Error: Json error: data/mods/dda/../../json/items/ammo/9mm.json:10:22: value outside supported range

    "volume": "250 ml",
    "price": 150,
    "price_postapoc":
                     ^
                      "-1 cent",
    "flags": [ "IRREPLACEABLE_CONSUMABLE" ],
    "material": [ "brass", "powder" ],

Considering it defaults to 0 if not specified: https://github.com/cataclysmbnteam/Cataclysm-BN/blob/1943ad1a02e8aff68c29b444bd7b21e7fb0efcf5/src/item_factory.cpp#L2303-L2304

I assume the condition was supposed to be if( obj.price_post <= 0_cent )


Btw, price_postapoc and price are such an awkward names. In 90% of cases we're dealing with the post-Cataclysm aka barter price, it should be the one called price with the other being price_preapoc or something. Pre-apoc is literally used only for vending machines and defence mode caravans.

Coolthulhu commented 3 years ago

The only reasons that non-barter price even exist are "realism" and vendor furniture. Non-barter prices are effectively obsolete.