ArmorPlus

ArmorPlus

9M Downloads

Enchants above default max level throw errors or crash

Xetaxheb opened this issue ยท 10 comments

commented

Among other possible ways to do this, one is obtaining them off a miniboss from a mod that gives some enemies enchanted armor that can drop; the error in the gist happened when a skeleton wearing lifesteal 4 armor shot me and it crashed, also throws errors if I wear it myself.

Affected Versions (Do not use "latest")::

  • ArmorPlus: armorplus-1.12.2-11.21.0.57.jar
  • TheDragonLib: thedragonlib-1.12.2-5.3.0.jar
  • Minecraft: 1.12.2
  • Forge: 2838

Affected Side (Client or Server):

  • Client
  • Server

What happens:
Enchants throw errors

What you expected to happen::
Enchants not to throw errors

If its a crash, post your crash report (via. pastebin/gist/etc):
https://gist.github.com/Xetaxheb/d9a3dc478a9f7eebf8b6cf1dc88932b7
etc

Your configuration files from config/armorplus/ (registry.cfg mainly):
Nothing interesting here

commented

Enchants above default max level aren't allowed.
There is a max and min level for enchantments set for a reason.
Also, chestplates should NOT be able to get the Life Steal enchant. Its ONLY available for swords and tools

commented

Invalid. Any item can have any enchantment ONLY by using custom commands.
Except plenty of mods circumvent this in various ways. My example was one that adds randomly enchanted armor to miniboss mobs, but there are plenty of others. Let alone that it shouldn't be crashing anyway just because someone commanded it in ??

I know where this can be fixed. but what is the point ? QoL sure, can be dealt with.
The point is it causes a crash... I don't really care if it won't function either more powerfully or even at all above your desired cap, but it can't be causing crashes.

Because it really just "happens" to fit the values, so IF I decide to expand the level cap later on I can easily do it. Its called future-proofing.

You can make equations or if statements to fit pretty much any power curve you want while also providing support to situations where the enchant is higher than your expected cap. It's not hard, you're just being obstinate :^)

I don't really mind making a reflection mod that disables your mods enchants outright if it comes to that. I can't have it crashing just because you didn't code it right.

commented

I'm stubborn, mainly because this issue is caused, by 3rd party mod.

Except plenty of mods circumvent this in various ways.

Yeah well, there is always a blacklist to add enchantments to. IF an enchantment is not supposed to be applied to an item, it should be considered before adding them. Why wouldn't other mods check if it can be applied ;).

The point is it causes a crash... I don't really care if it won't function either more powerfully or even at all above your desired cap, but it can't be causing crashes.

Once again, will be fixed, forgot to mention this code is particularly old, so my old stupid self hasn't future-proofed this.

You can make equations or if statements to fit pretty much any power curve you want while also providing support to situations where the enchant is higher than your expected cap.
It's not hard, you're just being obstinate :^)

Yeah, if statements:

  • Too many is bad usage.
  • Used to use a lot of if statements, because of the complexity of the Enchantment.
  • Its not hard, its stupid.

I don't really mind making a reflection mod that disables your mods enchants outright if it comes to that. I can't have it crashing just because you didn't code it right.

Go for it :). Code it right.

commented

Java is a compiled language, extra if statements are virtually no overhead in this situation.

And I just found one that lets me do it already, saves me 10 minutes of work
https://minecraft.curseforge.com/projects/enchantments-control
No more lifesteal or ferocity.

commented

Java is a compiled language, extra if statements are virtually no overhead in this situation.

You don't get it don't you ? Its not overheat that is the problem here, its bad code decisions that will be extra if statements when there is always a better way.

commented

image
;)

commented

Oh? What is that reason? And any item in the game can get any enchant, you should put in a check for it to not function at all if it's on an invalid item if you really feel that way.

It looks like poor coding to me, why not just make

public enum Levels { ZERO(0.0f), ONE(0.5f), TWO(1.5f), THREE(2.5f), ;

into
healingfactor = enchantmentLevel - 0.5f;

BTW:
Levels lvl = Levels.values()[level];
is the reason the error is thrown, you're not checking for "if (level > 3) {level = 3};" pretty easy fix if you really insist on an arbitrary limit. (which neuters the enchant when players are in a setup balanced around greater than 20 hearts)

commented

Ditto with this

public enum Levels { ZERO(), ONE(23, 0, false, 0, 0), TWO(23, 0, true, 23, 0), THREE(23, 1, true, 46, 0);

23, floor(enchantmentLevel / 3), enchantmentLevel > 1, (enchantmentLevel - 1) * 23, 0)

commented

Oh? What is that reason? And any item in the game can get any enchant, you should put in a check for it to not function at all if it's on an invalid item if you really feel that way.

Invalid. Any item can have any enchantment ONLY by using custom commands.

  • An invalid item CANNOT use its abilities.
  • An invalid item may obtain the enchantment ONLY up to the MAXIMUM value.
  • Definition of invalid for Life Steal: [Anything that doesn't extend ItemSword & ItemTool]

I know where this can be fixed. but what is the point ? QoL sure, can be dealt with.

I've intentionally made it limited up to lvl 3, because of how overpowered it can be.

Poor coding

Sure, I mean I like getting criticized but mainly if it is constructive.

It looks like poor coding to me, why not just make

public enum Levels { ZERO(0.0f), ONE(0.5f), TWO(1.5f), THREE(2.5f), ;

into
healingfactor = enchantmentLevel - 0.5f;

Because it really just "happens" to fit the values, so IF I decide to expand the level cap later on I can easily do it. Its called future-proofing.

commented