I made changes and comments explaining the reasoning:
using System;
using MCGalaxy;
using MCGalaxy.Events.ServerEvents;
using MCGalaxy.Events.PlayerEvents;
// REQUIRES THE LATEST VERSION OF MCGALAXY!!!!!!!!!!!!!!!!!!!!!!!!!!!
namespace PluginToneIndicators
{
public sealed class ToneIndicators : Plugin
{
public override string name { get { return "tones"; } }
// This should indicate the earliest version of MCGalaxy the plugin is compatible with, rather than the plugin's version
// Unfortunately we must use the current version because the development build of MCGalaxy hasn't incremented the version number
public override string MCGalaxy_Version { get { return "1.9.4.9"; } }
public override string creator { get { return "AllergenX"; } }
public override void Load(bool startup) {
OnChatEvent.Register(OnChat, Priority.High);
}
public override void Unload(bool shutdown) {
OnChatEvent.Unregister(OnChat);
}
public static void OnChat(ChatScope scope, Player source, ref string msg, object arg, ref ChatMessageFilter filter, bool relay) {
//The general criticism is that you have a lot of code that is copy-pasted and doing the same thing.
//This can be a problem because if you have to fix or change something, you have to be able to accurately
//copy and paste that change into many places, which can be a common source of mistakes.
//For instance, your original code had this:
//
// else if (msg.CaselessContains("/srs"))
// {
// const string removeString = "/nsrs";
//
//Beause of the copy pasting of each tone, you forgot to change the if /srs to if /nsrs
//To solve this, we can write a function that handles the parts that were copy pasted before,
//and pass arguments for the parts that are different, for much less repeated text.
if (HandleTone("/j", "joking", ref msg)) { return; }
if (HandleTone("/s", "sarcasm", ref msg)) { return; }
if (HandleTone("/hj", "half-joking", ref msg)) { return; }
if (HandleTone("/srs", "serious", ref msg)) { return; }
if (HandleTone("/nsrs", "not-serious", ref msg)) { return; }
if (HandleTone("/r", "romantic", ref msg)) { return; }
if (HandleTone("/t", "teasing", ref msg)) { return; }
//^ If the given tone was handled, we're done, so we can quit the function early with return.
//You could also use else-if. It's a personal preference IMO
}
// Returns true if the tone was found and the message was modified, otherwise false
public static bool HandleTone(string tone, string prefix, ref string msg) {
int toneStart = FindTone(tone, msg);
if (toneStart == -1) { return false; }
string startOfString = msg.Substring(0, toneStart);
string endOfString = msg.Substring(toneStart + tone.Length);
// Remove a space to prevent double spaces when removing a tone
if (endOfString.Length > 0 && endOfString[0] == ' ') { endOfString = endOfString.Substring(1); }
string cleanString = startOfString + endOfString;
msg = ("&7[&a"+prefix+"&7] " + cleanString);
return true;
}
// Returns the index of a tone if it was found, -1 if not found
public static int FindTone(string tone, string msg) {
int toneStart = msg.IndexOf(tone);
if (toneStart == -1) { return -1; }
//We need to make sure the tone isn't part of another word or tone.
//Both sides of the word must either be a space, or the start/end of the message
bool leftSideClear = false;
bool rightSideClear = false;
// Bonus info: when using || in if statements to do "OR", as soon as one of the conditions is true, it skips over the remaining ones.
// This means that we do not have to do string bounds checking when looking at index -1 because
// it's guaranteed that toneStart is not zero
if (toneStart == 0 || msg[toneStart-1] == ' ') { leftSideClear = true; }
if (toneStart + tone.Length == msg.Length || msg[toneStart+tone.Length] == ' ') { rightSideClear = true; }
if (leftSideClear && rightSideClear) { return toneStart; }
else { return -1; }
}
}
}