cataclysmbnteam / Cataclysm-BN

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

Early termination in shopkeeper inventory generation. #590

Open chaosvolt opened 3 years ago

chaosvolt commented 3 years ago

Describe the bug

Per discussion in https://github.com/cataclysmbnteam/Cataclysm-BN/pull/553 it was discovered that shopkeeper NPCs have a hardcoded function in their inventory generation, that gives it a chance of giving early which every spawn roll after the 10th item.

Steps To Reproduce

  1. Bump up some easy-to-notice items at the bottom of the evac merchant's shopkeeper inventory to 100 prob.
  2. Spawn in the refugee center and trade with the evac merchant.
  3. Note that the desired items, despite being given a 100% chance of appearing, will in practice almost never spawn.

Expected behavior

Shopkeepers have what the JSON author intends them to have, and at the desired rates, making balance of inventory spawns easier to ascertain and control.

Screenshots

Versions and configuration

Additional context

Relevant section of code noticed by Coolthuhlu during discussion in that PR:

    int count = 0;
    bool last_item = false;
    while( shop_value > 0 && total_space > 0_ml && !last_item ) {
        item tmpit = item_group::item_from( from, calendar::turn );
        if( !tmpit.is_null() && total_space >= tmpit.volume() ) {
            tmpit.set_owner( *this );
            ret.push_back( tmpit );
            shop_value -= tmpit.price( true );
            total_space -= tmpit.volume();
            count += 1;
            last_item = count > 10 && one_in( 100 );
        }
    }

I tried at one point testing what would happen if I changed it to this:

    while( shop_value > 0 && total_space > 0_ml ) {
        item tmpit = item_group::item_from( from, calendar::turn );
        if( !tmpit.is_null() && total_space >= tmpit.volume() ) {
            tmpit.set_owner( *this );
            ret.push_back( tmpit );
            shop_value -= tmpit.price( true );
            total_space -= tmpit.volume();
        }
    }

And found the issue still persisted.

RoyalFox2140 commented 1 week ago

@chaosvolt https://github.com/cataclysmbnteam/Cataclysm-BN/pull/4637 Please close if fixed by this.

chaosvolt commented 1 week ago
  1. Set gatling shotgun in NC_EVAC_SHOPKEEP_misc to appear 100% of the time.
  2. Started game in a refugee center.
  3. Found no gatling shotgun in merchant's inventory.

image

The list DOES seem to be a lot better-populated than I remember it being however, so we can potentially close this since it's possible the issue fixed by that PR may have been the bigger problem than the early termination. It's also possible that Coolthlhu's earlier solution of removing that function might actually work now that the itemgroup actually generates correctly on game start.