The bug
Villagers do not properly keep track of the last time they restocked, neither the number of times they have restocked in the day. This causes villagers indefinitely restocking each time they visit their POI during their work hours. This happens with each villager once the world has passed a a half-day of gametime.
How to reproduce
Let the world gametime reach at least 12,000 ticks (
/time query gametime
to check).Spawn a villager in an empty superflat world with access to a POI
Trade with this villager during work hours
❌ The villager will restock every time he works at his workstation, leading to the possibility of abusing his trades (@unknown's 2nd screenshot shows 4 stacks of emeralds obtainable in one day), even while the GUI is still open (@unknown's video:
[media]).
⚠️ As a result of the restocks not being tracked, the demand being reduced at every restock will absurdly go down, in the wrong direction (@unknown's 1st screenshot, making the prices not affected by an excessive use of the same trades.
:info: The nbt values LastRestock
and RestocksToday
always stay at 0.
Code analysis by @unknown:
This method (own mappings) from the Entity Villager class performs the restocking (set trades use
values to 0) and set tracking values which are used for the hasNotAlreadyRestocked() condition further below:
public void restock() {
this.goSetDemand();
Iterator iterator = this.getOffers().iterator();
// Do the restock
while (iterator.hasNext()) {
MerchantRecipe merchantrecipe = (MerchantRecipe) iterator.next();
merchantrecipe.resetUses();
}
// (…)
if (this.getVillagerData().getProfession() == VillagerProfession.FARMER) {
this.eE();
}
// Set the tracking values
this.lastRestock = this.world.getTime();
++this.restocksToday;
}
It is called each time the villager works at his workstation, provided the following requirement is met:
public boolean isRestockNeeded() {
// This is for wether or not calling for demand
long i = this.lastRestock + 12000L;
boolean flag = this.world.getTime() > i;
long j = this.world.getDayTime();
if (this.bP > 0L) {
long k = this.bP / 24000L;
long l = j / 24000L;
flag |= l > k;
}
this.bP = j;
if (flag) {
// method preparing to set the demand
}
// Real conditions
return this.hasNotAlreadyRestocked() && this.IsOutOfStock();
}
The method isOutOfStock() checks whether any stock has been depleted:
private boolean isOutOfSTock() {
Iterator iterator = this.getOffers().iterator();
MerchantRecipe merchantrecipe;
do {
if (!iterator.hasNext()) {
return false; // Can't find any fully used offer
}
merchantrecipe = (MerchantRecipe) iterator.next();
} while (!merchantrecipe.isFullyUsed());
return true;
}
Yet, the restock can already be potentially done inside the method called for demand:
private void prepareforSettingDemand() {
int i = 2 - this.restocksToday;
// This creates the issue: restock is prematurely been made here.
if (i > 0) {
Iterator iterator = this.getOffers().iterator();
while (iterator.hasNext()) {
MerchantRecipe merchantrecipe = (MerchantRecipe) iterator.next();
merchantrecipe.resetUses();
}
}
for (int j = 0; j < i; ++j) {
this.setDemand();
}
}
❌ This leads to isRestockNeeded() always be false once a certain amount of gametime has passed, because the restock is already done in the call for demand methods, but not tracked at all. Thus properly restocking is not possible.
Linked issues
is duplicated by 8
relates to 1
Attachments
Comments 12
Hi Johnibur. Thanks for condensing the ticket. It's the first one I have made.
What do labels do? Why the change to ones with no other tickets?
They are usefull when adding meta informations for internal game elements (blocks, entities, functions, NBT) or external libraries. These ones are new NBT, so they hadn't matched anything yet on this tracker.
This creates a fairly serious exploit. Trading with villagers at their poi is effectively infinite. Can the ticket be updated to show the severity?
Found out the cause of this issue: the action of restocking is performed inside of 2 different methods (while preparing for setting the demand value, and while doing the proper restock), where one of them is preceding the other and without keeping track of the restock (see code analysis in the description).
yeah i cured a zombie and got a tools smith that will infinitely trade at ridiculous sale prices if I give it a smithing table. Running 1.14.4 Java edition.
[media]I despise this fix. I mean, how is infinite restocks broken? It's not like emerald weapons and armor have been added yet. Could Mojang at least change the amount of restocks to a more generous number, like 5? Or maybe increase the amount of restocks on lower difficulties? Either way, screw this fix.
This platform is for bug reports only. For feedback, head out to the feedback website.
The issue is that the villagers are not keeping track of last trade and today's trade amount. Updated this ticket to emphasize this fact. This causes the trading exploit, and the demand to go down, but a priori it's a consequence of the root cause of this ticket − restocking not being properly tracked. Hence restocking too many times will naturally lead to demand plummeting. If you find another issue with demand, please create a new ticket. Note that demand was recently fixed in MC-156574.