Coloring needs to be overhauled

[1Rogue]

ColoredString should be deprecated/removed, and should instead utilize

ChatColor.translateAlternateColorCodes('&', yourString);

IMO a FilterState should have a private toggle-able boolean state for whether or not the string is colored. Currently, you can easily bypass coloring via a message like

&1Oh no I'm sad &&44but wait there's more!

Recursively searching for layers of that would be gross, just have the ability to set whether the message will be colored, and return according on your get method:

public String getFormattedString() {
    return (this.isColor) ? ChatColor.translateAlternateColorCodes('&', yourString) : yourString;
}

[ptoal]

ColoredString should be deprecated? Perhaps you don't understand it's purpose.

ColoredString does use translateAlternatColorCodes in the constructor.

The purpose of the ColoredString object is to provide a mechanism to match against the text of the message, independently of the formatting. Some people want to preserve formatting, while manipulating the text itself.

I don't understand why you think the internal FilterState object needs to know if the string is coloured or not?

Also, in PwnFilter 4.0, the filter engine and bukkit plugin are going to be separated from each other. The FilterEngine will, by default, have no awareness of bukkit formatting, though there will be a mechanism for operating on the "raw" string, if there is a desire to match/replace on the formatting.

[1Rogue]

I understand that you use the translation method in the ColoredString constructor, that's the problem. By default, this is the order of operations on someone without permissions for color:

• Input string &&bbThis is a test
• String is colored &<COLOR>bThis is a test
• Player does not have perm, string has color removed &bThis is a test
• String is re-colored on output <COLOR>This is a test

Meaning input like &&44Hey guys will be colored regardless of a user's permissions.

If the purpose of having a CharSequence implementation is to "ignore" formatting codes, then this current system isn't correct, considering I just have to nest formats to circumvent input.

[ptoal]

In the case above, I believe that if the only plugin you had installed was PwnFilter, what would be output is: '&bThis is a test'.

If you are using PwnFilter to decolor the string, but also using, for example, Essentials, and your user has the essentials.chat.color permission, then PwnFilter would strip the "valid" color code from the middle, and then Essentials would process the remaining color code. I don't think this is an issue with PwnFilter, per-se. Rather, what is happening is that after PwnFilter is done processing the message, you end up with something that another plugin in the event handler is converting into a formatting string.

One way around this would be to deny users the ability to use the ampersand character, by using a rule such as: match & then replace

Another option, as the last rule in your ruleset, you could do: match &+[0-9a-fk]+ then replace

That way, PwnFilter would check one last time for anything that would be a valid color code, and strip it before handing the event off to the next handler.

[ptoal]

I am going to close this ticket, and followup in the DBO tracker: http://dev.bukkit.org/bukkit-plugins/pwnfilter/tickets/60