This looks a lot like code that someone was asking about in the help channel. It's different than the last time they pastebin'ed it, but it still has problems from an overview look at it, as there's no point in trying to load it since I'm not in a channel where I'm encountering this kind of thing. And I wouldn't want to try running it, due to all the false positives that can easily trigger it.
You didn't give an idea about what's wrong with the code. Whether it just spits out errors left and right, or it does nothing, or it does things it shouldn't etc.
So I'll just make a mostly generic post about how to debug your own scripts.
--
While you're trying to get rid of the wrinkles, you should include some debugging messages to help see if/when things are going wrong. There should be some echoes to identify when the script is reaching certain key rows, so you can see if it's reaching a line that it shouldn't, or vice-versa. The echoes should include $scriptline so you can see which debug message is being displayed, as well as $nopath($script) so you can find stray debug messages that don't get removed when they should.
Using the -s switch with /set /inc etc is a great way to send debugging info without creating another echo statement, and is helpful when creating compound variable names, to not only see what number/string it's being set to, but also exactly what kind of compound variable name is being created. Sometimes things don't happen because an extra/missing square brace causes the wrong compound name to be created, or causes the string to contain things they shouldn't.
For some of the debugging echoes, it's helpful to show contents of variable names to help identify why the code is branching the wrong direction when it reaches an if() or while(). You can even mimic an if() or while() command using an echo immediately above it, though you rarely can simply copypaste the if() command following an "echo -s" and expect it to be intelligible. The parsing rules are different within an echo than they are within commands, for things like how they handle text that's touching parenthesis and commas, so you might need to insert extra spaces.
It also helps to include ;semicolon comments in your code while making it work, because it helps get you up to speed when you look at it the next day. Specifically, when you have several { section } of if() conditionals, it helps to be able to look at a closing curly brace and see what it's the close of, especially for a section that covers a dozen or more rows
Also, even though mSL lets you get away with not putting parenthesis around things which other languages would insist on, but you're better off using parenthesis unless you're coding something in one of those small-code challenges.
For one thing, parenthesis give visual breaks that make it easier to read the code, so you can identify the sections of the command more easily. For another thing, the presence/absence of parenthesis can affect how the $v1 and $v2 values are filled. For example, in your first if() you have parenthesis around the outer condition, but don't have it around each of the twin conditions inside it. This can cause $v1 and $v2 to be filled differently than if you had parenthesis around the twins. For example, take a look at:
//if ($left(0,1) isin abc || o isin foo) echo -a match $v1 vs $v2 | else echo -a nomatch $v1 vs $v2
//if ($left(0,1) isin abc || z isin foo) echo -a match $v1 vs $v2 | else echo -a nomatch $v1 vs $v2
//if (($left(0,1) isin abc) || (o isin foo)) echo -a match $v1 vs $v2 | else echo -a nomatch $v1 vs $v2
In the 1st 2 examples, the 1st twin is always false, however $v1 and $v2 are being correctly filled only when both twins are false. However the 3rd example correctly fills $v1 and $v2 based on the 2nd condition which was the final condition evaluated and actually caused the code to branch that direction. If you have $v1 and $v2 shown in one of your debugging echoes, the lack of parenthesis can cause you to be mis-informed as to why the code branched the way it did.
It's just personal preference, and I know it's syntax common in other languages, but I like to avoid the "%variable = string" syntax because of how mSL is designed. It's always a visual speedbump for me when I'm trying to read mSL code, because I'm expecting to see an actual command name at the front of each command, so my first reaction is seeing an unrecognized %commandname. Also, when I see "%name = string", I then need to look to see whether the intent is to modify a local /var created above, or whether this is intentionally trying to modify a global variable that should have somehow been created elsewhere in the code for later events to deal with. By always using /set or /var, there's no ambiguity, and it makes it easier for me to see when a command is using a variable name that it assumes is blank without properly initializing it to defend against global variables of the same name - created by other scripts because people use common names like %color or because the script didn't clean up after itself.
Having code be more complicated also makes it harder to read through when looking for errors.
var %axel = 1 | while ($gettok(%clnicks1,%axel,44)) { if (%clonednicks.flood isin $gettok(%clnicks1,%axel,44)) { set -u30 %clnicks2 $addtok(%clnicks2,$gettok(%clnicks1,%axel,44),44) } | inc %axel }
For example in the above, $gettok(%clnicks1,%axel,44) is used 3 times when only once is needed. If there's another if() between the if() which creates the $v1 and when you need to use the string, you can park it into %v1 or some other temp name until you need it. When you you use it the 2nd time, that string is already in $v1 from the while(), and when using it the 3rd time it's already in the $v2 created by the most recent if(), so there's no need to use the $gettok again. It would make the code shorter and easier for you to read, as well as making it much quicker for the engine to retrive the string stored in $v2 instead of telling $gettok to parse the original string again. And again. Though be sure to have the choice of $v1 or $v2 reflect the most recent conditional, and you can often rearrange the left/right halves of evaluations to keep the string always in the same $v1 instead of bouncing between them. Even something like "if ($null == $gettok(%clnicks1,%axel,44)" is legit if you're wanting to keep the value staying in $v2.
Also, if your code is working correctly, %clnicks2 is a temporary variable that should contain a list of nicks only for the duration needed to build the list for kicking, and you're wiping the nicks out of the list as soon as you kick them. That makes it pointless to /set a global variable with the -uN trigger for unsetting it, so you can just use /var to create the temporary variable. I always use /var unless I'm needing to create a global variable so an alias subroutine can see it without needing to pass it as an argument, or if it needs to be seen later on in another $event.
Also, %clnicks2 should be empty when it reaches that code, so if there's a possibility that it's not, you should be initializing it by unsetting it or setting the local copy of it to be blank. That also means you don't need the extra slowdown of using $addtok, when you can modify your code a little to avoid it, like:
var %axel = 1 | while ($gettok(%clnicks1,%axel,44)) { if (%clonednicks.flood isin $v1) { var %clnicks2 %clnicks2 $+ , $+ $v2 } | inc %axel }
if (%modechan1MR != $true) { .raw mode $chan +MRb $+(%clonednicks.flood,*!*@*) | echo $chan 01,08 ( cloned nicks Flood %clnicks2 ) !!!!! Channel Locked !!!!! | set -u15 %modechan1MR $true | .timermjs1RM 1 30 mode $safe2018($chan) -MR }
var %mcln 20 | while ($gettok(%clnicks2,1- %mcln,44) != $null) { kick $chan $v1 Cloned Nicks ) Detected | var %clnicks2 = $deltok(%clnick2,1- %mcln,44) } }
I added %clnicks2 to the echo message just to identify which of the recent joins was triggering it, and what's the magnitude of the flood, as well as making it easier to notice that someone's nick has been vacuumed up in the flood cleanup when they shouldn't have been.
I used the "!= $null" even though it's not needed, because that makes it easier for me to read my code, and also helps avoid the pitfall of treating tokens of 0 and $false the same as blank. It's not a problem here since nicks cannot begin with 0 under normal circumstances, but sometimes I see people using this against $1- in an ON TEXT event, which causes a failure when a word in the middle of the string is 0 or $false. Also, some IRCD's have the server give an alphanumeric nick to avoid a nick collision, and I suppose it's remotely possible that such a nick could be 00000ABC.
The above code creates a leading comma, but since the variable is now accessed only by $gettok, it's invisible, and the 1st $deltok removes it.
As an aside for code that's difficult to handle because there's a compound variable name that needs to contain several different variables, the code can be a lot simpler by storing the variables instead as hashtable items. Once you know how to use hashtables, you don't need to use square braces at all when creating items with /hadd nor when using $hget to retrieve their values. You can simply use $+($network,.,$chan,.,$nick) in both situations.
If it makes your code simpler to deal with, you can also have compound variable names used to preserve data between triggering events, then immediately transfer them to a simpler local variable name when needing to manipulate the data.
--
Now for design issues I see from looking at your code.
%clnicks1 is a variable that exists as soon as someone joins the channel, and it doesn't vanish until there's a 30 seconds interval when nobody joins. Until that interval happens, it keeps growing with tokens, so there's a potential for this to encounter $maxlenl if it's a large channel that naturally has a lot of legit joins without a 30 seconds pause.
It looks like you're doing an XY problem by giving us only part of your code, as evidenced by how you only gave us part of a $deltok, which would have created an error message that wouldn't have required posting for help about it. Unless your unstated problem is that the code never even reaches the $deltok in order to cause that error.
You have an identical if(this||that) inside the top one. It doesn't cause an error, but it would always be $true since your code can't do anything to cause it to become $false.
It also looks like the existing design is a great way to get yourself kicked for flooding, because of how it sends un-neccessary kick commands to the server. It kicks nicks even after you've already sent a command to kick them.
When RobertSmith joins, nothing happens except adding Rober to the list of nick prefixes who joined since the last time there was a 30-seconds interval where nobody was joining. Then as soon as RobertJones joins, it goes into panic mode and assumes that the channel is under attack. While it won't kick anyone in channel who's been in there a while, that "while" can be a long time if there hasn't been a 30 seconds interval which allows %clnicks1 to go away.
It's also vulnerable to some trickster activity. Someone could have a clone waiting in the wings for an @op to timeout and rejoin, or just waiting for anyone to join. As soon as the @op rejoins the channel, or after some random person joins the channel, they have their nickchange ready and have it join the channel too, and this gets the @op or RandomDude kicked, even if they've already been opped or voiced, since this code doesn't check the status of their victim, nor does it have any whitelist.
It doesn't even check if it's trying to kick someone who's no longer in the channel, and unless there's additional code not included, someone is also exempt from having their flood clones kicked if they wait until after joining then change their nick again. When I run this next snippet against #libera I get over 80 nicks who share the 1st 5 letters of their nick with someone else.
//var %i 1 , %chan #libera | hfree -w foo | hmake foo | while ($nick(%chan,%i)) { hinc foo $left($v1,5) | inc %i } | var -s %i 1 | while ($hget(foo,%i).item != $null) { var %v1 $v1 | if ($hget(foo,%v1) != 1) echo -a %v1 $v1 | inc %i } | echo -a nicks affected: $calc( $nick(%chan,0) - $hget(foo,0).item )
While some of them are intentional clones, many of them are not, including the dozen Guest nicks. So I'm afraid this design is going to be doing a lot of kicking that it shouldn't, since the chances are excellent that 2 unrelated people with similar nicks can join the channel at near the same time since the last time there was a 31 seconds gap between joins, especially when boomeranging from a netsplit. Plus, all the flooder needs to do is start coming in with dissimilar nicks.
In fact, one of the most easy ways for this to trigger a false positive is the common habit of having mIRC configured so that $anick is the same as $mnick except for appending _away to it. When nicks timeout, it's very common for them to rejoin the server and the channels before the server has killed the old nick for not responding, so I'm surprised you're not getting these kinds of innocents being hit by your drive-by kicks.
But even when it's doing the intended job against a legit flooder: When foobar1 joins, nothing happens except fooba is added to the list of recent joins. When foobar2 joins, then it tries to kick both foobar1 and foobar2. It's reasonable to assume that flooders can join in a short interval before the server can respond to perform the kick, so there's a chance that foobar3 joins before the server can perform the kick. Since foobar1 and foobar2 are still in %clnicks1, this means that it would send another kick attempt against foobar1 and foobar2, as well as against foobar3. The solution isn't to remove nicks from the list as soon as you kick them, because then foobar3 would no longer be flagged.
Now I look at:
if (%clonesjoin. [ $+ [ # ] $+ . $+ [ %clonednicks.flood ] ] > number)
Based on your script's logic, there should be no need to kick multiple nicks unless the clones counter is <= 'number+1'. When the counter exceeds number+1, it should assume that clones 1 thru number+1 were already kicked, and since you're adding nicks to the list in the order they're joining, you'd only need to kick the final nick in the list who matched %clonednicks.flood, which would by definition also be $nick
