Armory Expansion

Armory Expansion

10M Downloads

Error/Bug: Configs not loading properly. After first generation

SirAron111 opened this issue · 41 comments

commented

I found out trough recent testing and player reports, a certain bug. Certain materials lose traits and material parts vanish completely when loading the game for the second time (after configs have generated).

So I have been using for the test all the mods I have in my pack in a separately made instance.
I used Tinkers construct (of course), conarm, armory expansion, ice and fire, botania, draconic evolution and plustTIC, yoyos. And also tested it with just the tinkers mods and yours not using any other mods that are not tinkers related.

All materials from botania and all materials from plustic lose the material traits. And for some parts the integrated parts even vanish. Like the chaotic laser medium or other wyvern/awakened/chaotic materials same for botania parts. The Armor Parts are still there (not having any traits but all common tool parts or integration of tool parts are gone)

So what I think is happening is there is an error in the generated configs. And how they load in and overwrite the existing materials added by any other tinkers mod and the traits keeping only the newly added armor parts (who then also mostly miss traits).

Here are some pictures how the traits normaly are.
2019-07-10_13 50 07
2019-07-10_13 50 08
2019-07-10_13 50 10
2019-07-10_13 50 12

And here are some pictures showing that after materials lose parts (easy to see in jei) standart manyulin still has all parts to show the difference
2019-07-10_14 17 52
2019-07-10_14 17 13
2019-07-10_14 17 25
2019-07-10_14 17 34
2019-07-10_14 17 42

and lastly some pictures of the missing traits for some materials (there are more tought didnt make pictures for all) and for some reason it registers materials from mods that arent even isntalled like thaumium or psigem or twilight forest or others. (in the test instance whit only the tinkers related mods)
2019-07-10_14 18 20
2019-07-10_14 18 28
2019-07-10_14 18 51
2019-07-10_14 18 54
2019-07-10_14 19 00
2019-07-10_14 19 02

commented

still broken for me, all the mods 3 remix 1.5.1

commented

Does that modpack have version 1.4.0 of armory expansion? If not then its still broken, you would have to manually update the mod if its not the correct version.
From the looks of things, that pack has version 1.3.4b, which is broken.

commented

hmm you're right, sorry about that - there's no update button next to conarm in the twitch launcher so I assumed it had the latest version, my mistake

commented

I guess I didn't say what I meant by values being bad, but they're so obviously wrong that I didn't think you'd need more than that. Because I love what this mod adds I'll do what you ask...

Installed your modpack, loaded it up, and checked the values. All the values look sensible and right in the armor addendum. Not really more to say.

On the other hand, Installed a fresh atm3r pack, updated armoryexpansion to latest, deleted armoryexpansion.cfg and config/armoryexpansion/ folder, and started her up. Loaded into game, and the values in the armor addendum are obviously wrong:
https://imgur.com/a/DMuDEMY

I mean, compare them with your modpack if you like but some of those values are insane and obviously wrong.

commented

looking at the Superium from now and from a month or two ago, the values aren't the same, so looks like garbage values are being used at some stage.

commented

Keep in mind that those stats are generated from the tool stats and the formulas need proper balancing. Unfortunately I've yet to find something to satisfy everyone and my time is much better spent fixing more serious issues.

commented

Hmm, I think there is some miscommunication here. When I say garbage, I mean garbage in the sense of uninitialised memory, not suggesting your formulas aren't good or whatever.

Did you look at the pictures I attached? Many of the values don't pass basic sanity check. Multiplier zero for modifier? So the whole armor piece is gonna have its durability multiplied by zero? And those that have durability modifier 40+ when the typical value is in the range 0.5 to 3. And not on outlandish materials like Superium, but on basic gems like Topaz and Amethyst?

Toughness -0 sometimes. Not zero, minus zero. Strange stuff is going on, I don't mean numbers I'm dissatisfied with - I mean the numbers look actually wrong. And by what formula could Superium ever end up with zero toughness? Looks like your formulas are being applied to the wrong values or those values are being put in the wrong place.

tbh looks like uninitialised memory is being used and/or you've got an off-by-one error or something in writing the values to the config files. And when you do make the armor pieces, the effects of the keywords often don't trigger. It looks like a serious issue to me but hey that's really up to you, I love your mod but this makes it unusable in atm3r, and this is one of the most popular modpacks of the day.

I have no vested interest here, not trying to hound you into fixing it for me. I've gone to a lot of effort to verify the issue and demonstrate how to replicate it. What you do with that is up to you, but this isn't bad formulas, this is a bug.

commented

To that I can say one thing. First of all you need to adjust the values in the configs yourself as the configurability is what make s the mod do what it’s supposed to do. (If it’s in a modpack it’s the responsibility of the modpack author to balance things according to his pack.)…..of course it should be a thing that default values get added that are reasonable but since this mod grabs its materials from the materials that get added by other mods to the tinkers one its natural that it cant foresee what materials get added and its really hard to make it happen that literally any unknown new mod adding anything to thinkers gets preset values.

What I did to make it work as a modpack author is simply have the mod add the materials and the traits which this mod does perfectly. As for the stats I use the modtweaker .zs script function you can check these out in the scripts folder of my pack for reference. They are added by the base mod of constructs armory and really easy to use. And are the intended feature to change the stats of the armor parts in conarm.

It’s a wonder to me that this mod which grabs a material and adds it with stats and traits based on a function is even able to get some parts reasonable right since deviating armor values from durability etc can only end in high deviations in numbers as durability values are all over the place in thinkers construct.

In my opinion the mod works fine if the person who wants to use it actually modifies it. It’s a mod made to be modified after all not simply out of the box use. And this modification is up to a good modpack author. If the guys from atm3 just see oh a mod updated and hammer in the update not checking if the update changed anything important or broke something already existing its mostly the fault of that Modpack author if he pushes these bugs onto his players and shows how little they care about their player base and only update to get people to redownload the pack to get more downloads.

The only option I see to fix the default values of materials would be to give all common materials a proper preset value that gets put in until someone changes the values in the configs themselfes instead of using a function. And only have random/unknown materials like that chocolate stuff set to standard iron armor values by default to be changed via configs by the one who wishes to balance the mod into his personal pack or into a public modpack.

But getting proper values via a function that is balanced around tool materials is nearly impossible. Reason one is toughness doesn’t exist in these so it will always be 0 unless deviated from another value like durability, mining speed or damage etc but these values are so different and preset for the tool materials that making a one formula fit all things won’t be possible, if the materials added are unknown to begin with.

So my Conclusion: if you don’t like reading my long text:

Yaiba: get away from the formula to add the values you can’t make a one formula fits all magic formula that’s just not how math works when the base numbers of the materials added to thinkers are not made for it. Give preset values to the most common ones like botania mats and draconic stuff etc so that when the mod sees a these materials get added they get these values. And if an unknown material gets added simply default it to iron armor values and state in your mods description that proper balancing can only be done by actually tinkering with the generated configs changing the values to what the user sees fit and or maybe have them use the craft tweaker/modtweaker to change values like I do since with this you can also change the default materials values like ardite,cobalt,iron and Manyullyn etc.....or default all values to 1 and make it so people have to adjust the configs to values they see fit to begin with. Your mod simply isn’t made for easy out of the box use it’s a configurability monster and I love it for exactly that, the ability for pack makers to balance stuff themselves.
Edit:Another thing I see possible is to grab the values for the new materials from the base materials. Meaning that if a material gets added that has Damage higher or equal to iron gets added it will have the iron armor stat values. Until the next tier higher, so if a material gets added that has the Damage of Manyullyn or higher it gets the stats of Manyullyn as this is the highest default armor value. This way you can guarantee that no material will get values that are too high or too low. As for materials that have lowers stats than wood they will still default to wood! So at least until someone edits the config they will get at least somewhat decent values.

Porcupine: if you notice something is wrong in a modpack make sure to confront the pack maker as well so he can hotfix his modpack and fixing these values to a reasonable thing is really not hard to do even I managed it with ease and I bet the modpack author simply forgot to recharge the values to decent ones after updating. Second make your own report because this one was made from my end out about the armor parts and traits that went missing (not the values) which is the reason you have me butting in on your problem don’t take it a bad way just trying to help as well XD.

commented

In addition to SirAron's good suggestions, a lot of these new materials have armor made out of them as part of the mod they come from, Superium / Supremium for example. A Tinker's Armory piece made entirely out of that single metal should have the same armor/toughness as the default armor piece, imo (maybe slightly better to account for the metal casting, who knows). Maybe you already use those values in your formulas?

I didn't realise configuring this mod was part of the mod-makers job, I believed that sensible values had been put in by hand, forgive me. Also sorry if I got a bit salty, I do my best to keep level head.

commented

@porkupine well I may have been a bit salty in my tone as well seeing as it was my own fault for not simply exiting the closed bug report like I normally do. If you want decent values for all armors etc you can try my pack (playing it) it’s a bit hardcore scaling and so but for experienced players it should provide a better challenge than ATM. And seeing as you had a keen enough eyes to notice the error on missing parts or bad stat values I am certain you will enjoy a challenge like this, and I would love to see someone who isn’t scared to criticize on my packs discord as well. #totallynotselfadvertising.

commented

@porkupine nah, the one you need to update isn't conarm, just armory-expansion :)

commented

I know I've descredited myself but I'm pretty sure this still isn't working in atm3r. I've updated the mod to 1.4.0 (mod file armoryexpansion-1.4.0.jar) and deleted the associated config files (both the armoryexpansion.cfg and the folder named "armoryexpansion" in the config folder) and tried loading again, the values are still bad. Then I gutted the config folder completely and restarted and the values are still bad. After this, I again tried deleting just the armoryexpansion config files and restarted and the values are still bad.

Maybe there's something I'm doing wrong but I'm not sure what else to try.

commented

I should be clear, to test whether it worked I load the game completely and open the Armory Addendum book and look at the values there, I don't know if the config files end up with the correct values or not.

more info for reproducing:
atm3r 1.5.1 (latest version)
and a couple of things that won't matter: plus torchmaster mod, and draconic evolution hardmode set to false
everything else as per installing from the twitch launcher

commented

For me in my modpack „Dimension Zero“everything regarding the armory expansion mod is fixed. Feel free to download my pack to see if maybe something is wrong on your end by comparing the files. ….the only thing I had to do was set all the materials from the ice and fire module to craftable=true these where set to default false for some reason...which I send a pm to yaiba on discord at some point.......Also I f you are using the mod in a modpack (like ATM3) you need to also reset the config files as the will change and old ones will not be supported. And if you just go replacing the mod itself but not fix the config as well you will have a problem.

commented

per my previous comment, i not only deleted the config files for armory expansion, i deleted all config files and tried again, no dice.

commented

Also what do you mean with the values are still bad…. The mod doesn’t change any values on its own until you configure the config. It just adds more materials to constructs armory that’s all it does on default. So if you want to change the actual stats like armor, toughness or durability etc you would need to configure the generated files or simply use the mod/craft tweaked mod to make a simply .zs file..These mods should be in atm3 already anyway.... like i said take a look at my pack it has all the values and all the stats working exactly how i configured them with the mod should make for a decent refference.

commented

In the current version, the only values that have been curated are for the Ice and Fire materials, as those were done by hand. As for the traits not always working, TiC defaults to the Tool traits if the Armor parts have none. I do intend to make it so that a trait is automagically converted into an armor variant when I find the time.

commented

After further testing i came to the point of noticing that it wasn’t natura causing the problems (I just noticed it first on this mod when I was changing things around)
This issue actually happens on everything.

What actually happens is that when freshly installing armory expansion and no configs are generated yet it will have when immediately starting to play still all materials (loading of materials before armory expansion generates configs or at least parts of the configs get ignored on first load as they generate before the mod checks for the changes in the configs)

But then after exiting the world and closing the client and then restarting it (configs have already been generated at this point so the mod now checks for the changes in these propbably) suddenly a lot of materials are just gone and a lot of traits are lost (issue as explained in above pictures)
So I think what happens is that whenever there are already configs generated it overwrites all standard traits and materials parts from mods that aren’t the main mods (TiC and Conarm) so basically all mods affected I could list are these :

All botania materials / all natura materials / all PlusTiC parts made from PlusTiC added materials like the Draconic Materials (chaotic awakened wyvern etc). Extra util materials natura materials and basically anything in the conarmconfigs that generate in the armory expansion folder.

Here are some pictures showing that the affected materials also don’t show the normal ingots and items they are made from but only the nuggets/shards instead (from the standart TiC book, the Conarm book and the yoyos book (yoyosmod adds tinkers yoyos this bug also happens witouth it but its also affected from it so i listed it)) Only ice and fire integration works mostly fine with or without modified config for them Probably because they are only added by your mod anyway so they aren’t influenced anyway. It also shows the materials for these parts as the items and not chunks/nuggets.
2019-07-10_15 54 01
2019-07-10_15 54 09
2019-07-10_15 54 24
2019-07-10_15 54 25

commented

Mods list for testing
the non tinkers mods like RF flux or baubles are autodownloaded by other mods ice and fire isnt in there but it also happens with that mod had it moved out for testing once when i made the picture

commented

have some logs if they help from the second time loading up the game (after having the configs generated closing the game and loadign it up with already generated configs (themoment the materials parts and traits start to go missing))
Dont know if this is going to be of any help but if you need mroe info just let me know https://pastebin.com/DupGRyfa
Second load up part 2.txt
Second load up part 1.txt

commented

Just wanted to say we have noted this error in the All the Mods 3 Remix pack. I'm going to go reference this issue over there now.

commented

Has anyone found a workaround so far?

commented

Only workaround so far is to delete the Armory expansion configs every time they are created (every time you load the game). This prevents you from loading any custom configurations but if you just want it to work with its base settings it will this way.

commented

Till I find a proper fix, all I can do is push a toggle for the config generation, disabled by default. While it won't fix custom configs, it'll keep the config folder clean and solve the issue for those using the default values.

commented

If there are any logs or tests we can provide you with to help track this down let us know.

commented

It's mostly a debugging problem, I'll have to analyze the loading process step by step to see where I'm overriding what I shouldn't.

commented

ye until the fix is ready just do what i did disable the conarm integration in the .cfg and simply only use the dragon parts and make custom ones via json for whatever you need. The dragon parts work great thought.

commented

Just a little note from debugging. Last version in which the items in question worked consistently was 1.2.5

commented

Here is what I found out. If you look in the log, you can see that the materials have been registered but that "stats for" is missing. This are exactly the disappearing materials.
Full log: https://pastebin.com/R7eAnFQW

[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading config from local JSON file
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading config from source
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading all materials from local JSON files
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading all materials from source
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading all ore dictionary entries from local JSON files
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading all ore dictionary entries from source
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading all alloys from local JSON files
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done loading all alloys from source
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done saving config to local JSON file
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done saving all materials to local JSON files
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done saving all ore dictionary entries to local JSON files
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Done saving all alloys to local JSON files
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {awakened_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {bloodwood_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {darkwood_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {ancient_metal};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {chaotic_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {fusewood_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {tar_slime};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered tinker's material {ghostwood_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered stats for tinker's material {blackquartz_plustic};
[14:27:19] [Client thread/INFO] [armoryexpansion-conarm]: Registered stats for tinker's material {awakened_plustic};

commented

https://github.com/YaibaToKen/Armory-Expansion/blob/6869f1713d1211a632f3ced9e8bfcebef1b5f594/src/main/java/org/softc/armoryexpansion/common/integration/aelib/plugins/general/material/BasicMaterial.java#L80

I'm fairly certain the problem is here. If you change the method to immediately return, the bug never occurs. (and of course material configs aren't loaded)

commented

Fixed it, I'll make a pull request. Although, I had to drop ice and fire to 1.7 in order for gradle to find it.
Without a pull request, the fix is simply commenting out three lines as such in this file. This still allows the conarm-materials.json values to be tweaked and used.
https://github.com/YaibaToKen/Armory-Expansion/blob/master/src/main/java/org/softc/armoryexpansion/common/integration/modsupport/ConArmIntegration.java

 @Mod.EventHandler
    public void preInit(FMLPreInitializationEvent event) {
        this.modid = ConstructsArmory.MODID;
        this.logger = event.getModLog();
        this.configDir = event.getModConfigurationDirectory().getPath();
        if (ArmoryExpansion.isIntegrationEnabled(modid)){
            this.loadMaterialsFromOtherIntegrations(event);
            this.setIntegrationData(this.configDir);
            this.integrationConfigHelper.syncConfig(this.materials);
            this.saveIntegrationData(this.configDir);
           // this.registerMaterials();
           // this.registerAlloys();
           // this.registerMaterialStats();
        }
        ArmoryExpansion.config.save();
    }
commented

I have a question to the config .json. I noticed that they are saved every time to game has been started.
Why is that? They just need to be read only.

commented

Can confirm from testing between version 1.2.5 and any version 1.3.0 that if conarm integration is off, tool materials are unaffected. If conarm integration is on, tool materials become missing. However, if you change the "material" flag to false for any armor inside of armoryexpansion/conarm-config.json, then that armor will no longer be registered and the tool material will still exist.

commented

@grundyboy34 If I get you right, you mean if I turn off Natura stuff etc., it will appear again in Materials and You?

commented

Yes, from what I tested that's the case. Currently the options are to either get it working as a tool material or an armor material. Anything inside of conarm-config.json with the material flag set to true seems to get rid of the tool material, leaving only the armor material as long as conarm integration is on.

commented

By all appearances, you were correct. All my six materials reappeared.

commented

In the spirit of solving this, here's another noticed interaction.
Wiping conarm-materials.json to just contain "[ ]" causes everything to load as it should, except that conarm-materials.json is autofilled again causing the next load to experience the bug.
Doing this with conarm-config.json just results in the bug still being there, so I'm fairly certain this is happening somewhere that conarm-materials.json is loaded.

commented

Because if the file is missing info or has invalid data, it has to be rewritten.
I will however, look into a way to validate the file and copy it's contents to a #.json.err file before rewriting it.

commented

Ahh I didn't test that change with no configs to see if it generates them, which I should do.
As far as I'm aware as to why it works to load the configs though is that the same code in preInit is also right below it in the registerItems event. From what I could tell with testing, it seems the checks to see if a material is in the tinker registry before registering the material fails to detect some materials in the preInit stage, which the only reason I can think would be happening is because there isn't a guarantee that your mod runs its preInit code after all other mods have ran theirs.

commented

which the only reason I can think would be happening is because there isn't a guarantee that your mod runs its preInit code after all other mods have ran theirs.
PlusTiC actually runs some integration code in RegistryEvent.Register<Item> due to the late registration of some mods' items.

commented

Fixed it, I'll make a pull request. Although, I had to drop ice and fire to 1.7 in order for gradle to find it.
Without a pull request, the fix is simply commenting out three lines as such in this file. This still allows the conarm-materials.json values to be tweaked and used.
https://github.com/YaibaToKen/Armory-Expansion/blob/master/src/main/java/org/softc/armoryexpansion/common/integration/modsupport/ConArmIntegration.java

 @Mod.EventHandler
    public void preInit(FMLPreInitializationEvent event) {
        this.modid = ConstructsArmory.MODID;
        this.logger = event.getModLog();
        this.configDir = event.getModConfigurationDirectory().getPath();
        if (ArmoryExpansion.isIntegrationEnabled(modid)){
            this.loadMaterialsFromOtherIntegrations(event);
            this.setIntegrationData(this.configDir);
            this.integrationConfigHelper.syncConfig(this.materials);
            this.saveIntegrationData(this.configDir);
           // this.registerMaterials();
           // this.registerAlloys();
           // this.registerMaterialStats();
        }
        ArmoryExpansion.config.save();
    }

Sorry, only saw this just now.
Commenting those lines also means that the armor parts are never generated. Though, this.registerMaterials() and this.registerAlloys shouldn't be there iirc... I'll tinker with that and let you know if it fixes everything.
Meanwhile, thanks to all of you for the help fixing this.