Add item drop config settings #5
Conversation
Pyrofab
left a comment
Pyrofab
left a comment
There was a problem hiding this comment.
Sorry for being late, I have a lot to do and I actually forgot about this Pull Request :/
There are a few things that need to be changed, but nothing major.
src/main/java/ladysnake/spawnercontrol/SpawnerEventHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/ladysnake/spawnercontrol/SpawnerEventHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/ladysnake/spawnercontrol/SpawnerEventHandler.java
Outdated
Show resolved
Hide resolved
| //Try/catch handles this if null | ||
| item = Item.getItemFromBlock(ForgeRegistries.BLOCKS.getValue(itemRL)); | ||
| } | ||
| drops.add(new EntityItem(world, x, y, z, new ItemStack(item, split.length < 3 ? 1 : Integer.parseInt(split[2]), split.length < 4 ? 0 : Integer.parseInt(split[3])))); |
There was a problem hiding this comment.
The values computed from split should each be stored in a variable with an explicit name. This line is too long, and the parameters to new ItemStack() are not immediately obvious.
There was a problem hiding this comment.
I opted not to put the "chance" (index 4) part of the split strings into an explicit variable, because it would either prevent a possible short-circuit optimization (which is not a big deal in this case, but still bugs me), or add another level of bracing (and an additional boolean)
If you want the chance in an explicit var anyway, just poke me one more time and I'll do it lol
Leaving just this one "unresolved" in case you still want changes to it
src/main/java/ladysnake/spawnercontrol/SpawnerEventHandler.java
Outdated
Show resolved
Hide resolved
|
Need to leave for work; will fix the last one when I get back (or maybe during lunch) |
… in when making the Exception catch specific)
|
Hey sorry, I ended up working through lunch and then heading to my cousin's wedding...which was late, long, and an hour away :P Anyway, I think it's ready for another check. Hopefully I didn't do anything dumb since I'm half asleep. I'm off to bed |
2 participants
Working...for the most part.
Only thing that doesn't work right is the mob-specific stuff...by which I mean ALL the mob-specific stuff, including the previously existing xp settings. I ran another test of it using the released version on curseforge (1.6.2), and it doesn't work on the released version either, so that's not from me.
I have all the same style code logic in there, though, so if whatever existing loading issue is fixed, my own new mob-specific settings should start working right along with the mob-specific xp settings.
"Default settings" for vanilla spawners and custom spawners seem to be working flawlessly, for all settings, so here's the PR :)