Discussion in 'CommandHelper' started by switorik, Jun 29, 2018.
Setting the lore of an item per kill should be fine in terms of speed, however in MC whenever you modify a held item it resets the attack animation. This might look odd in practice and be undesirable. (It's possible to workaround that issue by buffering kills, but it'd add some complexity and make it a harder to avoid bugs.)
If you want to improve operation time for this script, there's a few things you could do, but nothing significant or really needed. But if you're still interested, I do have some tips. Typically you want to exit early before you do any work. So defining the allowed items after you check if there's a damager would mean lower operation time for deaths from fall damage or fire. When you're checking if the damager is in the list of all_players(), you don't have to use array_contains_ic() since case will be accurate from the event, so you can use array_contains(). get_itemmeta(@p, null) gives you the meta for the held item, so it's one less function you have to use. But if you instead use pinv(@p, null) to get the entire item array, you can then use it in your next check for allowed items, which means two pinfo() calls are not needed. However, you'll then need to check if either the item is null or the item meta is null. But if there's meta, 'lore' always exists, though the value might be null. You also don't need to add that last die() at the end of your script, since it'll end on its own. All of this adds up, but it's not really noticeable. Most of the time you don't have to worry about individual functions unless you're using get_value() and store_value() too much when using a slower persistence network database type.
I did notice a couple bugs in there, though, like if an entity was killed by a non-player shooter. It'd throw an exception that @p is not defined instead of die()ing. Also, the way you're setting lore with set_itemmeta() will overwrite all other existing meta. It does not add meta, it sets it. So you'd want to modify the item array and then use that array when setting the item meta back to the item. Lastly, you use a capitalized "Kills:" once, which would mess up the the "kills:" check.
I'm going to work on a couple tips you provided. I want it to be as efficient as possible because I know we have players that make huge mob farms. If its going to run 50+ times instantly, I don't want it to lag the server.
Good catch on a couple of the bugs, that saved a bit of headache.
As always, I appreciate your advice!
Separate names with a comma.