Bukkit/CraftBukkit — pull request #1351 by WolfieMario (2014-03-05 07:36:25+00:00)
Title: [C] Increment loop index whether or not command succeeded. Fixes BUKKIT-5455

**The Issue:**

When VanillaCommandWrapper dispatches a command containing a PlayerSelector with the "c" argument greater than -1 (which is true by default for `@a`), it loops over the selected players and executes the command with each player. However, the loop index is only incremented if the command fails. As a result, a successful command is repeated on the same player indefinitely (unless it does not always succeed), locking up the server.

**Justification for this PR:**

Due to the above issue, the "command-block-overrides" feature will not interact properly with selectors such as `@a` or `@r[c=2]`. Many commands, including `/give` and `/playsound`, will cause the server to be trapped in an infinite loop if they contain such selectors. Custom maps often use commands which trigger this issue. [The documentation for command-block-overrides suggests using it when running custom maps](http://wiki.bukkit.org/Commands.yml#command-block-overrides), which would be ironically poor advice if this issue is left unfixed.

**PR Breakdown:**

The fix is very simple, thankfully. The index used to iterate over the list of players is only incremented when the command fails. The fix moves the incrementation out of the catch block, and back up to the for statement, ensuring that the index is incremented once per iteration. This ensures that the command is only called once per player targeted by the PlayerSelector, regardless of whether it succeeds or fails. Note that j, the Success Count, is still only incremented upon success.

**Testing Results and Materials:**

I added the following to commands.yml in order to activate the command-block-overrides feature:
````
command-block-overrides:
- "*"
````
Testing was done in-game via various commands, including the following: `playsound tile.piston.in @p`, `playsound tile.piston.in @a`, `clear @p`, `clear @a`, and `give @a bone`. These commands were run within command blocks, powered by a button. I also tested more complicated mechanisms (part of an incomplete adventure map), and can upload schematics of them if requested; I only omit them now because this PR really feels like overkill for such a simple fix. Do let me know if I'm wrong.

**Relevant PR(s):**

None.

**JIRA Ticket:**

BUKKIT-5455 - https://bukkit.atlassian.net/browse/BUKKIT-5455