Clarification Regarding Bug 2473 - Overstocking Vendors

Everything about development and the OpenMW source code.
Post Reply
Pfxn
Posts: 2
Joined: 05 Jul 2016, 00:28

Clarification Regarding Bug 2473 - Overstocking Vendors

Post by Pfxn »

Trying to give back to the OpenMW community with an attempt at a patch for OpenMW bug 2473 (https://bugs.openmw.org/issues/2473). This is requesting that OpenMW restore the functionality from vanilla where vendors restock with the larger of the base restock number OR the maximum size stack that the player has left that particular vendor with.

As an example of this behavior in action:
Open barter with Ajira (Balmora, Guild of Mages) - she has 5 kwama cuttle (her base restock count)
Buy 5 kwama cuttle, close barter, open barter - she has 5, you have 5 (she restocked)
Sell 5 kwama cuttle, close barter, open barter - she has 10, you have 0 (no restock was necessary)
Buy 10 kwama cuttle, close barter, open barter - she has 10, you have 10 (she restocked with 10, not 5)
I hope that makes the expected Vanilla behavior clear.

I would like input on Implementation:

The logic for restocking behavior is found in MWWorld::ContainerStore::restock. The full function signature is:

Code: Select all

void MWWorld::ContainerStore::restock (const ESM::InventoryList& items, const MWWorld::Ptr& ptr, const std::string& owner);
The count used to restock is contained in the items argument (ESM::InventoryList wraps a std::vector<ContItem>; ContItem is just a struct with the count and name.) Note the const modifier. I would like to overwrite the count stored in ContItem with the larger of it's current value or the vendor count, but that would require that this function drop the const qualifier on the InventoryList. This loss of constness means numerous other functions which call ContainerStore::restock() must drop their constness guarantee as well. In principle I'm okay with that as I really would like to modify the InventoryList in some cases, so const doesn't make sense here, but I want to make sure you all feel the same way.

The other option is to leave the function signature as is and come up with some other place to record the fact that the player has sold back some larger amount than the base restock size, and to restock using that value whenever it's greater than the base restock size.

I'm happy to wrap all of this logic with a configuration setting that can TURN THIS FUNCTIONALITY OFF for those who feel like it's cheating, but Scrawl's comments on the bug report suggest that we want the vanilla behavior by default.

Today I have a branch that just const_casts this away as a proof of concept before I go re-plumbing the whole call chain to remove the const. How should I proceed with a correct implementation that will be accepted into the code base?
User avatar
scrawl
Posts: 2152
Joined: 18 Feb 2012, 11:51

Re: Clarification Regarding Bug 2473 - Overstocking Vendors

Post by scrawl »

You can not modify the ESM::InventoryList. The const is there for a reason.

An ESM::InventoryList belongs to an ESM::NPC. An ESM::NPC is just a template for NPC instances. There can be many instances using this same template. If you modified the InventoryList then all instances of this ESM::NPC would be affected, even though only one should be affected.

Not to mention that changes made to ESM::InventoryList would not persist when you save and reload the game.
Pfxn
Posts: 2
Joined: 05 Jul 2016, 00:28

Re: Clarification Regarding Bug 2473 - Overstocking Vendors

Post by Pfxn »

Okay, my current approach won't work. Do you have a suggestion as to which class or container might be the most logical place to store and keep track of the maximum value that the NPC store has been left with?

Perhaps this should persist by being saved/loaded in the InventoryState class which is passed through storeStates()/getState()?
Post Reply