CraftTweaker

CraftTweaker

151M Downloads

Entity drops with a min of zero cause crash

The-Arcanian opened this issue · 9 comments

commented

Issue Description:

If you add a drop with the min set to zero the game will crash if that zero is rolled.
java.lang.IllegalArgumentException: stack cannot be null

What happens:

The game crashes.

What you expected to happen:

The game to not crash. Neither the documentation or the mods syntax check say that this would be a problem.

Script used:

https://gist.github.com/The-Arcanian/01b09f936399bae93bf1d448b2dd8d68

crafttweaker.log file:

https://gist.github.com/The-Arcanian/028545e312da037d40e62aa5368b449c


Affected Versions (Do not use "latest"):

  • Minecraft: 1.12.2
  • Forge: 14.23.4.2705
  • Crafttweaker: 4.1.9
  • Using a server: No
  • If yes, does the client have the exact same scripts?

Your most recent log file where the issue was present:

crash report: https://gist.github.com/The-Arcanian/6c805acc1ae3217802b44127388bccb5

commented

Seems pretty self explanatory, a stack with a size of 0 is empty... Which should never be a thing. Use weighted itemstack to do random drops.

commented
  1. That goes against the Minecraft president (https://minecraft.gamepedia.com/Drops).
  2. If you still don't want it done then the documentation should say not to do it and the syntax check should catch it and not load the file.

It is not user error when the user has no way of knowing that the most available examples do not reflect how the product actually works.

commented
  1. This is a crafttweaker specific thing because we still treat empty itemstacks as null, so in our wrapper, if you try and make a wrapper for an empty stack, it errors out.

  2. The SYNTAX checker checks the SYNTAX, it doesn't run the code, and while the syntax is technically correct, the usage isn't, and that can only be verified when compiling and running the code.

commented

If stack sizes of 0 are not supported, then it needs to be documented and a useful error produced. Not a game crash.

MCEntityDefinition.java:45 has:

public void addDrop(IItemStack stack, int min, int max, float chance) {
        if (min < 0 || max < 0 || chance < 0 || chance > 1) {
            CraftTweakerAPI.logError(String.format("Invalid value provided: <entity:%s>.addDrop(%s, %d, %d, %s).", entityName, stack, min, max, Float.toString(chance)));
            return;
        }
        drops.add(new EntityDrop(stack, min, max, chance));
}

Its has catches, it needs more.

Have an else if for min < 1 && max != 0 that gives an error and ether: Does not load the script. Or brings the min up to 1.

commented

Dude. I have a real life, calm down.

Once again, as far as I'm concerned this isn't an issue, it may not be documented but that is another thing, we have a repo for the wiki, so the Ossie should go there.

commented

I am calm, I am sorry if I came across as angry, I am not. I waited a full day before asking that it be reopened so as to try and avoid coming across as angry. Reopening the issue is all that I was asking for, I know fixing it has to wait for your availability.

But as I said before, (documentation aside) A useful error should be produced, not a game crash. Documentation is so that users can provide the right information to the program, not so that users can avoid catastrophic failure. If user input can cause catastrophic failure then the program needs to be fixed (not saying immediately). Hence this issue.

commented
commented

Please reopen this issue, it is not solved. A crash is not something you respond to with "don't do that", it is something you slate to fix. Even if not immediately.

commented

You seem to have missed the shift in the conversation. You said that stack sizes of zero cannot be supported, that is fine. After you said that, I never once tried to insist that the mod support stack sizes of zero. All I said is that there should be a catch if a user tries to give an input that can cause a game crash. There are already checks against the most erroneous inputs, but the catches are insufficient if there is still a user input that can cause a crash.