CoreNetwork / TradeCraft

Custom villager tiers, trades and rebalancing. [No longer updated]
MIT License
2 stars 6 forks source link

All villagers are now named NO_CAREER and want to trade bedrock #48

Closed someone-somenet-org closed 9 years ago

someone-somenet-org commented 9 years ago

I am too migrating from RebalanceVillagers - like in #47

My problem is that after copying the jar into plugins and restarting the server all villager become NO_CAREER and refuse to trade anything but bedrock. Restarting without the plugin fixes this again.

If I change the default configuration to the config that is attached in #47 that villagers become "normal", but all others stay NO_CAREER.

This is what gets written to Server-log when i want to trade with any villager: [13:30:37] [Server thread/INFO]: [TradeCraft] No professions found for villager number 2. [13:30:37] [Server thread/INFO]: [TradeCraft] No trades found for career NO_CAREER! [13:30:39] [Server thread/INFO]: [TradeCraft] Villager e9cc6dce-4116-4c63-9bc6-992c727a123d has no trades!

Your Readme states "All villagers defined in config will have their trades replaced by config values." This reads to me like "Stuff you define overrides vanilla stuff, but stuff you dont define wont be changed"

Do I need to define all trades and professions in the config just to override/change a single profession? In that case: is there a full "vanilla-like" example config somewhere for me to copy?

Server: spigot 1.8.3 tradecraft 1.1.3

riddle commented 9 years ago

If this is the case, it’s unwanted behavior and I’ll talk with the team to have it fixed. Sorry for the issue, please give us some time to investigate it and/or resolve it to the best of our abilities.

someone-somenet-org commented 9 years ago

so... any news? :P

eliadil commented 9 years ago

As it is right now - all undefined professions will behave as you described (bedrock trade + NO_CAREER). So the workaround for now is to just define all trades to resemble vanilla. The readme might indeed be a bit misleading, probably due to us not being perfect in english + miscommunications~

I'll take a look at the code tonight and try to change it leaves a villager vanilla (for the most part), if no trades defined in config.

eliadil commented 9 years ago

I've added this behaviour - the new build will generate new setting node KeepVanillaTradesIfNoneSpecified: false

If set to true, it will keep vanilla trades on undefined villagers.

someone-somenet-org commented 9 years ago

Seems to work - you are great!

someone-somenet-org commented 9 years ago

Ok, maybe it does not work fully as expected.. If I define one profession, i need to create all Subprofessions...

Professions: '3':

Will get me only Armorers, if I do not define all other Subprofessions of 3. But its still awesome that I do not define ALL other Professions+Subprofessions.

riddle commented 9 years ago

I’ll let @eliadil answer fully but from what I understand, villager professions are not linked to the villager entity at all so there is no way to determine who is what.

In fact, if you have a client-side mod like Zyin’s you can see that villagers have labels which don’t fit the profession and trades displayed in the GUI window. This is because TradeCraft completely overwrites trades on first right click on a villager and will not use native NBT values of the living entity.

So keeping this in mind, there is no clear way to separate black apron villagers into 3 groups and only overwrite 1 of them. Why? Because even if we use 33% chance for that 1 defined TradeCraft profession (Armorer), the rest 67% could have native Armorer in NBT with vanilla trades, not the ones you defined.

So it’s my understanding the state of your issue now is Won’t Fix, sorry. I’ll keep it open for the developer to comment, I might have missed something.

eliadil commented 9 years ago

Yup, it is pretty much as riddle described it.

One more reasoning I would add to that - defining only one career (eg. armorer) for blacksmith profession (id 3) and deducing other careers from vanilla code & leaving vanilla trades would be dumb for one major reason - in current state you can define totally custom careers, you don't have to use vanilla names for them.

So if someone was to add a custom collection of careers, adding one deduced from vanilla would totally ruin the whole plan.