-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Recipe/Condition Factories cannot be registered from code #4274
Comments
Absolutely agreed. I see no benefit from registering these through JSON in the first place. What you lose is static verifiability and the ability to construct these objects however you want. What Forge gained is complexity. You could say it is equivalent to registering blocks and items through JSON. You could question why the built-in factories are not added via JSON. |
It's a code style choice, and a functional choice, which was NOT an afterthought. It's also designed to force modders to think about their recipes and clearly define how they are structured and hopefully make them generic and reusable. As well as to remove any conditional definitions of factories because of what you feel like that day. As having a stable target on the pack dev side is important. On top of that i'm honestly thinking of maybe expanding the factories to be javascript instead of just json. So that the actual functionality is controlled in the file and would potentially be more self-documenting as people could inspect what's actually going on. But that moves things to an entirely different realm which I don't want to do at this point. Again, at the end of the day this comes down to code style. Yes, having to put full class names into a json file is a bit annoying. But it's a fairly standard thing in plugin systems to define your entry points somewhere outside of code. I have also toyed with the idea of simply specifying a single class, Or forcing you to have a single class named something specific, that would allow for multiple factories to be defined in one .java file. But I haven't sat down and worked through those ideas yet. Part of the point is that I have to think of more than just people writing mods when it comes to this. I have to think of pack makers, end users, and 3rd party tools. Because yes those types of people are a big part of the recipe community/audience. You guys should care about them. Having a central place with this information that doesn't need to be parsed out/decompiled from binary classes is important. Toss in a few comments {Which reminds me, I think I turned comments on if not I need to fix that} in those json files to document how to use the factories and you have a nice resource that non-programmers can reference! "You could say it is equivalent to registering blocks and items through JSON." Which, if you haven't been paying attention to the last 2 years of development, is exactly where Minecraft is trying to go. Deal with it. "You could question why the built-in factories are not added via JSON." This one is honestly because at the time I was developing the entire loading system I had to re-write that stuff like 5 times and I wanted to move on to other things. One of them was the concept of registering this stuff in code. Which is why the register functions still exist. But I abandoned that in favor of giving more power to the pack makers. I should go back and clean that up into a _factories.json but that's later. TLDR: Yes I know specifying full class names is a little dirty/annoying to people. I have good reasons for going this route. And I am open to ways to make it cleaner without sacrificing the ability for non-programmers to know/handle/use this data. But for now this is what we got. |
Even though I don't think the outcome is satisfactory, it's nice to get some arguments for why the system is the way it is. Better than "Use the JSONs." Keep it up 👍 |
The only way to register recipe and condition factories is currently to insert a FQCN into a random json file which seems put in as an afterthought. Code-related identifiers have no place in data files, especially not fully-qualified classnames. Not only does it mean that plugins that statically analyse code for unused parts have a harder time and need to be needlessly complex to even work, but there's also no point to having a data file tell the engine which factory to load. Factories need not be loaded based on user-specified conditions, and they need not be loaded more than once.
Hence Recipe and Condition factory registration should be moved into an event on startup or similar, to cleanly seperate code from data.
The text was updated successfully, but these errors were encountered: