TinyModularThings / IC2Classic

IC2Classic Bugtracker
151 stars 39 forks source link

IC2C is incompatible with vanilla systems #895

Closed ieris19 closed 1 year ago

ieris19 commented 1 year ago

So, I am really baffled at this because I'm just unsure how this is even happening. Basically, Almost Unified cannot alter IC2C recipes and LootJS cannot target IC2C ore's loot tables.

To give a bit more context, I'm trying to develop a modpack where I want to unify all ores, Almost Unified scans items for tags and alters recipes so the inputs are always forge tags and the output's always the preferred mod's implementation, which can be changed. However, while AU successfully merges IC2C resources in every recipe, none of the machines are altered (this tells me recipes are not implemented through JSON, which sucks, because it's the most compatible with everything else. If I'm wrong, then maybe worth looking into it).

Meanwhile, I wanted to leave in each mods' ore generation, because I don't wanna mess with worldgen and I want it to be an easy pack. LootJS can modify block drop loot tables to make it so any ore will drop the prefered's mod item (so, for example, currently, silver ore from Thermal or Occultism will drop Thermal's Raw Silver). However, not only is LootJS unable to do this with IC2C ores (and only IC2C, every other mod is working as intended), but LootJS seems to be completely unable to modify IC2C loot tables at all. According to LootJS's support, the mod is likely not using the Vanilla Loot System.

This is a big bummer, and I'm not really sure why this mod specifically seems to refuse to interop with the features Mojang has provided in recent years to make modding easier.

Speiger commented 1 year ago

o/ You might be misunderstanding how things work.

Why do we do the things we do

Overall we saved a years worth of work at the VERY LEAST.

BUT, we do fully support Json Recipes and Loot tables.

How to make loot tables work:

Simply IC2Classic checks: If a loot table is present? Good apply the loot table, if not run our loot drop code. Or in TL:DR: You simply write loot tables for our blocks and it will work. (And if not make a bug report and give us a test case) Why should we write hundreds of useless files if we don't need them, you can do it if you want to >:)

How to add/change recipes

https://github.com/TinyModularThings/IC2Classic/wiki/1.19.x-ResourcePacks

Have fun :) Luckily this actually has a autogenerate command that generates all the "Existing recipes" as 99% finished Datapack (only the MC Meta File is missing)

Documentation should explain everything.

Simply said: We still support Mojangs stuff, but it is used as an override so we save years of lifetime, since less then 0.1% of the users actually use it, and then they only change like 5-10 files anyways. No harm done >:)

Anyways i hope that helps.

Edit: You are the first person who asks for loot table stuff :)

Speiger commented 1 year ago

@ieris19 ping :)

ieris19 commented 1 year ago

Hey, While the answer solves my problem, I can help but ask, as a dev myself, I'm curious why making recipes is not easier through the vanilla/more compatible way?

Json can easily be automated programatically, and any feature not supported in JSON recipes/loot tables should probably be the exception, not the norm.

Anyway, I guess my problem is solved, but there MUST be another reason since "not typing long JSONs" is so easily circumvented.

Just wondering to be honest, its more curiosity than anything else at this point.

Only think I'd point out is that if the wiki section includes datapacks on top of resourcepacks, perhaps a better title could be found? I didn't find it poking through the wiki because I didn't think it'd have any relevant info for me

Speiger commented 1 year ago

Ok lets start with the easy one.

Only think I'd point out is that if the wiki section includes datapacks on top of resourcepacks, perhaps a better title could be found? I didn't find it poking through the wiki because I didn't think it'd have any relevant info for me

Yeah true. That could be renamed at some point.

While the answer solves my problem, I can help but ask, as a dev myself, I'm curious why making recipes is not easier through the vanilla/more compatible way?

Well outside of crafting recipes we actually do not use minecrafts crafting system. Why? It is very laggy to query and extremely limited.

I could go through a tirade, but let me put it this way:

public void addRecipes(IMachineRecipeList list) {
    list.addIC2ChanceRecipe("flint_to_gunpowder", new ItemStack(Items.GUNPOWDER), 0F, 0.25F, Items.FLINT);
    list.addIC2SimpleRecipe("sliced_potatoes", new ItemStack(IC2Items.POTATOE_SLICES, 8), Items.POTATO);
    list.addIC2XPRecipe("charcoal_block_to_dust", new ItemStack(IC2Items.DUST_CHARCOAL, 9), 0.1F, IC2Blocks.CHARCOAL_BLOCK);
    list.addIC2Recipe("scrapbox_unpacking", new ScrapBoxOutput(), IC2Items.SCRAPBOX);
    list.addIC2RangeRecipe("recycling_chestplate_"+type[i], results[i].copy(), 0, 8, new DamagedInput(chestplate[i]).setUndamaged());
}

This is how we do recipes at the moment. If you haven't realized these recipes are from 5-6 different machine types, but they are technically all valid in any machine. (Hence the folder only matters for the basic machines)

Writing that manually in json is insane, if you lookup the file names in the datapack i assure you you will see how big these files are. Now the question is: "But since you have this auto exporter already why not use that instead?" I have 2 reasons:

So instead of having to write a Exporter/Importer we simply had to write a few hundred lines and any recipe we wanted to hook into just worked fine. And it was expandable at any point.

Now why did we do it this way? Fairly simple. The small team we had needed to have simple structures to work with. a simple: addRecipe function they could add at any point without having to know magic variables ("type: macerator", or "type:extractor") was simply more viable. On top of that the IDE support was in general better.

Combine that with only 1 person of the team who realistically could develop extensive features and or a framework the others to work in, it was simply a simple solution to not waste time and get everyone more efficiently working.

For single person people or a experienced team Datapacks are totally fine and should be used. The moment you put a mixed experienced set of people together with a 3 year project, you have to start taking measures to ensure it doesn't explode.

A recipe change/addition shouldn't take more then 30-50 seconds and should be live testable by simply calling the /reload button. Running a separate program to get things working isn't feasible when you have to do hundreds of microscopic changes.

Overall from what i learned.

Speiger commented 1 year ago

Closing this since it's completed, but i will continue answering questions. I just want to close this since its no longer an "issue" but a conversation.

ieris19 commented 1 year ago

That is actually enlightening, makes a lot of sense.

As for the wiki, I am not sure what your policy for contributions is, or even how GitHub wikis work, but I'd be happy to help if you need a hand, I quite enjoy writing docs.

At this point, this is just to feed my dev curiosity. In the past, when I had to do some project that needed custom serialization logic, I found myself writing reflective code to serialize JSON. I would still use things like .doThis() or .addThat() but when "saved", a custom JSON would be output.

Wouldn't a "RecipeHelper" class of sorts that takes a few methods and represents them in memory, but still implements the recipe through outputting JSON recipes be easier? In fairness, I don't know if implementing recipes through custom logic would be hard, I assume IC2 had that nailed from the times before datapacks, but just a thought

LLytho commented 1 year ago

Hey, LootJS and AU dev here. And I wanna give some thoughts on this because I'm not a fan when mods hardcore stuff. Especially because there are no reasons anymore in modern minecraft. We all create mods for the same game and the goal should be to make it as easy as possible for everyone.

I mean at least you auto check if a resource exist and you take the resource instead. But with this you already wrote serialisation, so why not already us this for data generation at all?

Writing that manually in json is insane, if you lookup the file names in the datapack i assure you you will see how big these files are.

Okay but why you even manually create the jsons? Thats why datagen exist and minecraft provides easy to use systems to generate. I would probably call u insane if you really would create all the jsons manually lol.

On top of that the IDE support was in general better.

You have this with datagen too.

Fairly simple. The small team we had needed to have simple structures to work with.

In your example you show abstracted code, so it does not matter what the code would look behind it. It could just generate all your data.

when you have to do hundreds of microscopic changes.

tbf this sound more like that the main design does not work as expected if it's really not feasible to debug recipes or reworking them. But I don't wanna make assumptions here, because I did not look into your src.

Minecraft Data/Resourcepacks are fine for small/medium sized projects but should be a afterthought for big/giant sized projects.

Dont see the point here. Code & Data generation are two huge benefits especially for bigger projects.

Intellij sucks at hot compiling by default and you really have to go out of your way to make it work.

Not really. Yes the old DCEVM does not get updated anymore but we now have Jetbrains Runtime we can use to provide full hot reloading. No matter if static variables, removing classes or adding classes. If you are interested in this, you may wanna add these vm args to get all features.

The simplistic approach is usually the best approach. The easier it is to use for team members the simpler your life will be! Because you know your code, others don't, and if they don't its a nightmare for everyone, even you.

You can't just say it like this. The simplistic approach is also using the already existing systems because most of the time this is what devs already use.

Speiger commented 1 year ago

Hey, LootJS and AU dev here. And I wanna give some thoughts on this because I'm not a fan when mods hardcore stuff. Especially because there are no reasons anymore in modern minecraft. We all create mods for the same game and the goal should be to make it as easy as possible for everyone.

Ahh a fellow dev. Hope you are doing well @LLytho.

I mean at least you auto check if a resource exist and you take the resource instead. But with this you already wrote serialisation, so why not already us this for data generation at all?

Because i didn't want to write a parser in the first place. Your whole argument falls apart because it ignores that a JsonParser has to be included. Which it wasn't And you also lovely ignore that i said: "The exporter only existed after the mod was basically done. Actually not sure if the exporter existed in the first official release even."

When we started, and the whole 2.5 years of development, we only really had a internal system for setting/changing recipes. There was no parsing support for it at all. All that was done after or shortly before the end of the second beta in 1.19.2 (the mod started when 1.14.4 was the first version).

Okay but why you even manually create the jsons? Thats why datagen exist and minecraft provides easy to use systems to generate. I would probably call u insane if you really would create all the jsons manually lol.

Again, we would have to either write a parser/exporter system to write these, and then we would have to run this on any small change we did. Minecrafts data generator was useless to us because we didn't even use minecrafts system because it didn't provide what we wanted.

In your example you show abstracted code, so it does not matter what the code would look behind it. It could just generate all your data.

https://github.com/TinyModularThings/IC2Classic/blob/51be41106579fb34d0883f74533ac6d1c74a62bb/src/main/java/ic2/api/recipes/registries/IMachineRecipeList.java#L21

Oh yeah it is abstract code but it all results in this: Pseudo code

public class MachineRecipeRegistry implements IMachineRecipeRegistry {
    Map<ResourceLocation, RecipeEntry>  recipeMap = new HashMap<>();
    Map<ItemStack, List<RecipeEntry>> itemToRecipe = new CustomHashMap<>(ItemStackComparator.INSTANCE); //Love fastutils CustomHashMaps <3

    public void addRecipe(RecipeEntry entry) {
       if(recipeMap.putIfAbsent(entry.getId(), entry) != null) {
          //throw error
       }
       for(ItemStack stack : ItemStackComparator.deduplicate(entry.getAllInputs())) {
           itemToRecipe.computeIfAbsent(stack, T -> new ObjectArrayList<>()).add(recipe);
       }
    }

Which is much simpler then writing a parser. Note that not everything you see in the end result was there in the end, and some iterations were done, but overall to get things going this is much simpler then writing a parser/json writer to then finalize the format of the recipes. This gave us much needed volatility.

tbf this sound more like that the main design does not work as expected if it's really not feasible to debug recipes or reworking them. But I don't wanna make assumptions here, because I did not look into your src.

You assume when we started writing recipes that we had all planned out and were just implementing the final result. At a smaller scale that may be working but at our scale, we didn't know that yet. We were actually planning only a rough outlook and the exact details were done as we were trying. That means we didn't know what we needed and needed something we could iterate over.

And minecrafts Datapack system doesn't support iteration very well. Yes you can change things, but any change internally/parser wise can not be done easily. It takes A LOT of time to make it work. That's also a reason we went that way.

Dont see the point here. Code & Data generation are two huge benefits especially for bigger projects.

If you have a giant project that you want to finish with its release, and don't have concrete plans you want a system that doesn't constantly bites you in the ass, especially if you include minecraft version changes.

In other words we were prepping for a system we could use in ANY MC version.

Not really. Yes the old DCEVM does not get updated anymore but we now...

Nowdays doesn't matter, it was back then, and even if unlike eclipse you have to "know" these things. There is a knowledge requirement.

You can't just say it like this. The simplistic approach is also using the already existing systems because most of the time this is what devs already use.

Ofc i can say it like this. Even if i build a parser i have to write it in my own way, the same logic applies. I know my parsing system because i wrote it. For every single machine/crafting recipe. It doesn't follow mojangs ways. Yes it uses datapacks but parsing != datapack.

And that is a thing you lovely ignore :)

Also, just to remind you. This is just a question about: "Why we did it that way". It is not a system that is going to change, it is just a explanation for our choices back then. Would i make different choices if i would restart from scratch again? Yeah totally. I see a lot of things i would love to see different but that is for the next attempt.

Do you have to agree with my choices? Hell no.

Can you force me to change it? Hell no too.

I hope you don't feel attacked by it.

Small edit because someone reminded me of this: And there wasn't even time for adding it during the betas too. I was patching daily reported bugs, like 20-30 per day, taking my entire time for 2 months for each beta. I barely had enough time to play myself, during the second beta. (first beta i had maybe 20 hours of total playtime in 2 months)

Speiger commented 1 year ago

@ieris19

As for the wiki, I am not sure what your policy for contributions is, or even how GitHub wikis work, but I'd be happy to help if you need a hand, I quite enjoy writing docs.

I have no problem with suggestions. What i pick out of that is still a different story. I hope that is understandable :)

At this point, this is just to feed my dev curiosity. In the past, when I had to do some project that needed custom serialization logic, I found myself writing reflective code to serialize JSON. I would still use things like .doThis() or .addThat() but when "saved", a custom JSON would be output.

I mean I like to use json for a few things too, but these are usually a bit more final or only expanding on existing functionality. For example if a recipe should support XP, or be random in range, but not a rewrite from scratch for the system itself.

Also yeah, that's why i am willing to explain because of the interest of what led to choices.

Wouldn't a "RecipeHelper" class of sorts that takes a few methods and represents them in memory, but still implements the recipe through outputting JSON recipes be easier? In fairness, I don't know if implementing recipes through custom logic would be hard, I assume IC2 had that nailed from the times before datapacks, but just a thought

Under different circumstances, with WAY less pressure, a builtin exporter would have been a different solution. Where inDev we would just use our internal system, but in production json files would be auto exported into jar. (upon building the jar) But back then times were a lot different. Life was different for me. And honestly it was a learning process I am happy that about that it happend.

Those weren't my best years in life, but they lead into my best years.

Ofc nowday i "could" (but don't) say i could have done things different, but that is a fine line. And could have gone way worse, then it did.

Enough personal stuff. Glad that we at least could help you.

rlnt commented 1 year ago

Damn, what a mess. I'm not sure if this is pure arrogance or incompetence.

All of your arguments just fall apart when you look at GregTech. Your mod is a lightweight mod compared to GT and guess what, they managed to make it fully compatible without reinventing the wheel. If you don't want to use data-gen because you end up with thousands of JSONs, that's fine. If you want to avoid it because you have to restart the generation process every time you make a small change, that's fine too.

But for all of that, there's a simple solution that you could have easily implemented without much effort if you had just invested a little time in researching it instead of continuing the snowflake tradition that IndustrialCraft keeps leading. It's called dynamic datapacking. Easy to generate at runtime without JSONs, it's just code. Many mods with a lot of content use this. Yet you guys chose not to, preferring to make an incompatible mess.

Very ironic considering your history. Especially when it comes to mods like AE2 or authors like Technician that you got upset about because they didn't properly invalidate their caps. They didn't follow the conventions, you got upset about it, and now you're doing the exact same thing.

Your very first message proves that you don't have much idea about what you are talking about. Years of lifetime? For such a simple system? Are you good?

I had already drafted a message where I would completely blow every sentence of yours out of proportion with a simple counter-message. But the past has shown that one does not need to discuss with you anyway. I'll save myself the time and just declare IndustrialCraft incompatible with all projects in the future and give as a hint to uninstall the mod for anyone running into compatibility issues because the author is not able to use the systems that already exist to make the end-user experience halfway bearable.

LLytho commented 1 year ago

Your whole argument falls apart because it ignores that a JsonParser has to be included. Which it wasn't And you also lovely ignore that i said: "The exporter only existed after the mod was basically done. Actually not sure if the exporter existed in the first official release even."

I did not ignore that lol. From what I wrote it does not matter if the importer and exporter existed from the beginning. They seem to exist now (for idk how long). My argument does not fall apart, because from my understanding your importer and exporter are json parsers.

Again, we would have to either write a parser/exporter system to write these, and then we would have to run this on any small change we did.

How often do you change recipes that this is something you have to think of?

You assume when we started writing recipes that we had all planned out and were just implementing the final result. At a smaller scale that may be working but at our scale, we didn't know that yet.

No I don't assume that. I'm just wondering. You never know how stuff will change but with time and the code changes, refactorings happen.

Yes you can change things, but any change internally/parser wise can not be done easily. It takes A LOT of time to make it work. That's also a reason we went that way.

Not sure if I understand this correctly. If you have a recipe type and extend this type with new fields for new features. Yes it takes time, ofc it does. But how often does this really happen?

Ofc i can say it like this. Even if i build a parser i have to write it in my own way, the same logic applies. I know my parsing system because i wrote it. For every single machine/crafting recipe. It doesn't follow mojangs ways. Yes it uses datapacks but parsing != datapack.

It doesn't follow mojangs ways. You know what the fun part here is? For datapacks there is no "real" mojang way here. You just have to give the generator jsons. How? Mojang does not care . But mojang has stuff, if you want to use it.

Also, just to remind you. This is just a question about: "Why we did it that way". It is not a system that is going to change, it is just a explanation for our choices back then. Would i make different choices if i would restart from scratch again? Yeah totally. I see a lot of things i would love to see different but that is for the next attempt.

Of course it takes time, code evolves over time. I never said anything different. I just have the standpoint of that we all should work within minecraft or loaders system. Because this helps everyone at the end imo. So I just wanted to know why you went this way and especially why you want to keep it. Thats just something I don't quite understand.

I hope you don't feel attacked by it.

nah all good

Speiger commented 1 year ago

@LLytho

So I just wanted to know why you went this way and especially why you want to keep it. That's just something I don't quite understand

Why i went that way. Circumstances that i am not willing to disclose, personal for that matter, lead to it.

As for why i am not changing it. The version is done, with it's gems and faults. I am not doing major rewrites anymore. I may be doing fixes to it, but that's about it.

Speiger commented 1 year ago

@rlnt that's your choice if you want to argue. I only told what was the point of the time, also the difference between AE and IC2C is. AE is primarily made for the users. IC2C is made for my friends and me. So the fact that i released and give it any support after the beta, including older versions too, is me being nice. If you don't like it, leave.