The bug
Villagers can get into a state where all there trades are locked.
Cases
Unloading villager during refresh
This bug happens when you unload the villager before it refreshed its trades.
How to reproduce
Spawn a villager
Get all items it wants for its first trades in your inventory. It is easier if you are in Creative to be able to clone items if you need more.
Start trading with the villager
Without closing the GUI use all trades until they are disabled
Close the GUI and directly afterwards close the world
Load the world again
→ The trades will still be disabled and won't be enabled again
In some cases, such as when trading with Leather Workers or other villagers with low numbers of total trades, it is possible to skip steps 5 and 6 as those villagers can have all trades become locked without closing the world.
Code analysis
Based on 1.11.2 decompiled using MCP 9.35 rc1
The fields net.minecraft.entity.passive.EntityVillager.timeUntilReset
and net.minecraft.entity.passive.EntityVillager.needsInitilization
are not saved to NBT.
Suggested fix
See this comment.
Trade not being used for the first time and having bad luck
Based on 1.11.2 decompiled using MCP 9.35 rc1
As of 1.11.2 (and earlier versions) the method net.minecraft.entity.passive.EntityVillager.useRecipe(MerchantRecipe)
only enables all disabled trades if a trade was used for the first time or with a one fifth chance. This means if you have bad luck, you can end up with all trades locked as well.
It is not known if this is intended or if the developers just did not want that trading any trade always enables all disabled trades, but forgot that it might happen that all trades get disabled.
Fix for affected villagers
This comment describes a fix for already affected villagers in case it is not intended that villagers with only disabled trades exist.
Remark: Consider adventure maps, where sometimes all trades are intentionally locked. Unlocking them when loading the villager might break the map, because players can get item they shouldn't have.
Linked issues
is duplicated by 9
Attachments
Comments 108
"This is a GUI issue."
Why? Doesn't seem like a GUI issue to me-- or at least not only a GUI issue.
From MinecraftWiki: Villagers will remove offers if the offer has been used some number of times and it is not the villager's only offer. http://www.minecraftwiki.net/wiki/Trading
Is that what's going on here? Do you think that's what's happening but the GUI is indeed failing to update?
It is supposed to only allow a certain number of trades, which is slightly random. However for some reason the X won't appear, but the trade does stop. Closing and opening should show the X, no?
My apologies. I was using an altered texture pack, i though i had everything updated to 1.4.x, but apparently i missed the X.
My understanding was that you could trade as much of an item as you wanted as long as you don't leave the trading screen. I.E. that a trade was only removed once you leave the trading GUI. This is apparently no longer the case.
But there is still the issue that "removed" trades stay in the GUI – even sometimes if there are no other trade options.
I will update my bug report.
Trades remain because they can come back.
"Trades remain because they can come back."
How is that supposed to happen? And what's the point of showing me only some of the possible items that villager X currently won't trade with me.
I have one villager who is only offering me one X-ed out trade. What am i supposed to do get him to update his one invalid offer?
When you do enough trades they will all unlock again
If i trade with villager A, eventually that will unlock a trade with villager B? I've never observed that. As i mentioned, some of my villagers have only a single, locked trade offer.
You only unlock the trades if you use the last trade a villager have.
Every time you make the last trade, the villager have a chance to get a new trade and/or renew a locked offer. It is possible however to lock the last trade and don't get a new last offer. In that case, trades would never get unlocked again for that villager.
I think the trading needs a few more tweeks.
If the villagers last trade becomes locked, then you cannot trade any more with them, as the other trade options can still become locked eventually.
Wouldn't a simple fix be, the last trade option is unable to be locked out? If it's something like paper for emeralds, this still wouldn't be unbalanced as eventually they'll get a new trade, and this one will no longer be the last trade.
I've seen this too, in 1.4.6, and it wasn't just the villager's last trade, but the one and only trade he ever offered, so I'll never be able to trade anything more with him ever. My setup looks just like the original screen grab - note how the forward and backward trade options are /both/ inactive, so you're forever stuck with a single, locked, trade option. (Slight possible complication in my case; the villager was a cured zombie with whom I may previously have traded, but I presume a 'cured zombie' is a completely new entity in game with no memory of his former life/lives.)
This is still and issue in the latest snapshot: 13w07a.
I've experienced the same issue: 1.4.7
I bred a LOT of villagers in my complex (because you can do that in Minecraft), and one of them which I got was a priest offering 7 Glowstone for 2 Emeralds. First reaction: BUY IT ALL. Which I did. Bought three bunches of them in one interaction, which caused the trade to lock. When I closed the interface and reopened it, the offer was there, crossed out, and no new offers were available, making the villager basically useless. And disappointing, because I want MOAR GLOWSTONE.
Reproducing the issue
Was finally able to figure out a method to reproduce. (At least for single-player mode; multi-player may have other solutions and reasons.)
Trade the last (or only) option fully (as much as the merchant allows) in one trade-session, but immediately after closing trade window, quit the world.
After loading the world again, the last trade option will be locked. As it can not be used, no trade options will ever replenish.
(One) reason
The reason to this is that the merchant's "refresh" of stock/options is delayed a little bit, and the delay is not persisted. If the process gets interrupted, it is totally forgotten, leaving the last trade option in the "traded too much already"-state.
Fixes
The first change adds some robustness (i.e. just in case things go bad, for reason or other, in future), and to fix the already accumulated bugged merchants.
EntityVillager.readEntityFromNBT()
...
if (par1NBTTagCompound.hasKey("Offers")) {
NBTTagCompound var2 = par1NBTTagCompound.getCompoundTag("Offers");
this.buyingList = new MerchantRecipeList(var2);
// FIX: Robustness, and fixes accumulated bugged merchants:
if (((MerchantRecipe) this.buyingList.get(this.buyingList.size() - 1)).isUsedEnough()) {
this.timeUntilReset = 40;
this.needsInitilization = true;
}
}
That is, if it detects a merchant with locked last trade option, it is safe to assume the refresh process had been interrupted, and it starts the process again. Above fix is tested to solve the case on 1.4.7. However, alone it requires a reloading of the world to get the situation fixed, which might not be that trivial (e.g. on multiplayer?). So, better try and fix things so the problem will not happen in the first place.
The second part of fixing the cause: when the problem is caused by a quit made too soon after, the above remedy of loading the world again is actually a perfect match. (If the process was persisted (a fix), it would continue after load; the forget + above fix does in practice the same thing, even though no explicit knowledge about the process is persisted).
However, if anyone can figure out other reasons/mechanisms/methods to get the merchant's last offer locked, feel free to provide reproduction instructions.
Edit: another "robustness" fix might be to add additional check for the last offer, e.g. in the updateAITick(), say, done every couple minutes (+/- random seconds to spread it). That could also be used if that fix in the loading method is deemed a bit too much for a method which is supposed to only load data.
Affects 13w09c. (Tested with the reproduction instructions above, using a default superflat to quickly find a village and using the first victim villager found. Easy.)
After upgrading from 1.3.1 to 1.5, then to 1.5.1, I have this issue as well.
So, confirmed unfixed in 1.5.1
After consulting Reddit, I realized what the problem was:
I shift-click traded with him until he refunded me, then his new offer replaced his original offer. He offered me 5 Glass instead of 4.
But he still remembered that I had bought too much glass last time, so he refuses to feed my addiction further.
So, it looks like there might be a disconnect between replacing an offer with a better one of the same type, and disabling that type of offer.
A replacement offer should do a force enable by default, maybe?
Confirmed unfixed in 13w19a: Logging out immediately after maxing out last trade results in that last trade being forever locked
Confirmed unfixed in 1.6.1. I had a villager who only first spawned in 1.6.1, whose only trade offer was X'd out.
I can also confirm this issue remains unfixed in 1.6.1.
This is a major issue on legit SMP servers where a player may invest a great deal of resources into training a testificate to unlock all trade options, only to have that testificate's trades rendered useless by a server crash or by a forced disconnect that causes the chunk to unload while the villager's last trade option is being traded.
I have personally lost one blacksmith with all diamond armour, tool and weapons trades plus two librarians with paper as their last trade in the last two weeks. On two occasions the server crashed during trading, and on the other occasion a connection dropout ("End of Stream" error) immediately after I closed the trade window was the issue. On all three occasions the last trade option was locked when I logged back into the server, and those testificates are now useless for further trading as their trade options cannot be reset.
To fix the issue, it seems a simple logic check is required on opening of the trade window - if the last trade is locked, then force a refresh of the trade updates.
The fix proposed by me above wouldn't probably work as well in multiplayer (e.g. maybe the other case for Tim), as it is implemented in the loading of a villager. In multiplayer, in a "worst case" scenario, the villager is always near at least one player and never gets the fixing load. Indeed, switching the checking/fixing code to opening the trade window sounds better. (My version was just the obvious place to tinker with it, as it was the quitting (forces unload/load) that I used to cause the bug, and I only play single-player.. or well, currently not playing at all until a bunch of bugs gets fixed.)
There are actually scenarios in which can make the villagers "break":
1.) As described in the description of this bug report the first trade breaks.
This happens as follows:
The villager has a certain trade to start with, for instance a librarian offers 1 emerald for 24 paper.
You trade with the villager and close the trade window. A brief moment after closing the villager will update his trades. As he chooses randomly from all the trades he can possibly offer, it can happen that he chooses the paper trade again, but this time for 27 paper per emerald.
Since the 27 Paper per Emerald trade is rated worse than the 24 paper per emerald he does not replace it and will therefore not replace the trade. Note that this only happens on the first trade slot.
2.) The Villager has multiple trades:
You trade with the villager and use up all the trades in the last slot. Then you close the trade window.
If now for whatever reason the villager becomes unloaded before he is able to update his trade list his last trade will not be refreshed.
"becoming unloaded" includes
-the server or in single player client crashing.
-leaving the dimension unless the villager is in the spawn chunks or there is another nearby player to keep the villager loaded.
-leaving the game which will break the villager even in the spawn chunks in single player and in multiplayer with the exceptions mentioned above.
For every single trade in his list the villager has the two attributes
uses
and
maxUses
While uses < maxUses you are allowed to trade with the villager.
For uses = maxUses he will refuse to do that particular trade with you again.
If now one the above scenarios occurs the trade in the last slot can never be used again which makes this villager useless
which can be very frustrating if you as Tim Wilson described in his post. In order to unlock all the trade on a blacksmith you can estimate around 2.5 stacks of emerald blocks which requires a lot of trading to begin with.
In my opinion the most reliable and easiest fix would be to check
if(uses >= maxUses) on opening the trade window and set maxUses = uses + 1 in that case.
Since my description is the most detailed next to (https://mojang.atlassian.net/browse/MC-749?focusedCommentId=49233&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-49233) which covers the 2nd case described, I would appreciate if the reporter or a moderator could add this information to the report's description in order to clear things up.
Very well summarised Tilman - great post, and I agree that your proposed solution would be the ideal fix for the problem. If implemented, it would not only fix all future villager trades but also 'repair' existing broken villagers in both SP and SMP environments.
@Tilman Perres
Have you checked that the solution you propose does not have any issues with it. Quick "ideas" I got about it was that the player could close and reopen the trade window repeatedly to get multiple "one more" trades with the merchant; and how it would behave with the possibly already started (and delayed) refresh?
The idea to check the trading status (uses >= maxUses) when opening the trade window is, I think, good, but instead of increasing the maxUses, just making sure a refresh is being issued (either one is noted to be already in progress or the process (re)started as usual) sounds like more appropriate action. I'd do such fix myself, but I still have the knot in my brains from the last time on this issue, and I'm again looking at five different classes touched by this case... I'l leave the rest of this issue to Mojang with properly documented code at hand (or someone else with more time and energy).
you are indeed right, but that also could be fixed by triggering the whole trade refresh to the event that a player opens the trade window.
In that case the check for uses >= maxUses would only be necessary for scenario 1 from my previous post.
Actually the first trade gets generated on opening the trade window to a villager for the very first time.
To keep the delay in if desired, there could be timeframe of 40 ticks or so in which the Villager does not interact with the player. This would then have to be implemented in a way that it does not break the villager with that forced idle time due to the same reasons as stated above.
Is this still a concern in the latest Minecraft version 14w03b? If so, please update the affected versions in order to best aid Mojang ensuring bugs are still valid in the latest releases/pre-releases.
Can not confirm the snapshot, but in 1.7.4 it is still there, tested with the reproduction method I described in earlier comment. The voices the villagers make now help in noticing when you have traded too much (or enough, depending on the point of view), especially since the item counts (stacks) etc. seem to still have some inconsistencies during trading.
Note, the "can not confirm the snapshot" meant I didn't test it. unless mod checked a snapshot to be fixed, I'd guess it is still there.
Still in 1.7.5.
Hint to mods: Might want to also undo that "fixed" resolution, not just add new "affects versions" 😛
Why? It affects 1.7.5, still means its fixed in 14w06a. 1.7.5 comes before 14w06a
Except that it is not fixed in 14w08a, so unlikely in that 14w06a, either. They apparently made every villager to start with two trade options, but that just means I needed to consume both options fully before doing the earlier reproduction trick. Now the villager has two blocked options and does not give a new one.
(That is, it is less probably that someone triggers the issue accidentally, but the bug is still there.)
I cannot reproduce it. If I trade with one, it always seems to unlock another one before I can trade the second slot out.
Reproduced again in 14w10c, too. Adjusted instructions from my earlier instructions for the snapshot, taking into account the two options already to begin with:
Reproducing the issue
Works in single-player mode/creative, at least.
Find villager, check what he wants, prepare inventory to have full stack of emeralds (if he wants some) and multiple stacks of whatever else he wants (enough to exhaust his trading limit, typically 6 stacks should do it). Open trade and do not close it until fully done the following and reading the next step instructions:
Trade his first option as long as he allows (shift-clicking helps to speed the process).
Switch to his other option.
Trade the other option as long as he allows (shift-clicking helps to speed the process).
Now, one needs to do the following quickly (preferably complete them all in less than couple seconds, the faster the better). Might be impossible to be quick enough with a beastly computer or a too slow computer (I don't remember the details, but since timing issue is part of the problem...)
Close the trade (esc)
Open menu (esc)
Click save and quit the world.
After loading the world again, both trade options will be locked, and clicking on the arrows to get a new one will not do anything, and as neither trade option can not be used, no new trade options will be generated.
Was able to reproduce following markku's instructions.
While I agree this is definitely an issue, its very hard to unintentionally reproduce.
@Ezekiel I have to strongly disagree here. If the client crashes for any reason, the result of the broken villager is the same. Same goes for servers with the little upside that, if trading with the villager in the spawn chunks, it is safe from breaking unless the server crashes. While outside the spawn chunks it is an option to have a second person stand nearby to keep the villager loaded, this still only applies to multiplayer.
You sure can pay close attention that you keep your villagers loaded until they finished updating their trades IF you are aware of the possible problem described above. However, you can safely assume that more than 95% of all Minecraft players do not know this bug and those, who encounter it, will very likely get frustrated.
@Ezekiel The instructions given by Markku explaining how to reproduce the issue are to allow it to be demonstrated in SSP.
If playing SMP, all that needs to happen for the issue to occur is for the chunk containing the villager to be unloaded before the villager has renewed it's last trade option, i.e. while the player is still trading or immediately after the trade window is closed. This can happen due either a client disconnect from server or due to a server crash/restart, both of which unfortunately occur all too commonly when playing SMP.
Ok, ok apparently your games all crash much more then mine, however I definitely see your point.
I've seen this with villagers who haven't been unloaded. I just don't know how to reproduce the problem without intentionally unloading the chunk (log out, crash, etc.)
Would it be possible to add some logic that detects if the last trade fails to generate new trades?
I can confirm this issue in 1.7.10
What happend to me was that I stood there trading with a villager and then the server crashed. (I dont know why, but I was the only person online and the only thing I was doing was trading to different villagers)
Anyway the server crashed and when we got it up and running again, the villager had a cross over its last trade making it impossible to get it to generate new trades or unlock any trade (including that one).
I have a screenshot of the villager with a cross and the right arrow greyed out, but I think such a screenshot would add very little that I havnt allready said.
Edit:
Note regarding Ezekiels post about this not beeing an issue, since a crash doesnt happen that often: When you trade with villagers that take a LONG time trading back and forth. You have to trade a lot to get a profit out of it. When you do something a lot the chanses of the server or client crashing during a trade gets higher.
I would say that I have spent several days trading, and my most important villager (that I get my emerald income from) just got locked. When reading about this bug now when I encountered it, I am surpriced that I havnt experienced it earlier. Also, this probably wont be the last time I experience it.
This happens all the time on multiplier servers where you have a Trading Village a lot of people use.
We have now lost on the order of a dozen perfect paper traders through this bug.
However as 1.8 changes everything, and resets at a 20% chance on any trade, I doubt this bug will ever be resolved.
The question is, and this is very hard to check, if on the rare occasion you had to trade all the villages trades, before the villager resets, if the disconnect (and chunk unload) happens, could you get a village with all trades permanently locked?
This would be a super rare occurence (more like 2 orders of magnitude rarer than 1.7), but I can see it being possible that this bug can happen in 1.8. However as you can not have perfect villagers, I doubt it is as important. The exception maybe with a librarian with one of the ultra good enchanted book trades (unbreaking III, sharpness V, etc).
This still happens in 1.8.1-pre3.
Easy way to reproduce:
1. Summon a villager
2. Use up all his trades in one go.
3. Unload him within 2 seconds after you closed the GUI. (Easiest is to Save & Quit)
4. Next time you want to trade with him the trades will still be Xed.
The reason for this bug is that the delay till the villager unlocks trades is not written to NBT.
Reopened, thanks.
I wanted to add that it still happens in 15w44a, and that it is actually quite possible to lock it up using normal use as well, not just using the way described by Panda.
A very easy solution would be to refresh all trades of a villager when all trades are locked. Having villagers lock up all their trades does not seem like intended behaviour.
Confirmed that this is still a problem as at 1.8.8 and it doesn't only happen with a single trade. It happens with a villager who has multiple trades. They can all be locked out with no way to reset them, and it has nothing to do with unloading the chunk since it has happened to me when I've deliberately waited for the villager to reset his trades.
Why doesn't the game logic simply check if all available trades are locked out and if so, reset all trades upon the player exiting the trading screen?
Confirmed for 1.8.9 and 15w51b.
Confirmed for 16w02a.
Confirmed for 1.9.1-pre3. Got a villager with 2 trades, both locked.
Confirmed for 1.9.3-pre3.
Confirmed for 1.9.4.
Please link to this comment in the description
The following is based on a decompiled version of Minecraft 1.9 using MCP 9.24 beta.
The reason for this is very likely that the method net.minecraft.entity.passive.EntityVillager.useRecipe(MerchantRecipe)
re-enables the trades with a chance of one fifth or if the current trade was used for the first time. If both is not the case it is possible that all trades get locked.
Might also want to link my earlier description and fix for one part of the problem (https://bugs.mojang.com/browse/MC-749?focusedCommentId=49233&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-49233) and the later
update for how to reproduce the single player case (https://bugs.mojang.com/browse/MC-749?focusedCommentId=146137&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-146137)
Either or both could be obsolete by now (years...), but at least I did not notice anything to that effect among the comments.
(EDIT: note, this issue sounds like it could have multiple different reasons for it, single fix might not handle all the possible scenarios.)
@unknown, saving the timeUntilReset
field as NBT value and removing the needsInitilization
field would probably solve this scenario you described, because needsInitilization
is if I see this correctly only always set in combination with timeUntilReset
. The value -1
could be used as "create no new trades", however the method net.minecraft.entity.passive.EntityVillager.updateAITasks()
would need to count down to -1 then and test if the value is exactly 0. Additionally the field needs to be initialized with -1 as value.
Saving proper state (including that timeUntilReset) would of course be more correct solution, although this is a case where dropping the data can be considered ok, as it is redundant (as proven by my proposed fix). However, as I mentioned, it would not fix those NPC that are already stuck. My more or less 'partial workaround' solution would work as a fix (at least for one reason), and fix the already stuck NPC at next load, and, most importantly, was an easy improvement to show here, and thus easy to import by the devs.
I purposefully did not touch save formats when I made changes to code (in any of the fixes in various issues i've been poking at). Also, Mojang could do totally something else to make it work correctly, I merely try and give something that helps in case the devs are too busy to investigate it thoroughly, or for someone else to use as a starting point.
(By now, things may have changed in a way that makes that old fix/workaround obsolete. And I'm not wasting more of my time on this issue, so I'm not going to check the current status. It has been mojang's turn to do their part for few years now.)
Confirmed for 16w20a.
Confirmed for 16w21a.
Confirmed for 16w21b.
Confirmed for 1.10-pre1.
Confirmed for 1.10-pre2.
Reproduced in 1.10 with Panda's steps from 31/Oct/14 12:24 AM.
Confirmed for 1.10.1.
Confirmed for 1.10.2.
Still a problem for 1.12. A villager who I trade with very frequently is all of a sudden completely trade blocked on all his trade pages. No chunk was unloaded and server did not crash. World restarts do not fix the problem. After weeks of trading, he simply decided to stop refreshing his trades. I normally trade with this villager dozens of times per day to exchange string from a spider farm into emeralds.
Seems like this ought to be an easy fix:
if (allTradesLocked) {
// unlock all trades
}
But when to trigger that? If you want to contribute code, there's a program called "MCP" with which you can look at the Minecraft source code.
Thanks Fabian! Maybe I'll try checking that out. I was thinking that could execute when the trade interface is loaded, performance-willing.
If really going to dive into the code side, then, if not already done so, read through the older comments, there are some existing tries to tackle this (in various ways, at various times). Could give some hints, though some of them could be useless by now (if corresponding functionality has changed)..
Still present in 18w20c
Confirmed for 18w22c
If this is an intended “feature”, make their trade screen say “Retired” or change them into a Nitwit. If this is a bug (which it appears to be), a simple logic check on server start should fix this problem. There’s really no excuse for this. It’s been years.
No, this does not only occur on server start. A proper fix would be to save the unlock time to NBT.
Yep, the "fix" at server start, or when loading the villager (even when the server keeps running otherwise) is merely a workaround and does not help with all possible problem scenarios. But it does also recover in situations where the villager has already become locked earlier (before other fixes prevent the lockup without saved timer from happening).
Saving the timer is better, but also not necessarily a complete fix, e.g. during crash the saving might never complete properly. Again, the loading time check will handle=workaround such rare cases (but only after restart or reloading the villager, so it is not a perfect fix).
The problem with this bug: There are sometimes villagers with all trades locked in adventure maps (as environmental mob or whatever). So all possible fixes will either unlock those, possibly breaking the map, or keep the normal ones in Survival locked. And you can't even make it depend on if cheats are active (which would be pretty weird anyway), because someone might use the server console for all commands when creating the map.
Ah, I haven't played other than survival or creative, so I wouldn't have even thought of such possibilities. I guess the villagers should then have some specific state for the cases where they are locked and should stay so. In perfect case should state data would not be needed, but as proven, things don't run (or stop) perfectly 😛
Such state data would allow also trickier cases like having a map (scenario) where some villagers are set to be locked, while some others are set to work normally, and still handle possible save-time glitches or crashes properly during loading, as needed for each villager.
No, if the NBT tag for the unlock timeout gets saved, everything is fine for the future. You can have both permanently locked villagers with commands and never get them in Survival. Then using the last trade will instantly set the unlock timeout (just like before, except that it's in NBT) and you can restart the server, unload chunks, whatever, he will still unlock once he's loaded again for a few seconds. But if you summon him with all trades already unlocked (or edit the NBT), he doesn't have an unlock timer, so he will never unlock.
"if the NBT tag for the unlock timeout gets saved, everything is fine for the future."
Note that "if" in the begin. It has already been proven that file handling during server shutdown doesn't always work out correctly (for whatever reasons, sometimes the users are to blame), and of course never works correctly during crashes. That is why I considered the additional status; that would not depend on anything else working correctly (except at the time the status is set, but that action can be easily and safely repeated later, if it happens to fail).
And also, if there are locked villagers without timers (that should have trades open but had failed with this issue before) already in the save data, the saving the timer in future lock cases will not do anything for those old bugged ones. It might be impossible to know which way a locked villager without saved timer should now be handled, though, especially considering various player-made hand-tweaked scenarios. Perhaps Mojang should have fixed this when it was still easy, about 5-6 years ago 😛 (Software development 101 -stuff which Mojang has blissfully ignored.)
And here is yet another thing to consider: why is there such a timer at all? At least at the time when I was debugging this (years ago), the timer was so short that it had no practical gameplay effect whatsoever. It would be better to simply reroll the trades immediately, and update the trade UI accordingly, or let the player close and reopen it if feeling lazy. (As I didn't know the background for the timer (and still do not), and touching it would have been much bigger code change, I didn't do anything towards that back then.)
There might also be some other corner cases left to consider, but since I'm not paid for this, I'd leave thinking these details for Mojang devs. (We players have already done like 95% of the work to fix this bug for free, so, I'd let Mojang do the last 5%.)
When your server crashes, that's bad anyway. It's likely that your world gets reset to the state of a few seconds ago. If that happens during the unlock timer, the trade is undone, so there's no problem. And if trade locking and unlock timer are set at the same time, there's no way for a crash to occur between the two. Of course it could still happen in theory, but the chances of that are so tiny that villagers shouldn't be your first worry.
Yes, removing the timer is of course another option. I guess it's there for the same reason as the slow furnace and brewing stand.
"if trade locking and unlock timer are set at the same time, there's no way for a crash to occur between the two."
That is true.
Confirmed for 1.13-pre1
Why did you remove the links? A text like "this comment" without a link is pretty useless.
Also, if something doesn't depend on the environment, the "Environment" field is not needed.
I didn't remove the links (if they were somehow removed it was unintentional). I modified the 'how to reproduce' section to also include the original case (and much more likely occurrence) of this bug since it had been significantly modified without mentioning the original bug I reported which as it stands now is nearly distinct from the bug report.
There are two separate issues here but they have all been grouped into one bug ticket here. One is that trades can become disabled after quickly logging out upon trading with a villager (or some variant of that) and the other is that trades can become disabled through normal trading if you are very unlucky.
Still in 1.13-pre2
And in 1.13-pre3
In regards to
@unknown added a comment - 02/Jun/18 9:26 AM
Couldn't the auto-refresh routine just use either the Profession or Career data to determine whether auto-refreshing should be initiated? If it isn't on the list of default, vanilla minecraft Professions or Careers, ignore any such routine.
Confirmed for 1.13.1.
Here's my suggestion.
If the player is in survival gamemode (to prevent interference when setting custom villagers and in adventure maps), and all trades are locked when a villager is opened, and it has no trades that are not normally part of it's career (not a custom villager), then unlock all trades.
This shouldn't interfere with anything
Adventure map villagers could have a flag that specifically marks them as custom villagers. Then it shouldn't be necessary to resort to unreliable ways of checking such as checking their career, or their trades. Just check the flag.
Can confirm in 18w47b
This still persists in 18w50a across villagers of all types. You can still get unlucky and get a low trade # villager like leatherworker or fletcher through normal trading with all trades locked out. It's also possible with other villagers but very unlikely.
Confirmed for 19w03a
Confirmed for 19w03b
Confirmed for 19w03c
This happened to me without quitting the world. I simply traded 4 x 34 paper with a cartographer and the trade became disabled. He will not trade anymore with me. I didn't do anything hostile to them, but a zombie had snuck out of a cave and infested one of the other villagers – maybe this upset them?
Confirmed for 19w05a
I just bought a book shelf from a librarian that got bugged earlier (paper trade won't refresh), and didn't get any XP. The book shelf option became unavailable, so I closed the GUI. No particle effects. I opened the trading GUI again and saw that now both the paper trade and the book shelf trade (both in different tiers) are disabled, and also I still have the 12 emeralds that I had before I bought the book shelf. I quit and reloaded the world to make sure, and yep – I still have all my emeralds and the bookshelf, so I did get a bookshelf for free somehow. This villager is totally broken. The other villagers are okay but that's probably because I've been carefully doing only one trade at a time since I left my last comment.
New village, trading as usual and over time, all villagers were similarly affected with no negative actions toward them. Out of curiosity, went looking in villages.dat and it looks like the player settings (reputation, UUID) have disappeared. Occasionally, it will reappear but I've not been able to figure out when this happens and when it doesn't.
That seems like a different bug. Are you playing in 1.9? Then it could be this bug: MC-101325
This one is quite simple: Unload the villager while it's trying to regenerate trades. Nothing with reputation or "not wanting to trade", it just happens to have all trades locked simultaneously.
Nope. According to MC wiki reputation is forgotten when villages are unloaded. Based on an edit on 9th May 2014, it sounds like there was intention to fix this in 1.8 but apparently it was not accomplished.
Bugs should usually not be in the wiki. Does this cause all trades to be locked? Reputation 0 shouldn't fo that.
I'm on 19w05a.
To clarify what I said above regarding player data inside villages.dat:
I was inside an active village when I checked villages.dat, in a world that I'd created in 19w05a about two hours before, and I'd not left the village since entering it. I'd not visited any other villages and have not traveled outside of this village's chunks. I'd successfully traded with villagers as normal (without trades being immediately disabled at first attempt) earlier in the day, and had not done anything to negatively affect reputation. I'd had this issue in my main world (combined with a different bug; entities become invisible), so when I noticed it happening again in this new world I checked villages.dat. There was no player data (i.e., 0 players, so no UUID, no S). I have a habit of rebuilding villages so I spawn new villagers periodically to test trades; when I noticed trades functioning normally again I checked villages.dat again and it had an entry for Players (1 entry, UUID = mine, S = a positive #). All villagers that I trade with when this happens are affected, and those affected villagers remain locked. I don't know if this is a separate bug or not, but it does seem to have some relation at least in my experience.
I couldn't begin to come up with a use case though because it doesn't seem like I'm doing anything reproducible when this happens.
Confirmed for 19w06a
Confirmed for 19w07a
confirmed for 19w08b
Confirmed for 19w09a
This bug can't be properly tested in 19w11a, because trade unlocking has changed. I don't know how exactly, but I've just traded 4 times normally and it locked everything in the villager UI. This report should probably be marked as blocked by some other report, but I don't know which one.
Quite possibly this issue has now become completely invalidated with the new trading system. No locked trades are unlocked anymore with normal trading (I believe), they get unlocked when the villager restocks by doing their jobs, so the problem is more obvious (however, intended according to the blog).
I will close this as fixed since with the new trading system the villagers will restock.
This is a GUI issue.