SpongePowered / SpongeForge

A Forge mod that implements SpongeAPI
http://www.spongepowered.org/
MIT License
1.14k stars 306 forks source link

SpongeForge is over-writing the custom villager trades from the Villager Trade Tables mod. #2067

Closed CrypticDivide closed 6 years ago

CrypticDivide commented 6 years ago

Versions/OS Information

Mods

SevTech: Ages ─ Version 3.0.1 Minecraft, Minecraft Coder Pack, Forge Mod Loader, Minecraft Forge, SpongeAPI, SpongeForge, A Block of Charcoal, AbyssalCraft, AbyssalCraft Integration, Actually Additions, Actually Baubles, Advanced Generators, Advanced Mortars, Antique Atlas, AppleCore, AppleSkin, Applied Energistics 2, ArmoreableMobs, Astikoor, Astral Sorcery, AutoRegLib, B.A.S.E, BC Builders, BC Factory, BC Robotics, BC Transport, BD Lib, BNBGamingLib, Baubles, Better Advancements, Better Builder's Wands, Better With Mods, Better with Addons, BiblioCraft, Blood Magic: Alchemical Wizardry, BnBGamingCore, Bonsai Trees, Bookshelf, BuildCraft, BuildCraft Lib, CD4017BE_lib, Caliper, Car Mod, Carry On, Ceramics, Chameleon, Chargers, Chisel, Chisels & Bits, Clumps, CodeChicken Lib, CommonCapabilities, Compact Machines 3, ContentTweaker, Cooking for Blockheads, CraftStudio API, CraftTweaker JEI Support, CraftTweaker2, Cucumber, Cyclic, Cyclops Core, Dark Utilities, Despawning Spawners, Dimension Stages, Dimensional Control, Dung Pipe, Elevator Mod, EmberRoot Zoo, Ender Utilities, EnderStorage, Environmental Tech, Example Mod, Extended Crafting, Extra Planets, FTB Utilities, FTBLib, Farming for Blockheads, Farseek, FastWorkbench, Ferdinand's Flowers, FoamFix, FoamFixCore, Forge Microblocks, Forge Multipart CBE, Galacticraft Core, Galacticraft Planets, Game Stages, Geolosys, Gobble Core, Guide-API, Harvest, Horse Power, Hunting Dimension, Immersive Craft, Immersive Engineering, Immersive Petroleum, Immersive Tech, Improved Backpacks, InControl, Inductive Logistics, Industrial Foregoing, Info Accessories, Integrated Dynamics, IntegratedDynamics-Compat, Inventory Tweaks, Iron Backpacks, Iron Chest, Iron Jetpacks, Item Stages, IvToolkit, JourneyMap, Just A Raft Mod, Just Enough Items, Just a Few Fish, JustTheTips, KleeSlabs, Loot Table Tweaker, MJRLegendsLib, MTLib, Mantle, McJtyLib, Mekanism, Mercurius, Micdoodle8 Core, Minecraft Multipart Plugin, Mo' Bends, Mo' Villages, Mob Stages, Mod Tweaker, ModPack Basic Tools, ModPack Utilities, Modular Machinery, Modular Routers, Morpheus, Multiblockstages, Mundane Redstone, Mystical Agradditions, Mystical Agriculture, Natura, NetherEx, No Recipe Book, No World Gen 5 You, Not Enough Items, Nutrition, Ocean Floor Classic, Ore Stages, OreExcavation, OreExcavation Integration, Overloaded, Pickle Tweaks, Placebo, Player Skin Mobs, PneumaticCraft: Repressurized, Power Adapters, Prestige, PrimalChests, PrimalCore, Progression Tweaks, Prospectors, QuantumStorage, Quark, Quick Leaf Decay, RFTools, RFTools Control, Ranged Pumps, Realistic Item Drops, Reborn Core, RebornStorage, RecipeStages, Recurrent Complex, Redstone Flux, Refined Storage, Refined Storage Addons, Road Runner, Rustic, Scannable, Shadowfacts' Forgelin, Simple Generators, Simple Storage Network, SimpleAutoRun, Simply Arrows, Smooth Font, Smooth Font Core, Spartan Shields, Spatial Server Mod, Steve's Carts 2, Storage Drawers, Streams, Stuff A Sock In It, Super Sound Muffler, SwingThroughGrass, TallGates, Tesla Core Lib, Tesla Core Lib Registries, The Beneath, The Betweenlands, The Twilight Forest, The Weirding Gadget, Thirsty Bottles, Tinker Stages, Tinkers Tool Leveling, Tinkers' Complement, Tinkers' Construct, To the Bat Poles!, TogetherForever, Tomb Many Graves, Totemic, Translocators, Traverse, Triumph, Trumpet Skeleton, Tumbleweed, Valkyrie Lib, ViesCraft, Villager Trade Tables, Waddles, Waila, Waila Stages, WanionLib, Water Control Extreme, Water Strainer, What Are We Looking At, Wild Crops, Wither Skeleton Tweaks, Wopper, XNet, YNot, Yoyos, animalium, death_compass, fat_cat, galacticrafttweaker, magma_monsters, mob_grinding_utils, primal_tech, sev_tweaks_npc, uppers, FTBUtils, FTBLib

Issue Description

When generating a new world in the SevTech: Ages version 3.0.1, using the quark_realistic world generation type, the villagers do not have the same trades as defined in the configuration files of the Villager Trade Tables mod, which can be found in the "/config/vtt" folder of the pack.

We already know that the issue is not within the pack because when running a server with a clean install of the pack, without using SpongeForge, this issue does not occur. That said, when we add SpongeForge, to another clean install of the pack, this issue occurs.

Please note, that all of the testing to confirm this issue was done using just SpongeForge─no additional plugins were installed.

gabizou commented 6 years ago

I can understand how this is happening. Truth be told, since vanilla MC does not handle any customization of villagers short of a multidimensional array to determine the trade offers made for villagers, sponge had to essentially overwrite in the entirety how villager trades are handled and generated. On top of that, sponge has to then handle the Forge added systems to integrate with mod added villager professions and careers and trades. The problem here is that the overwrites sponge makes relies on the sponge provided constructs to be handled by common natively, and it doesn’t rely on the Forge provided methods or fields otherwise.

I’ve been meaning to rewrite the villager code for a while (on top of many other systems in sponge) to have better compatibility, but in short, the wrench thrown here isn’t that the villagers won’t work, but that using reflection and expecting it to work all the time is a bad bad bad bad idea. I’ll get to rewriting the system if someone doesn’t get to it before me, but investigating what villager trade mod does is just straight up reflection to fields that sponge doesn’t use, causing customized trades to be thrown out the window. I could say it’s almost better for someone to write a plugin to modify the trades with sponge api, but I feel that would be in poor taste.

Yorkforce commented 5 years ago

This still seems to be an issue, all the villagers I have visited so far have been vanilla trades.

Using SF3718

Noticed this is startup log https://pastebin.com/pK4Ns9Dm

phit commented 5 years ago

make a new issue please, that seems to be specific to the "villagertrades" mod